alamb commented on code in PR #14572:
URL: https://github.com/apache/datafusion/pull/14572#discussion_r1957090671
##########
datafusion/core/tests/dataframe/mod.rs:
##########
@@ -5274,3 +5274,61 @@ async fn register_non_parquet_file() {
"1.json' does not match the expected extension '.parquet'"
);
}
+
+// Test inserting into checking.
+#[tokio::test]
+async fn test_insert_into_checking() -> Result<()> {
+ // Create a new schema with one field called "a" of type Int64, and
setting nullable to false
+ let schema = Arc::new(Schema::new(vec![Field::new("a", DataType::Int64,
false)]));
+
+ let session_ctx = SessionContext::new();
+
+ // Create and register the initial table with the provided schema and data
+ let initial_table = Arc::new(MemTable::try_new(schema.clone(),
vec![vec![]])?);
+ session_ctx.register_table("t", initial_table.clone())?;
+
+ // 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 datatype of the schema of the plan and the schema of the table
should be the same
+
+ // Test case 1:
+ let write_df = session_ctx.sql("values (1, 2), (3, 4)").await.unwrap();
+
+ match write_df
+ .write_table("t", DataFrameWriteOptions::new())
+ .await
+ {
+ Ok(_) => {}
+ Err(e) => {
+ assert_contains!(
+ e.to_string(),
+ "Inserting query must have the same schema length as the
table."
+ );
+ }
+ }
Review Comment:
The same comment applies to the code below as well
##########
datafusion/core/tests/dataframe/mod.rs:
##########
@@ -5274,3 +5274,61 @@ async fn register_non_parquet_file() {
"1.json' does not match the expected extension '.parquet'"
);
}
+
+// Test inserting into checking.
+#[tokio::test]
+async fn test_insert_into_checking() -> Result<()> {
+ // Create a new schema with one field called "a" of type Int64, and
setting nullable to false
+ let schema = Arc::new(Schema::new(vec![Field::new("a", DataType::Int64,
false)]));
+
+ let session_ctx = SessionContext::new();
+
+ // Create and register the initial table with the provided schema and data
+ let initial_table = Arc::new(MemTable::try_new(schema.clone(),
vec![vec![]])?);
+ session_ctx.register_table("t", initial_table.clone())?;
+
+ // There are three cases we need to check
Review Comment:
```suggestion
// There are two cases we need to check
```
##########
datafusion/core/tests/dataframe/mod.rs:
##########
@@ -5274,3 +5274,61 @@ async fn register_non_parquet_file() {
"1.json' does not match the expected extension '.parquet'"
);
}
+
+// Test inserting into checking.
+#[tokio::test]
+async fn test_insert_into_checking() -> Result<()> {
+ // Create a new schema with one field called "a" of type Int64, and
setting nullable to false
+ let schema = Arc::new(Schema::new(vec![Field::new("a", DataType::Int64,
false)]));
+
+ let session_ctx = SessionContext::new();
+
+ // Create and register the initial table with the provided schema and data
+ let initial_table = Arc::new(MemTable::try_new(schema.clone(),
vec![vec![]])?);
+ session_ctx.register_table("t", initial_table.clone())?;
+
+ // 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 datatype of the schema of the plan and the schema of the table
should be the same
+
+ // Test case 1:
+ let write_df = session_ctx.sql("values (1, 2), (3, 4)").await.unwrap();
+
+ match write_df
+ .write_table("t", DataFrameWriteOptions::new())
+ .await
+ {
+ Ok(_) => {}
+ Err(e) => {
+ assert_contains!(
+ e.to_string(),
+ "Inserting query must have the same schema length as the
table."
+ );
+ }
+ }
Review Comment:
I think you can write this much more concisely using
[`unwrap_err`](https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_err)
```suggestion
let e = write_df
.write_table("t", DataFrameWriteOptions::new())
.await
.unwrap_err();
```
##########
datafusion/common/src/dfschema.rs:
##########
@@ -1028,21 +1028,32 @@ impl SchemaExt for Schema {
})
}
- fn logically_equivalent_names_and_types(&self, other: &Self) -> bool {
+ fn logically_equivalent_names_and_types(&self, other: &Self) -> Result<()>
{
if self.fields().len() != other.fields().len() {
- return false;
+ _plan_err!(
+ "Inserting query must have the same schema length as the
table. \
+ Expected table schema length: {}, got: {}",
+ self.fields().len(),
+ other.fields().len()
+ )
+ } else {
+ self.fields()
+ .iter()
+ .zip(other.fields().iter())
+ .try_for_each(|(f1, f2)| {
+ if f1.name() != f2.name() ||
!DFSchema::datatype_is_logically_equal(f1.data_type(), f2.data_type()) {
+ _plan_err!(
+ "Inserting query schema mismatch: Expected table
field '{}' with type {:?}, \
Review Comment:
Perhaps we could update the comments to reflect this
--
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
