iffyio commented on code in PR #1531:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1531#discussion_r1846910729
##########
src/dialect/mod.rs:
##########
@@ -638,6 +638,11 @@ pub trait Dialect: Debug + Any {
false
}
+ /// Returns true if the dialect supports the `UNLISTEN` statement
+ fn supports_unlisten(&self) -> bool {
+ false
+ }
Review Comment:
Oh actually I was initially wondering if it makes sense to reuse the
existing `supports_listen()` flag for this but then also realised we could
introduce a `dialect.supports_listen_notify()` to mark the feature itself,
replacing `dialect.supports_listen()`, `dialect.supports_notify()` and
`dialect.supports_unlisten()`. Thinking since it would be expected for example
that a dialect supporting listen should also support notify?
##########
src/dialect/postgresql.rs:
##########
@@ -209,6 +214,11 @@ impl Dialect for PostgreSqlDialect {
fn supports_comment_on(&self) -> bool {
true
}
+
+ /// See <https://www.postgresql.org/docs/current/sql-load.html>
+ fn supports_load_extension(&self) -> bool {
+ true
+ }
Review Comment:
Oh, I'm not sure I followed this change, I can see this flag and tests for
it but there doesn't seem to be any changes to the parser itself?
##########
tests/sqlparser_common.rs:
##########
@@ -11864,6 +11895,56 @@ fn parse_load_data() {
);
}
+#[test]
+fn test_load_extension() {
+ let dialects = all_dialects_where(|d| d.supports_load_extension());
+ let only_supports_load_data_dialects =
+ all_dialects_where(|d| !d.supports_load_extension() &&
d.supports_load_data());
+ let not_supports_load_dialects =
+ all_dialects_where(|d| !d.supports_load_data() &&
!d.supports_load_extension());
Review Comment:
given we're only looking to test the load_extension syntax, would it make
sense to only keep the test scoped to only `all_dialects_where(|d|
d.supports_load_extension())` in order to keep the test focused?
Otherwise its not clear if/why the load_data syntax is relevant here. Then
for the `not_supports_load_dialects` I think we can skip verifying the parser
error on other dialects, since by definition they don't support the syntax so
its not super important
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
