Implement SQL parsing for CREATE TABLE .. FROM SOURCE [ENG-TASK-17] #28125
Conversation
rjobanp
force-pushed
the
create-table-from-source
branch
from
d3653b4
to
748330f
Compare
src/sql-parser/src/parser.rs
Outdated
| // and include full definitions | ||
| match &columns { | ||
| None => { | ||
| return self.expected( |
There was a problem hiding this comment.
Checks like this and the diverging parse paths suggest that we may instead need a CreateTableFromSource top level struct. Thoughts? Would that make parsing easier to think about and remove the need for this verification?
There was a problem hiding this comment.
Yes and no. We'd still need many of these checks during parsing since columns are optional in the 'from source' mode, and the column references can just be names rather than both names and types. But it could make downstream logic easier by having distinct CreateTableStatement and CreateTableFromSourceStatement structs, since I did have to make a couple of fields on CreateTableStatement optional to accommodate these semantics.
There was a problem hiding this comment.
Ok. So that's same parsing but separate structs? That SGTM. Big fan of that if it prevents adding downstream validation that was previously not needed.
There was a problem hiding this comment.
Yep exactly! Thanks for the feedback, will refactor it when I'm back next week
There was a problem hiding this comment.
Alright I've replaced this commit with one that introduces a new top-level statement and it does make things dramatically simpler
rjobanp
force-pushed
the
create-table-from-source
branch
from
748330f
to
6401eb4
Compare
rjobanp
force-pushed
the
create-table-from-source
branch
from
6401eb4
to
b8fc853
Compare
bosconi
changed the title
Motivation
Initial scaffolding for the work described in #27907 in service of https://github.com/MaterializeInc/database-issues/issues/6051
This just implements parsing of the new statement and will return an error during planning if this statement is actually used. In a follow on PR I'll be working on the planning bits.
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.