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: allow virtual table descriptors in a concrete db #10319

Closed
knz opened this issue Oct 30, 2016 · 7 comments
Closed

sql: allow virtual table descriptors in a concrete db #10319

knz opened this issue Oct 30, 2016 · 7 comments
Labels
A-schema-descriptors Relating to SQL table/db descriptor handling. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Milestone

Comments

@knz
Copy link
Contributor

knz commented Oct 30, 2016

Requested by @petermattis in #10317 (comment): host the tables in cdb_internal in system instead.

To do this a couple of steps are needed:

  1. defer the virtual description initialization code to the point after the system descriptor is created and available.
  2. change the initialization code to allow a concrete db descriptor as parent descriptor for a virtual schema.
  3. change the virtual table lookup code in data_source.go

@nvanbenschoten are there any other steps needed?

@knz
Copy link
Contributor Author

knz commented Oct 30, 2016

Alternatively, we can hide the concrete system descriptor and create a new virtual schema descriptor also called "system" which proxies the system tables.

@dt
Copy link
Member

dt commented Oct 31, 2016

This would be a side effect of a goal @nvanbenschoten, @a-robinson, @paperstreet and I have discussed a couple times: moving to having physical descriptors for virtual tables, just like for normal tables and views, with the only difference from a view being that they'd have an enum pointing to a row production function rather than a string query.

IIRC we all think such a unification would remove some special-cases and simplify some of the descriptor handling code, but we're waiting on that until we have a better story for upgrades -- right now, since the virtual tables are entirely synthetic, we can iterate on them without having to consider migrating older, persisted versions.

@knz
Copy link
Contributor Author

knz commented Oct 31, 2016

Yes I sense there is much to be gained by persisting and versioning virtual table descriptors.

Versioning is another interest of mine (which extends to other things including our grammar and builtin functions), because as we change these semantics we run the risk of invalidating existing views that use them.

@nvanbenschoten
Copy link
Member

nvanbenschoten commented Oct 31, 2016

Yeah I think this would be a natural result of persisting virtual descriptors (see #9361). The complication, as discussed above and in the issue, is that our descriptor migration story becomes much more critical. #9404 is a big step in the right direction, but its full implementation may be a ways off, and it still adds a lot of overhead to the currently trivial task of adding virtual tables. With information_schema and pg_catalog in flux, I'd like to avoid adding such a large barrier.

In the short-term, I wouldn't be opposed to adding some aliasing logic to virtual descriptors, so that we could hang virtual tables off of system. This may present some complications if we don't special-case system and try to make this work generally for all databases, but it's certainly doable.

@dt
Copy link
Member

dt commented Oct 31, 2016

my $0.02: I'd rather we not add more special-cases to descriptor handling, and instead just wait for #9404 to do this The Right Way.

@nvanbenschoten
Copy link
Member

I agree. If this issue isn't critical to resolve soon, I'd rather we just wait.

@petermattis petermattis added this to the Later milestone Feb 23, 2017
@knz knz added A-schema-descriptors Relating to SQL table/db descriptor handling. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) labels Apr 28, 2018
@knz
Copy link
Contributor Author

knz commented May 3, 2018

I've been keeping an eye on this issue and I think the initial phrasing, as-is, is not relevant any more. With #22371 merged, the ship has sailed on the idea that there are virtual schemas that automagically appear in every database; that must be true for compatibility for information_schema and pg_catalog, and we made it true (for better or for worse) for crdb_internal.

The idea to "persist" virtual schemas that are to appear in every database doesn't make sense any more. I am closing this issue as a result.

@knz knz closed this as completed May 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-descriptors Relating to SQL table/db descriptor handling. 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