Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: use IDs for columns instead of names in serialized expressions/queries #22212

Closed
BramGruneir opened this issue Jan 30, 2018 · 9 comments
Closed
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-semantics C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@BramGruneir
Copy link
Member

BramGruneir commented Jan 30, 2018

Instead of storing the name of columns in expressions, we should instead store column IDs. This makes renaming a column super quick. And should speed up evaluation as no string matching needs to occur.

Applications:

  • view queries
  • stored DEFAULT and CHECK expressions
  • perhaps other places (e.g. the statement used for a background job?)
@knz
Copy link
Contributor

knz commented May 14, 2018

@BramGruneir can you remind me what prompted this issue? Where do we store expressions in a place that would benefit from storing IDs instead?

@knz knz added the C-investigation Further steps needed to qualify. C-label will change. label May 14, 2018
@BramGruneir
Copy link
Member Author

This was prompted from when I was exploring some of the column renaming logic. It seemed to me that we optimized for the wrong part, the pretty printing, instead of the more common case of evaluation. Evaluating any expression should never rely on the column names and they should be converted to IDs as soon as possible.

It's been a while since I looked at that code, but I remember being confused by it.

@knz
Copy link
Contributor

knz commented May 14, 2018

FYI the column names are converted to numeric IDs very early on (during name resolution), so for the purpose of query execution we already do what you suggestion.

The one area where this is not yet true is when storing an expression in a descriptor, e.g. for view queries, DEFAULT or CHECK. Was this what you had in mind?

@BramGruneir
Copy link
Member Author

Yep. I was dealing with both of those during my deep dive into FK land.

@knz
Copy link
Contributor

knz commented May 17, 2018

Ok then this issue greatly overlaps with #10083.

What we need is a serialization format which doesn't embed names, just IDs. USe that both for views and other things that store queries/expressions.

@knz knz changed the title sql: use IDs for columns instead of names in expressions sql: use IDs for columns instead of names in serialized expressions/queries May 17, 2018
@knz knz added A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-semantics C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) and removed C-investigation Further steps needed to qualify. C-label will change. labels May 17, 2018
@knz
Copy link
Contributor

knz commented May 17, 2018

For reference, how postgres does it: when a SQL query/expression gets serialized, it translates the names of things (table names, column names etc) to OIDs, and it stores the OID. The OID is translated back to a name when pretty-printing.

We may be able to do something similar.

@github-actions
Copy link

github-actions bot commented Jun 8, 2021

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@ajwerner
Copy link
Contributor

Related to #56592 and #24559.

@ajwerner
Copy link
Contributor

Duplicates #10083.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-semantics C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

4 participants