fix: Operator::returns_null_on_null() should include string concat (||) (apache#22458)

## Which issue does this PR close?

- Closes apache#22457

## Rationale for this change

`Operator::returns_null_on_null()` incorrectly claimed that the string
concatenation operator (`||`) treats NULL input as the empty string; it
does not. (Note that the `concat()` _function_ has the NULL->empty
string behavior, but the operator does not.)

While we're here, also teach `Operator::negate()` that
`Operator::RegexMatch` and `Operator::RegexNotMatch` are pairs, and the
same for the case-insensitive versions.

## What changes are included in this PR?

* Teach `Operator::returns_null_on_null()` that `||` returns NULL on
NULL inputs
* Teach `Operator::negate()` that `Operator::RegexMatch` and
`Operator::RegexNotMatch` are pairs; same for case-insensitive versions
* Fix typos in comments
* Add tests for new behavior

## Are these changes tested?

Yes; new tests added.

## Are there any user-facing changes?

Yes, but minor: expression simplification, outer join elimination, and
other optimizer can take advantage of this to optimize queries a bit
more effectively.

Author: neilconway

datafusion/expr-common/src/operator.rs

@@ -36,15 +36,15 @@ pub enum Operator {
     Plus,
     /// Subtraction
     Minus,
-    /// Multiplication operator, like `*`
+    /// Multiplication
     Multiply,
-    /// Division operator, like `/`
+    /// Division
     Divide,
-    /// Remainder operator, like `%`
+    /// Remainder
     Modulo,
-    /// Logical AND, like `&&`
+    /// Logical AND
     And,
-    /// Logical OR, like `||`
+    /// Logical OR
     Or,
     /// `IS DISTINCT FROM` (see [`distinct`])
     ///
@@ -80,20 +80,20 @@ pub enum Operator {
     BitwiseShiftRight,
     /// Bitwise left, like `<<`
     BitwiseShiftLeft,
-    /// String concat
+    /// String concatenation, like `||`
     StringConcat,
     /// At arrow, like `@>`.
     ///
     /// Currently only supported to be used with lists:
     /// ```sql
-    /// select [1,3] <@ [1,2,3]
+    /// select [1,2,3] @> [1,3]
     /// ```
     AtArrow,
     /// Arrow at, like `<@`.
     ///
     /// Currently only supported to be used with lists:
     /// ```sql
-    /// select [1,2,3] @> [1,3]
+    /// select [1,3] <@ [1,2,3]
     /// ```
     ArrowAt,
     /// Arrow, like `->`.
@@ -120,7 +120,7 @@ pub enum Operator {
     ///
     /// Not implemented in DataFusion yet.
     IntegerDivide,
-    /// Hash Minis, like `#-`
+    /// Hash Minus, like `#-`
     ///
     /// Not implemented in DataFusion yet.
     HashMinus,
@@ -163,17 +163,17 @@ impl Operator {
             Operator::ILikeMatch => Some(Operator::NotILikeMatch),
             Operator::NotLikeMatch => Some(Operator::LikeMatch),
             Operator::NotILikeMatch => Some(Operator::ILikeMatch),
+            Operator::RegexMatch => Some(Operator::RegexNotMatch),
+            Operator::RegexIMatch => Some(Operator::RegexNotIMatch),
+            Operator::RegexNotMatch => Some(Operator::RegexMatch),
+            Operator::RegexNotIMatch => Some(Operator::RegexIMatch),
             Operator::Plus
             | Operator::Minus
             | Operator::Multiply
             | Operator::Divide
             | Operator::Modulo
             | Operator::And
             | Operator::Or
-            | Operator::RegexMatch
-            | Operator::RegexIMatch
-            | Operator::RegexNotMatch
-            | Operator::RegexNotIMatch
             | Operator::BitwiseAnd
             | Operator::BitwiseOr
             | Operator::BitwiseXor
@@ -377,17 +377,16 @@ impl Operator {
             | Operator::Question
             | Operator::QuestionAnd
             | Operator::QuestionPipe
-            | Operator::Colon => true,
+            | Operator::Colon
+            | Operator::StringConcat => true,
 
             // E.g. `TRUE OR NULL` is `TRUE`
             Operator::Or
             // E.g. `FALSE AND NULL` is `FALSE`
             | Operator::And
             // IS DISTINCT FROM and IS NOT DISTINCT FROM always return a TRUE/FALSE value, never NULL
             | Operator::IsDistinctFrom
-            | Operator::IsNotDistinctFrom
-            // DataFusion string concatenation operator treats NULL as an empty string
-            | Operator::StringConcat => false,
+            | Operator::IsNotDistinctFrom => false,
         }
     }
 

datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs

@@ -2919,6 +2919,21 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_simplify_concat_by_null() {
+        let null = Expr::Literal(ScalarValue::Utf8(None), None);
+        // A || null --> null
+        {
+            let expr = binary_expr(col("c1"), Operator::StringConcat, null.clone());
+            assert_eq!(simplify(expr), null);
+        }
+        // null || A --> null
+        {
+            let expr = binary_expr(null.clone(), Operator::StringConcat, col("c1"));
+            assert_eq!(simplify(expr), null);
+        }
+    }
+
     #[test]
     fn test_simplify_composed_bitwise_and() {
         // ((c2 > 5) & (c1 < 6)) & (c2 > 5) --> (c2 > 5) & (c1 < 6)
@@ -3538,6 +3553,32 @@ mod tests {
         assert_no_change(regex_match(col("c1"), lit("foo|bar|baz|blarg|bozo|etc")));
     }
 
+    #[test]
+    fn test_simplify_not_regex_match() {
+        let pattern = || lit("foo.*");
+
+        // NOT (c1 ~ pattern)  --> c1 !~ pattern
+        assert_eq!(
+            simplify(regex_match(col("c1"), pattern()).not()),
+            regex_not_match(col("c1"), pattern()),
+        );
+        // NOT (c1 !~ pattern) --> c1 ~ pattern
+        assert_eq!(
+            simplify(regex_not_match(col("c1"), pattern()).not()),
+            regex_match(col("c1"), pattern()),
+        );
+        // NOT (c1 ~* pattern)  --> c1 !~* pattern
+        assert_eq!(
+            simplify(regex_imatch(col("c1"), pattern()).not()),
+            regex_not_imatch(col("c1"), pattern()),
+        );
+        // NOT (c1 !~* pattern) --> c1 ~* pattern
+        assert_eq!(
+            simplify(regex_not_imatch(col("c1"), pattern()).not()),
+            regex_imatch(col("c1"), pattern()),
+        );
+    }
+
     #[track_caller]
     fn assert_no_change(expr: Expr) {
         let optimized = simplify(expr.clone());