-
Notifications
You must be signed in to change notification settings - Fork 554
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
Parse Postgres's LOCK TABLE statement #1614
base: main
Are you sure you want to change the base?
Parse Postgres's LOCK TABLE statement #1614
Conversation
Handle empty projection for pg
See: https://www.postgresql.org/docs/current/sql-lock.html PG's full syntax for this statement is supported: ``` LOCK [ TABLE ] [ ONLY ] name [ * ] [, ...] [ IN lockmode MODE ] [ NOWAIT ] where lockmode is one of: ACCESS SHARE | ROW SHARE | ROW EXCLUSIVE | SHARE UPDATE EXCLUSIVE | SHARE | SHARE ROW EXCLUSIVE | EXCLUSIVE | ACCESS EXCLUSIVE ``` It is implemented to not intefere with the roughly equivalent (but different) syntax in MySQL, by using a new Statement variant.
7ce8f33
to
47a3e5e
Compare
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.
Thanks @freshtonic!
let projection = if dialect_of!(self is PostgreSqlDialect | GenericDialect) | ||
&& self.peek_keyword(Keyword::FROM) | ||
{ | ||
vec![] | ||
} else { | ||
self.parse_projection()? | ||
}; |
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.
we can probably skip these changes in this PR given it's now in #1613?
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.
@iffyio oh my bad - I should have branched from the apache main
branch instead of our fork's main
before pushing. I'll remedy this.
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.
@iffyio I tried this and it's not straightforward without storing a value on the variant the identifies the dialect that was used to parse the AST.
The following syntax would be problematic (to render, in Display
):
LOCK customers;
In PG, the TABLE
keyword is optional. In MySQL one of TABLE
or TABLES
is mandatory.
The Display
impl for Statement
, in the LockTable { .. }
match arm could potentially generate SQL that will not be parsable by Postgres if a TABLES
keyword is emitted.
Is there precedent for choosing how to render an AST fragment using a stored value to encode the dialect (or a proxy to the dialect) that was used to parse the AST?
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.
ah yeah we could probably use an enum to represent the variants, something like?
enum LockTableKind {
TABLE
TABLES
}
Statement::LockTable { table_kind: Option<LockTableKind> }
see TableSampleKind for example
See: https://www.postgresql.org/docs/current/sql-lock.html
PG's full syntax for this statement is supported:
It is implemented to not intefere with the roughly equivalent (but different) syntax in MySQL, by using a new Statement variant.