iffyio commented on code in PR #1927:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1927#discussion_r2216110543
##########
tests/sqlparser_common.rs:
##########
@@ -16031,6 +16031,30 @@ fn parse_create_procedure_with_parameter_modes() {
}
}
+#[test]
+fn parse_not_null_supported() {
+ let _ = all_dialects().expr_parses_to("x NOT NULL", "x IS NOT NULL");
+ let _ = all_dialects().expr_parses_to("NULL NOT NULL", "NULL IS NOT NULL");
+}
+
+#[test]
+fn test_not_null_precedence() {
+ assert_matches!(
+ all_dialects().expr_parses_to("NOT NULL NOT NULL", "NOT NULL IS NOT
NULL"),
+ Expr::UnaryOp {
+ op: UnaryOperator::Not,
+ ..
+ }
+ );
+ assert_matches!(
+ all_dialects().expr_parses_to("NOT x NOT NULL", "NOT x IS NOT NULL"),
+ Expr::UnaryOp {
+ op: UnaryOperator::Not,
+ ..
+ }
+ );
+}
Review Comment:
Similar to the `notnull` scenarios, can we merge these into the same
function?
##########
tests/sqlparser_common.rs:
##########
@@ -16174,3 +16198,39 @@ fn test_identifier_unicode_start() {
]);
let _ = dialects.verified_stmt(sql);
}
+
+#[test]
+fn parse_notnull_unsupported() {
+ // Only Postgres, DuckDB, and SQLite support `x NOTNULL` as an expression
+ // All other dialects consider `x NOTNULL` like `x AS NOTNULL` and thus
+ // consider `NOTNULL` an alias for x.
+ let dialects = all_dialects_except(|d| d.supports_notnull_operator());
+ let _ = dialects
+ .verified_only_select_with_canonical("SELECT NULL NOTNULL", "SELECT
NULL AS NOTNULL");
+}
+
+#[test]
+fn parse_notnull_supported() {
+ // Postgres, DuckDB and SQLite support `x NOTNULL` as an alias for `x IS
NOT NULL`
+ let dialects = all_dialects_where(|d| d.supports_notnull_operator());
+ let _ = dialects.expr_parses_to("x NOTNULL", "x IS NOT NULL");
+}
+
+#[test]
+fn test_notnull_precedence() {
+ // For dialects which support it, `NOT NULL NOTNULL` should
+ // parse as `(NOT (NULL IS NOT NULL))`
+ let supported_dialects = all_dialects_where(|d|
d.supports_notnull_operator());
+ let unsupported_dialects = all_dialects_except(|d|
d.supports_notnull_operator());
+
+ assert_matches!(
+ supported_dialects.expr_parses_to("NOT NULL NOTNULL", "NOT NULL IS NOT
NULL"),
+ Expr::UnaryOp {
+ op: UnaryOperator::Not,
+ ..
+ }
+ );
+
+ // for unsupported dialects, parsing should stop at `NOT NULL`
+ unsupported_dialects.expr_parses_to("NOT NULL NOTNULL", "NOT NULL");
+}
Review Comment:
Since they're the same feature, can we merge these scenarios into the same
`fn parse_notnull(){}` test function?.
Also for the comments, ideally we skip mentioning the individual dialects
(Postgres, duckdb etc), so that the comment doesnt' become stale as other/new
dialects are extended with support for this feature.
--
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
