-
Notifications
You must be signed in to change notification settings - Fork 325
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
Complete initial pass on virtual tables #858
Conversation
f2a29bb
to
4ff4c6d
Compare
163e73d
to
53d227f
Compare
6a95e34
to
b3cc7b5
Compare
core/translate/planner.rs
Outdated
Ok(TableReference { | ||
op: Operation::Scan { iter_dir: None }, | ||
join_info: None, | ||
table: Table::Virtual(vtab.clone().into()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if we could get rid of enum TableReferenceType
if Table::Virtual
had the args
in it. Semantically it sort of fits I think, generate_series(1,10,1)
is an instantiation of generate_series
with those arguments. but mainly I'm just looking for excuses not to have TableReferenceType
anymore :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gahhh I swear was wondering about that I almost just removed it. Yeah I think it is, and it applies in the case of other examples I am thinking of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok that wasn't so bad 👍
b7bee6f
to
eb06cfa
Compare
eb06cfa
to
ae88d51
Compare
EDIT:
Ok this should finally be all set. There were some massive changes that had taken place since the original PR 🫠 🫠 merge conflicts were serious
As soon as this is merged, I will finish the rest of what is needed for
CREATE VIRTUAL TABLE some_table USING some_extension('some argument or sql');
I added tests for the series extension.
This PR is already big and covers a LOT of surface area, so we should ideally try to get this in first, and I'll go back and complete the rest of the functionality 👍