Split physical_eq from base_eq

Author: ggevay

src/expr/src/relation.rs

@@ -1707,7 +1707,7 @@ impl MirRelationExpr {
                 .column_types
                 .iter()
                 .zip_eq(typ.column_types.iter())
-                .all(|(t1, t2)| t1.scalar_type.base_eq(&t2.scalar_type)));
+                .all(|(t1, t2)| t1.scalar_type.physical_eq(&t2.scalar_type)));
         }
         let mut typ = typ.unwrap_or_else(|| self.typ());
         typ.keys = vec![vec![]];

src/expr/src/scalar/func.rs

@@ -33,8 +33,8 @@ use mz_ore::fmt::FormatBuffer;
 use mz_ore::lex::LexBuf;
 use mz_ore::option::OptionExt;
 use mz_ore::result::ResultExt;
-use mz_ore::{soft_assert_eq_or_log, soft_assert_or_log};
 use mz_ore::str::StrExt;
+use mz_ore::{soft_assert_eq_or_log, soft_assert_or_log};
 use mz_pgrepr::Type;
 use mz_pgtz::timezone::{Timezone, TimezoneSpec};
 use mz_proto::chrono::any_naive_datetime;
@@ -8034,7 +8034,9 @@ impl VariadicFunc {
             .nullable(true),
             ArrayCreate { elem_type } => {
                 soft_assert_or_log!(
-                    input_types.iter().all(|t| t.scalar_type.base_eq(elem_type)),
+                    input_types
+                        .iter()
+                        .all(|t| t.scalar_type.physical_eq(elem_type)),
                     "Args to ArrayCreate should have types that are compatible with the elem_type"
                 );
                 match elem_type {

src/repr/src/relation.rs

@@ -69,7 +69,7 @@ impl ColumnType {
                     nullable: self.nullable || other.nullable,
                 })
             }
-            (scalar_type, other_scalar_type) if scalar_type.base_eq(&other_scalar_type) => {
+            (scalar_type, other_scalar_type) if scalar_type.physical_eq(&other_scalar_type) => {
                 Ok(ColumnType {
                     scalar_type: scalar_type.without_modifiers(),
                     nullable: self.nullable || other.nullable,

src/repr/src/scalar.rs

@@ -2908,12 +2908,25 @@ impl ScalarType {
         self.eq_inner(other, false)
     }
 
-    // Determines equality among scalar types that ignores any custom OIDs or
-    // embedded values.
+    /// Determines equality among scalar types that ignores any custom OIDs or
+    /// embedded values.
     pub fn structural_eq(&self, other: &ScalarType) -> bool {
         self.eq_inner(other, true)
     }
 
+    /// //////////// todo: comment
+    pub fn physical_eq(&self, other: &ScalarType) -> bool {
+        use ScalarType::*;
+        if self.base_eq(other) {
+            return true;
+        }
+        match (self, other) {
+            (String, VarChar { max_length: None }) => true,
+            (VarChar { max_length: None }, String) => true,
+            _ => false,
+        }
+    }
+
     pub fn eq_inner(&self, other: &ScalarType, structure_only: bool) -> bool {
         use ScalarType::*;
         match (self, other) {
@@ -2961,9 +2974,6 @@ impl ScalarType {
                                 && a.1.scalar_type.eq_inner(&b.1.scalar_type, structure_only)
                         })
             }
-            //////////////// todo: comment
-            (String, VarChar { max_length: None }) => true,
-            (VarChar { max_length: None }, String) => true,
             (s, o) => ScalarBaseType::from(s) == ScalarBaseType::from(o),
         }
     }

src/transform/src/column_knowledge.rs

@@ -635,7 +635,7 @@ impl DatumKnowledge {
                 unreachable!();
             };
 
-            if !s_typ.base_eq(o_typ) {
+            if !s_typ.physical_eq(o_typ) {
                 ::tracing::error!("Undefined join of non-equal base types {s_typ:?} != {o_typ:?}");
                 *self = Self::top();
             } else if s_val != o_val {
@@ -735,7 +735,7 @@ impl DatumKnowledge {
                 unreachable!();
             };
 
-            if !s_typ.base_eq(o_typ) {
+            if !s_typ.physical_eq(o_typ) {
                 soft_panic_or_log!("Undefined meet of non-equal base types {s_typ:?} != {o_typ:?}");
                 *self = Self::top(); // this really should be Nothing
             } else if s_val != o_val {

src/transform/src/normalize_lets.rs

@@ -318,7 +318,7 @@ mod support {
                         .column_types
                         .iter()
                         .zip(typ.column_types.iter())
-                        .all(|(t1, t2)| t1.scalar_type.base_eq(&t2.scalar_type))
+                        .all(|(t1, t2)| t1.scalar_type.physical_eq(&t2.scalar_type))
                     {
                         Err(crate::TransformError::Internal(format!(
                             "scalar types do not match: {:?} v {:?}",

src/transform/src/typecheck.rs

@@ -397,8 +397,7 @@ pub fn scalar_subtype_difference(sub: &ScalarType, sup: &ScalarType) -> Vec<Colu
             }
         }
         (_, _) => {
-            // TODO(mgree) confirm that we don't want to allow numeric subtyping
-            if ScalarBaseType::from(sub) != ScalarBaseType::from(sup) {
+            if !sub.physical_eq(sup) {
                 diffs.push(ColumnTypeDifference::NotSubtype {
                     sub: sub.clone(),
                     sup: sup.clone(),

test/sqllogictest/github-5536.slt

@@ -44,14 +44,14 @@ Explained Query:
               Filter (#0 <= #0) AND (#0 >= #0)
                 ReadStorage materialize.public.t5
           ArrangeBy keys=[[]]
-            Filter (text_to_varchar(boolean_to_text(like["0.31161855206970124"](padchar(#1)))) = char_to_text(#1))
+            Filter (#1 = text_to_char(text_to_varchar(boolean_to_text(like["0.31161855206970124"](padchar(#1))))))
               ReadStorage materialize.public.t3
           ArrangeBy keys=[[]]
             ReadStorage materialize.public.t5
 
 Source materialize.public.t0
 Source materialize.public.t3
-  filter=((text_to_varchar(boolean_to_text(like["0.31161855206970124"](padchar(#1)))) = char_to_text(#1)))
+  filter=((#1 = text_to_char(text_to_varchar(boolean_to_text(like["0.31161855206970124"](padchar(#1)))))))
 Source materialize.public.t5
 
 Target cluster: quickstart

test/sqllogictest/type-promotion.slt

@@ -317,7 +317,7 @@ query T multiline
 EXPLAIN RAW PLAN FOR
     SELECT 'a'::"char" < 'a'::varchar;
 ----
-Map (("char"_to_text(text_to_"char"("a")) < text_to_varchar("a")))
+Map (("char"_to_text(text_to_"char"("a")) < varchar_to_text(text_to_varchar("a"))))
   Constant
     - ()
 
@@ -343,7 +343,7 @@ query T multiline
 EXPLAIN RAW PLAN FOR
     SELECT 'a'::char < 'a'::varchar;
 ----
-Map ((char_to_text(text_to_char(text_to_char("a"))) < text_to_varchar("a")))
+Map ((text_to_char(text_to_char("a")) < text_to_char(text_to_varchar("a"))))
   Constant
     - ()
 
@@ -369,7 +369,7 @@ query T multiline
 EXPLAIN RAW PLAN FOR
     SELECT 'a'::varchar < 'a'::text;
 ----
-Map ((text_to_varchar("a") < "a"))
+Map ((varchar_to_text(text_to_varchar("a")) < "a"))
   Constant
     - ()