jonahgao commented on code in PR #14572:
URL: https://github.com/apache/datafusion/pull/14572#discussion_r1950956643


##########
datafusion/common/src/dfschema.rs:
##########
@@ -1028,20 +1028,48 @@ impl SchemaExt for Schema {
             })
     }
 
-    fn logically_equivalent_names_and_types(&self, other: &Self) -> bool {
+    // There are three cases we need to check
+    // 1. The len of the schema of the plan and the schema of the table should 
be the same
+    // 2. The nullable flag of the schema of the plan and the schema of the 
table should be the same
+    // 3. The datatype of the schema of the plan and the schema of the table 
should be the same
+    fn logically_equivalent_names_and_types(&self, other: &Self) -> Result<(), 
String> {
         if self.fields().len() != other.fields().len() {
-            return false;
+            return Err(format!(
+                "Inserting query must have the same schema length as the 
table. \
+            Expected table schema length: {}, got: {}",
+                self.fields().len(),
+                other.fields().len()
+            ));
         }
 
         self.fields()
             .iter()
             .zip(other.fields().iter())
-            .all(|(f1, f2)| {
-                f1.name() == f2.name()
-                    && DFSchema::datatype_is_logically_equal(
+            .try_for_each(|(f1, f2)| {
+                if f1.is_nullable() != f2.is_nullable() {

Review Comment:
   This seems a regression to me 🤔.  Even though the schema of a source is 
nullable, all of its data can be non-nullable, and in such cases, it can still 
be inserted into a non-nullable sink. When inserting, we currently validate 
against the actual data rather than the schema. See 
[check_not_null_constraints](https://github.com/apache/datafusion/blob/2c73fcd47ed7391c7fd34d6c593c6f0c455670d0/datafusion/physical-plan/src/execution_plan.rs#L1003)



-- 
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

Reply via email to