fixup: handle expr NOT NULL directly in parse_subexpr.

ryanschneider · ryanschneider · commit 307c465cbe04 · 2025-07-10T17:05:13.000-07:00
diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs
@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::dialect::{Dialect, IsNotNullAlias};
+use crate::dialect::Dialect;
 
 /// A [`Dialect`] for [DuckDB](https://duckdb.org/)
 #[derive(Debug, Default)]
@@ -95,12 +95,9 @@ impl Dialect for DuckDbDialect {
         true
     }
 
-    /// DuckDB supports `NOT NULL` and `NOTNULL` as aliases
-    /// for `IS NOT NULL`, see DuckDB Comparisons <https://duckdb.org/docs/stable/sql/expressions/comparison_operators#between-and-is-not-null>
-    fn supports_is_not_null_alias(&self, alias: IsNotNullAlias) -> bool {
-        match alias {
-            IsNotNullAlias::NotNull => true,
-            IsNotNullAlias::NotSpaceNull => true,
-        }
+    /// DuckDB supports `NOTNULL` as an alias for `IS NOT NULL`,
+    /// see DuckDB Comparisons <https://duckdb.org/docs/stable/sql/expressions/comparison_operators#between-and-is-not-null>
+    fn supports_notnull_operator(&self) -> bool {
+        true
     }
 }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
@@ -55,7 +55,6 @@ use crate::keywords::Keyword;
 use crate::parser::{Parser, ParserError};
 use crate::tokenizer::Token;
 
-use crate::dialect::IsNotNullAlias::{NotNull, NotSpaceNull};
 #[cfg(not(feature = "std"))]
 use alloc::boxed::Box;
 
@@ -651,17 +650,9 @@ pub trait Dialect: Debug + Any {
                 Token::Word(w) if w.keyword == Keyword::MATCH => Ok(p!(Like)),
                 Token::Word(w) if w.keyword == Keyword::SIMILAR => Ok(p!(Like)),
                 Token::Word(w) if w.keyword == Keyword::MEMBER => Ok(p!(Like)),
-                Token::Word(w)
-                    if w.keyword == Keyword::NULL
-                        && self.supports_is_not_null_alias(NotSpaceNull) =>
-                {
-                    Ok(p!(Is))
-                }
                 _ => Ok(self.prec_unknown()),
             },
