freshtonic commented on code in PR #1614:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1614#discussion_r1903354362
##########
src/ast/mod.rs:
##########
@@ -7278,16 +7279,126 @@ impl fmt::Display for SearchModifier {
}
}
+/// A `LOCK TABLE ..` statement. MySQL and Postgres variants are supported.
+///
+/// The MySQL and Postgres syntax variants are significant enough that they
+/// are explicitly represented as enum variants. In order to support additional
+/// databases in the future, this enum is marked as `#[non_exhaustive]`.
+///
+/// In MySQL, when multiple tables are mentioned in the statement the lock mode
+/// can vary per table.
+///
+/// In contrast, Postgres only allows specifying a single mode which is applied
+/// to all mentioned tables.
+///
+/// MySQL: see <https://dev.mysql.com/doc/refman/8.0/en/lock-tables.html>
+///
+/// ```sql
+/// LOCK [TABLE | TABLES] name [[AS] alias] locktype [,name [[AS] alias]
locktype]
+/// ````
+///
+/// Where *locktype* is:
+/// ```sql
+/// READ [LOCAL] | [LOW_PRIORITY] WRITE
+/// ```
+///
+/// Postgres: See <https://www.postgresql.org/docs/current/sql-lock.html>
+///
+/// ```sql
+/// LOCK [ TABLE ] [ ONLY ] name [, ...] [ IN lockmode MODE ] [ NOWAIT ]
+/// ```
+/// Where *lockmode* is one of:
+///
+/// ```sql
+/// ACCESS SHARE | ROW SHARE | ROW EXCLUSIVE | SHARE UPDATE EXCLUSIVE
+/// | SHARE | SHARE ROW EXCLUSIVE | EXCLUSIVE | ACCESS EXCLUSIVE
+/// ``````
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
-pub struct LockTable {
- pub table: Ident,
+#[non_exhaustive]
+pub enum LockTables {
Review Comment:
> I think we want to avoid variants that are specific to dialects, those
tend to make it more difficult to reuse the parser code and ast representations
across dialects
MySQL's & Postgres's `LOCK` statements have minimal overlap. They are
similar in name only.
- MySQL allows different lock modes per table vs Postgres one lock mode
applied to all tables
- MySQL's lock modes and Postgres's lock modes do not overlap at all
- MySQL has freely interchangeable table keywords, one of which MUST be
present: `TABLE` or `TABLES`
- Postgres has one `TABLE` keyword but it's optional
- Postgres supports additional (optional) `ONLY` and `NOWAIT` keywords
- It is _never_ valid to mix and match the Postgres-specific syntax with
MySQL-specific syntax.
I agree that explicit database-specific AST fragments make parser reuse more
difficult but in the case of `LOCK` pretty much nothing is reusable.
The db-specific AST pieces do make implementing `Display` a lot less error
prone.
It also makes it (almost) impossible be able to represent invalid AST (e.g.
a mix of PG & MySQL) except I didn't take a hard stance on this for
`LockTableType` which does mix MySQL & Postgres bits.
None of this is a hill I will die on, but handling the burden of grammar
differences in the parser and AST design means _consuming_ the AST correctly in
downstream projects will be easier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