-            Token::Word(w)
-                if w.keyword == Keyword::NOTNULL && self.supports_is_not_null_alias(NotNull) =>
-            {
+            Token::Word(w) if w.keyword == Keyword::NOTNULL && self.supports_notnull_operator() => {
                 Ok(p!(Is))
             }
             Token::Word(w) if w.keyword == Keyword::IS => Ok(p!(Is)),
@@ -1089,13 +1080,10 @@ pub trait Dialect: Debug + Any {
         false
     }
 
-    /// Returns true if the dialect supports the passed in alias.
-    /// See [IsNotNullAlias].
-    fn supports_is_not_null_alias(&self, alias: IsNotNullAlias) -> bool {
-        match alias {
-            NotNull => false,
-            NotSpaceNull => false,
-        }
+    /// Returns true if the dialect supports the `x NOTNULL`
+    /// operator expression.
+    fn supports_notnull_operator(&self) -> bool {
+        false
     }
 }
 
@@ -1123,13 +1111,6 @@ pub enum Precedence {
     Or,
 }
 
-/// Possible aliases for `IS NOT NULL` supported
-/// by some non-standard dialects.
-pub enum IsNotNullAlias {
-    NotNull,
-    NotSpaceNull,
-}
-
 impl dyn Dialect {
     #[inline]
     pub fn is<T: Dialect>(&self) -> bool {
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
@@ -28,7 +28,7 @@
 // limitations under the License.
 use log::debug;
 
-use crate::dialect::{Dialect, IsNotNullAlias, Precedence};
+use crate::dialect::{Dialect, Precedence};
 use crate::keywords::Keyword;
 use crate::parser::{Parser, ParserError};
 use crate::tokenizer::Token;
@@ -264,11 +264,8 @@ impl Dialect for PostgreSqlDialect {
     }
 
     /// Postgres supports `NOTNULL` as an alias for `IS NOT NULL`
-    /// but does not support `NOT NULL`.  See: <https://www.postgresql.org/docs/17/functions-comparison.html>
-    fn supports_is_not_null_alias(&self, alias: IsNotNullAlias) -> bool {
-        match alias {
-            IsNotNullAlias::NotNull => true,
-            IsNotNullAlias::NotSpaceNull => false,
-        }
+    /// See: <https://www.postgresql.org/docs/17/functions-comparison.html>
+    fn supports_notnull_operator(&self) -> bool {
+        true
     }
 }
diff --git a/src/dialect/sqlite.rs b/src/dialect/sqlite.rs
@@ -20,7 +20,7 @@ use alloc::boxed::Box;
 
 use crate::ast::BinaryOperator;
 use crate::ast::{Expr, Statement};
-use crate::dialect::{Dialect, IsNotNullAlias};
+use crate::dialect::Dialect;
 use crate::keywords::Keyword;
 use crate::parser::{Parser, ParserError};
 
@@ -111,12 +111,9 @@ impl Dialect for SQLiteDialect {
         true
     }
 
-    /// SQLite supports ``NOT NULL` and `NOTNULL` as
-    /// aliases for `IS NOT NULL`, see: <https://sqlite.org/syntax/expr.html>
-    fn supports_is_not_null_alias(&self, alias: IsNotNullAlias) -> bool {
-        match alias {
-            IsNotNullAlias::NotNull => true,
-            IsNotNullAlias::NotSpaceNull => true,
-        }
+    /// SQLite supports `NOTNULL` as aliases for `IS NOT NULL`
+    /// See: <https://sqlite.org/syntax/expr.html>
+    fn supports_notnull_operator(&self) -> bool {
+        true
     }
 }
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
@@ -35,7 +35,6 @@ use IsOptional::*;
 use crate::ast::helpers::stmt_create_table::{CreateTableBuilder, CreateTableConfiguration};
 use crate::ast::Statement::CreatePolicy;
 use crate::ast::*;
-use crate::dialect::IsNotNullAlias::{NotNull, NotSpaceNull};
 use crate::dialect::*;
 use crate::keywords::{Keyword, ALL_KEYWORDS};
 use crate::tokenizer::*;
@@ -1244,6 +1243,21 @@ impl<'a> Parser<'a> {
 
             expr = self.parse_infix(expr, next_precedence)?;
         }
+
+        // Special case: if expr is an identifier or NULL, accept `expr NOT NULL`
+        // as an alias for `expr IS NOT NULL`.
+        if match &expr {
+            Expr::Identifier(_) if self.parse_keywords(&[Keyword::NOT, Keyword::NULL]) => true,
+            Expr::Value(v)
+                if v.value == Value::Null
+                    && self.parse_keywords(&[Keyword::NOT, Keyword::NULL]) =>
+            {
+                true
+            }
+            _ => false,
+        } {
+            return Ok(Expr::IsNotNull(Box::new(expr)));
+        }
         Ok(expr)
     }
 
@@ -3563,7 +3577,6 @@ impl<'a> Parser<'a> {
                     let negated = self.parse_keyword(Keyword::NOT);
                     let regexp = self.parse_keyword(Keyword::REGEXP);
                     let rlike = self.parse_keyword(Keyword::RLIKE);
-                    let null = self.parse_keyword(Keyword::NULL);
                     if regexp || rlike {
                         Ok(Expr::RLike {
                             negated,
@@ -3573,8 +3586,6 @@ impl<'a> Parser<'a> {
                             ),
                             regexp,
                         })
-                    } else if dialect.supports_is_not_null_alias(NotSpaceNull) && negated && null {
-                        Ok(Expr::IsNotNull(Box::new(expr)))
                     } else if self.parse_keyword(Keyword::IN) {
                         self.parse_in(expr, negated)
                     } else if self.parse_keyword(Keyword::BETWEEN) {
@@ -3612,7 +3623,7 @@ impl<'a> Parser<'a> {
                         self.expected("IN or BETWEEN after NOT", self.peek_token())
                     }
                 }
-                Keyword::NOTNULL if dialect.supports_is_not_null_alias(NotNull) => {
+                Keyword::NOTNULL if dialect.supports_notnull_operator() => {
                     Ok(Expr::IsNotNull(Box::new(expr)))
                 }
                 Keyword::MEMBER => {
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
@@ -32,8 +32,8 @@ use sqlparser::ast::TableFactor::{Pivot, Unpivot};
 use sqlparser::ast::*;
 use sqlparser::dialect::{
     AnsiDialect, BigQueryDialect, ClickHouseDialect, DatabricksDialect, Dialect, DuckDbDialect,
-    GenericDialect, HiveDialect, IsNotNullAlias, MsSqlDialect, MySqlDialect, PostgreSqlDialect,
-    RedshiftSqlDialect, SQLiteDialect, SnowflakeDialect,
+    GenericDialect, HiveDialect, MsSqlDialect, MySqlDialect, PostgreSqlDialect, RedshiftSqlDialect,
+    SQLiteDialect, SnowflakeDialect,
 };
 use sqlparser::keywords::{Keyword, ALL_KEYWORDS};
 use sqlparser::parser::{Parser, ParserError, ParserOptions};
@@ -15975,69 +15975,53 @@ fn parse_create_procedure_with_parameter_modes() {
     }
 }
 
-#[test]
-fn parse_not_null_unsupported() {
-    // Only DuckDB and SQLite support `x NOT NULL` as an expression
-    // All other dialects fail to parse the `NOT NULL` portion
-    let dialects =
-        all_dialects_except(|d| d.supports_is_not_null_alias(IsNotNullAlias::NotSpaceNull));
-    let _ = dialects.expr_parses_to("x NOT NULL", "x");
-}
-
 #[test]
 fn parse_not_null_supported() {
-    // DuckDB and SQLite support `x NOT NULL` as an alias for `x IS NOT NULL`
-    let dialects =
-        all_dialects_where(|d| d.supports_is_not_null_alias(IsNotNullAlias::NotSpaceNull));
-    let _ = dialects.expr_parses_to("x NOT NULL", "x IS NOT NULL");
+    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() {
-    // For dialects which support it, `NOT NULL NOT NULL` should
-    // parse as `(NOT (NULL IS NOT NULL))`
-    let supported_dialects =
-        all_dialects_where(|d| d.supports_is_not_null_alias(IsNotNullAlias::NotSpaceNull));
-    let unsuported_dialects =
-        all_dialects_except(|d| d.supports_is_not_null_alias(IsNotNullAlias::NotSpaceNull));
-
     assert_matches!(
-        supported_dialects.expr_parses_to("NOT NULL NOT NULL", "NOT NULL IS NOT NULL"),
+        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,
             ..
         }
     );
-
-    // for unsupported dialects, parsing should stop at `NOT NULL`
-    unsuported_dialects.expr_parses_to("NOT NULL NOT NULL", "NOT NULL");
 }
 
 #[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_is_not_null_alias(IsNotNullAlias::NotNull));
+    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_is_not_null_alias(IsNotNullAlias::NotNull));
+    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_is_not_null_alias(IsNotNullAlias::NotNull));
-    let unsuported_dialects =
-        all_dialects_except(|d| d.supports_is_not_null_alias(IsNotNullAlias::NotNull));
+    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"),
@@ -16048,5 +16032,5 @@ fn test_notnull_precedence() {
     );
 
     // for unsupported dialects, parsing should stop at `NOT NULL`
-    unsuported_dialects.expr_parses_to("NOT NULL NOTNULL", "NOT NULL");
+    unsupported_dialects.expr_parses_to("NOT NULL NOTNULL", "NOT NULL");
 }
