You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/10/22 14:58:23 UTC
[PR] Improve documentation and examples for SchemaAdapterFactory and related classes [datafusion]
alamb opened a new pull request, #13063:
URL: https://github.com/apache/datafusion/pull/13063
## Which issue does this PR close?
N/A
## Rationale for this change
When discussing schema adapting with @ccciudatu's on https://github.com/datafusion-contrib/datafusion-table-providers/pull/147#issuecomment-2424745760 I realized there were several levels of indirection that made it non trivial to figure out to use these classes to adapt schemas and how the different factories/methods are related to each other
## What changes are included in this PR?
1. Add examples
2. Add docs
## Are these changes tested?
CI
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1815673753
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -79,11 +87,17 @@ pub trait SchemaAdapter: Send + Sync {
) -> datafusion_common::Result<(Arc<dyn SchemaMapper>, Vec<usize>)>;
}
-/// Maps, by casting or reordering columns from the file schema to the table
-/// schema.
+/// Maps, columns from the file schema to the table schema.
+///
+/// See [`DefaultSchemaAdapterFactory`] for more details and examples.
pub trait SchemaMapper: Debug + Send + Sync {
- /// Adapts a `RecordBatch` to match the `table_schema` using the stored
- /// mapping and conversions.
+ /// Adapts a `RecordBatch` to match the `table_schema`
+ ///
+ /// # Errors:
+ ///
+ /// * If a column in the table schema is non-nullable but is not present
+ /// in the file schema (i.e. it is missing), this method tries to fill it
+ /// with nulls resulting in a schema error
Review Comment:
It is a good call -- tried to clarify in d966fd112
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "itsjunetime (via GitHub)" <gi...@apache.org>.
itsjunetime commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1811219153
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
+///
+/// # Illustration of Schema Mapping
+///
+/// ```text
+/// ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
+/// ┌───────┐ ┌───────┐ │ ┌───────┐ ┌───────┐ ┌───────┐ │
+/// ││ 1.0 │ │ "foo" │ ││ NULL │ │ "foo" │ │ "1.0" │
+/// ├───────┤ ├───────┤ │ Schema mapping ├───────┤ ├───────┤ ├───────┤ │
+/// ││ 2.0 │ │ "bar" │ ││ NULL │ │ "bar" │ │ "2.0" │
+/// └───────┘ └───────┘ │────────────────▶ └───────┘ └───────┘ └───────┘ │
+/// │ │
+/// column "c" column "b"│ column "a" column "b" column "c"│
+/// │ Float64 Utf8 │ Int32 Utf8 Utf8
+/// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
Review Comment:
As just a personal question: What do you use to create these sort of diagrams? Do you make them by hand or is there a program that allows you to easily draw these text-based squares and arrows and such?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for SchemaAdapterFactory and related classes [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1810899533
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
Review Comment:
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -32,11 +32,19 @@ use std::sync::Arc;
///
/// This interface provides a way to implement custom schema adaptation logic
/// for ParquetExec (for example, to fill missing columns with default value
-/// other than null)
+/// other than null).
+///
+/// Most users should use [`DefaultSchemaAdapterFactory`]. See that struct for
Review Comment:
The idea was to route the readers to `DefaultSchemaAdapterFactory` if they want to use this code outside the context of DataFusion
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1811275590
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
Review Comment:
It errors -- I will add a test / improve the docs
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1810942936
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
Review Comment:
What happens if the field is not nullable? I've looked through the code but couldn't find an explicit check for this, so does this error or does this produce invalid record batches? :thinking:
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1812168705
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -79,11 +87,17 @@ pub trait SchemaAdapter: Send + Sync {
) -> datafusion_common::Result<(Arc<dyn SchemaMapper>, Vec<usize>)>;
}
-/// Maps, by casting or reordering columns from the file schema to the table
-/// schema.
+/// Maps, columns from the file schema to the table schema.
+///
+/// See [`DefaultSchemaAdapterFactory`] for more details and examples.
pub trait SchemaMapper: Debug + Send + Sync {
- /// Adapts a `RecordBatch` to match the `table_schema` using the stored
- /// mapping and conversions.
+ /// Adapts a `RecordBatch` to match the `table_schema`
+ ///
+ /// # Errors:
+ ///
+ /// * If a column in the table schema is non-nullable but is not present
+ /// in the file schema (i.e. it is missing), this method tries to fill it
+ /// with nulls resulting in a schema error
Review Comment:
I think this is specific to the `DefaultSchemaAdapterFactory` and should be moved to there. You could imagine that someone writes a mapper that fills in missing columns with placeholder values or intra-/extrapolates the data.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "itsjunetime (via GitHub)" <gi...@apache.org>.
itsjunetime commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1811221218
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
+///
+/// # Illustration of Schema Mapping
+///
+/// ```text
+/// ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
+/// ┌───────┐ ┌───────┐ │ ┌───────┐ ┌───────┐ ┌───────┐ │
+/// ││ 1.0 │ │ "foo" │ ││ NULL │ │ "foo" │ │ "1.0" │
+/// ├───────┤ ├───────┤ │ Schema mapping ├───────┤ ├───────┤ ├───────┤ │
+/// ││ 2.0 │ │ "bar" │ ││ NULL │ │ "bar" │ │ "2.0" │
+/// └───────┘ └───────┘ │────────────────▶ └───────┘ └───────┘ └───────┘ │
+/// │ │
+/// column "c" column "b"│ column "a" column "b" column "c"│
+/// │ Float64 Utf8 │ Int32 Utf8 Utf8
+/// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
+/// Input Record Batch Output Record Batch
+///
+/// Schema { Schema {
+/// "c": Float64, "a": Int32,
+/// "b": Utf8, "b": Utf8,
+/// } "c": Utf8,
+/// }
+/// ```
+/// # Example of using the `DefaultSchemaAdapterFactory` to map [`RecordBatch`]s
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow::datatypes::{DataType, Field, Schema};
+/// # use datafusion::datasource::schema_adapter::{DefaultSchemaAdapterFactory, SchemaAdapterFactory};
+/// # use datafusion_common::record_batch;
+/// // Table has fields "a" and "b"
+/// let table_schema = Schema::new(vec![
+/// Field::new("a", DataType::Int32, true),
+/// Field::new("b", DataType::Utf8, true),
+/// Field::new("c", DataType::Utf8, true),
+/// ]);
+///
+/// // The file provides only fields "b" and "c" that oder
Review Comment:
```suggestion
/// // The file provides only fields "b" and "c" that order
```
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
+///
+/// # Illustration of Schema Mapping
+///
+/// ```text
+/// ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
+/// ┌───────┐ ┌───────┐ │ ┌───────┐ ┌───────┐ ┌───────┐ │
+/// ││ 1.0 │ │ "foo" │ ││ NULL │ │ "foo" │ │ "1.0" │
+/// ├───────┤ ├───────┤ │ Schema mapping ├───────┤ ├───────┤ ├───────┤ │
+/// ││ 2.0 │ │ "bar" │ ││ NULL │ │ "bar" │ │ "2.0" │
+/// └───────┘ └───────┘ │────────────────▶ └───────┘ └───────┘ └───────┘ │
+/// │ │
+/// column "c" column "b"│ column "a" column "b" column "c"│
+/// │ Float64 Utf8 │ Int32 Utf8 Utf8
+/// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
+/// Input Record Batch Output Record Batch
+///
+/// Schema { Schema {
+/// "c": Float64, "a": Int32,
+/// "b": Utf8, "b": Utf8,
+/// } "c": Utf8,
+/// }
+/// ```
+/// # Example of using the `DefaultSchemaAdapterFactory` to map [`RecordBatch`]s
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow::datatypes::{DataType, Field, Schema};
+/// # use datafusion::datasource::schema_adapter::{DefaultSchemaAdapterFactory, SchemaAdapterFactory};
+/// # use datafusion_common::record_batch;
+/// // Table has fields "a" and "b"
+/// let table_schema = Schema::new(vec![
+/// Field::new("a", DataType::Int32, true),
+/// Field::new("b", DataType::Utf8, true),
+/// Field::new("c", DataType::Utf8, true),
+/// ]);
+///
+/// // The file provides only fields "b" and "c" that oder
+/// let projected_table_schema = table_schema.project(&[2, 1]).unwrap();
+///
+/// // create an adapter for the table schema and file schema
+/// let adapter = DefaultSchemaAdapterFactory.create(
+/// Arc::new(table_schema),
+/// Arc::new(projected_table_schema)
Review Comment:
```suggestion
/// Arc::new(projected_table_schema),
/// Arc::new(table_schema)
```
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
+///
+/// # Illustration of Schema Mapping
+///
+/// ```text
+/// ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
+/// ┌───────┐ ┌───────┐ │ ┌───────┐ ┌───────┐ ┌───────┐ │
+/// ││ 1.0 │ │ "foo" │ ││ NULL │ │ "foo" │ │ "1.0" │
+/// ├───────┤ ├───────┤ │ Schema mapping ├───────┤ ├───────┤ ├───────┤ │
+/// ││ 2.0 │ │ "bar" │ ││ NULL │ │ "bar" │ │ "2.0" │
+/// └───────┘ └───────┘ │────────────────▶ └───────┘ └───────┘ └───────┘ │
+/// │ │
+/// column "c" column "b"│ column "a" column "b" column "c"│
+/// │ Float64 Utf8 │ Int32 Utf8 Utf8
+/// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
+/// Input Record Batch Output Record Batch
+///
+/// Schema { Schema {
+/// "c": Float64, "a": Int32,
+/// "b": Utf8, "b": Utf8,
+/// } "c": Utf8,
+/// }
+/// ```
+/// # Example of using the `DefaultSchemaAdapterFactory` to map [`RecordBatch`]s
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow::datatypes::{DataType, Field, Schema};
+/// # use datafusion::datasource::schema_adapter::{DefaultSchemaAdapterFactory, SchemaAdapterFactory};
+/// # use datafusion_common::record_batch;
+/// // Table has fields "a" and "b"
+/// let table_schema = Schema::new(vec![
+/// Field::new("a", DataType::Int32, true),
+/// Field::new("b", DataType::Utf8, true),
+/// Field::new("c", DataType::Utf8, true),
+/// ]);
+///
+/// // The file provides only fields "b" and "c" that oder
+/// let projected_table_schema = table_schema.project(&[2, 1]).unwrap();
Review Comment:
This looks like it's projecting fields `c` and `b`, in that order. Is the above comment wrong about which order they're being projected or am I misunderstanding what this is doing?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1810938685
##########
datafusion/common/src/test_util.rs:
##########
@@ -347,7 +347,7 @@ macro_rules! record_batch {
let batch = arrow_array::RecordBatch::try_new(
schema,
vec![$(
- create_array!($type, $values),
+ $crate::create_array!($type, $values),
Review Comment:
Without this the macro errors if `create_array` is not also `use`d
##########
datafusion/core/tests/macro_hygiene/mod.rs:
##########
@@ -37,3 +37,13 @@ mod plan_datafusion_err {
plan_datafusion_err!("foo");
}
}
+
+mod record_batch {
+ // NO other imports!
+ use datafusion_common::record_batch;
+
+ #[test]
+ fn test_macro() {
+ record_batch!(("column_name", Int32, vec![1, 2, 3])).unwrap();
Review Comment:
test that we can use `record_batch` without any other imports
fyi @timsaucer
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1811281197
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
+///
+/// # Illustration of Schema Mapping
+///
+/// ```text
+/// ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
+/// ┌───────┐ ┌───────┐ │ ┌───────┐ ┌───────┐ ┌───────┐ │
+/// ││ 1.0 │ │ "foo" │ ││ NULL │ │ "foo" │ │ "1.0" │
+/// ├───────┤ ├───────┤ │ Schema mapping ├───────┤ ├───────┤ ├───────┤ │
+/// ││ 2.0 │ │ "bar" │ ││ NULL │ │ "bar" │ │ "2.0" │
+/// └───────┘ └───────┘ │────────────────▶ └───────┘ └───────┘ └───────┘ │
+/// │ │
+/// column "c" column "b"│ column "a" column "b" column "c"│
+/// │ Float64 Utf8 │ Int32 Utf8 Utf8
+/// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
+/// Input Record Batch Output Record Batch
+///
+/// Schema { Schema {
+/// "c": Float64, "a": Int32,
+/// "b": Utf8, "b": Utf8,
+/// } "c": Utf8,
+/// }
+/// ```
+/// # Example of using the `DefaultSchemaAdapterFactory` to map [`RecordBatch`]s
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow::datatypes::{DataType, Field, Schema};
+/// # use datafusion::datasource::schema_adapter::{DefaultSchemaAdapterFactory, SchemaAdapterFactory};
+/// # use datafusion_common::record_batch;
+/// // Table has fields "a" and "b"
+/// let table_schema = Schema::new(vec![
+/// Field::new("a", DataType::Int32, true),
+/// Field::new("b", DataType::Utf8, true),
+/// Field::new("c", DataType::Utf8, true),
+/// ]);
+///
+/// // The file provides only fields "b" and "c" that oder
+/// let projected_table_schema = table_schema.project(&[2, 1]).unwrap();
+///
+/// // create an adapter for the table schema and file schema
+/// let adapter = DefaultSchemaAdapterFactory.create(
+/// Arc::new(table_schema),
+/// Arc::new(projected_table_schema)
Review Comment:
wow -- you are totally right. 🤦
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for SchemaAdapterFactory and related classes [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#issuecomment-2429526463
FYI @itsjunetime as you also previously worked with this code
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#issuecomment-2437796666
> needs a clippy fix, but otherwise OK 👍
Thank you for sticking with this and helping @crepererum -- much appreciated
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1811255682
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
+///
+/// # Illustration of Schema Mapping
+///
+/// ```text
+/// ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
+/// ┌───────┐ ┌───────┐ │ ┌───────┐ ┌───────┐ ┌───────┐ │
+/// ││ 1.0 │ │ "foo" │ ││ NULL │ │ "foo" │ │ "1.0" │
+/// ├───────┤ ├───────┤ │ Schema mapping ├───────┤ ├───────┤ ├───────┤ │
+/// ││ 2.0 │ │ "bar" │ ││ NULL │ │ "bar" │ │ "2.0" │
+/// └───────┘ └───────┘ │────────────────▶ └───────┘ └───────┘ └───────┘ │
+/// │ │
+/// column "c" column "b"│ column "a" column "b" column "c"│
+/// │ Float64 Utf8 │ Int32 Utf8 Utf8
+/// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
Review Comment:
I use https://monodraw.helftone.com/ (though I hear https://asciiflow.com is pretty good too)
I feel like I should get a commission for all the times I recommend Monodraw 😆
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#issuecomment-2430094697
> Am I confused about the doc test being wrong? Or should we set up doc tests to run with CI?
The doctest passes for me locally
```shell
(venv) andrewlamb@Andrews-MacBook-Pro-2:~/Software/datafusion$ cargo test --doc -p datafusion -- schema_adapter
...
running 1 test
test datafusion/core/src/datasource/schema_adapter.rs - datasource::schema_adapter::DefaultSchemaAdapterFactory (line 144) ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 97 filtered out; finished in 1.23s
```
I am pretty sure that the CI runs the doc tests as well. Here is this newly added test running in this PR:
https://github.com/apache/datafusion/actions/runs/11463343177/job/31897653058?pr=13063#step:5:921
However, your catch on the doc test is excellent and I will fix
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1811312764
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
Review Comment:
In [b065efe](https://github.com/apache/datafusion/pull/13063/commits/b065efe0456ed88bb7c89aff293acd71eef9cef4)
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #13063:
URL: https://github.com/apache/datafusion/pull/13063#discussion_r1811299207
##########
datafusion/core/src/datasource/schema_adapter.rs:
##########
@@ -99,8 +107,86 @@ pub trait SchemaMapper: Debug + Send + Sync {
) -> datafusion_common::Result<RecordBatch>;
}
-/// Implementation of [`SchemaAdapterFactory`] that maps columns by name
-/// and casts columns to the expected type.
+/// Default [`SchemaAdapterFactory`] for mapping schemas.
+///
+/// This can be used to adapt file-level record batches to a table schema and
+/// implement schema evolution.
+///
+/// Given an input file schema and a table schema, this factor can make
+/// [`SchemaMapper`]s that:
+///
+/// 1. Reorder columns
+/// 2. Cast columns to the correct type
+/// 3. Fill missing columns with nulls
+///
+/// # Illustration of Schema Mapping
+///
+/// ```text
+/// ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
+/// ┌───────┐ ┌───────┐ │ ┌───────┐ ┌───────┐ ┌───────┐ │
+/// ││ 1.0 │ │ "foo" │ ││ NULL │ │ "foo" │ │ "1.0" │
+/// ├───────┤ ├───────┤ │ Schema mapping ├───────┤ ├───────┤ ├───────┤ │
+/// ││ 2.0 │ │ "bar" │ ││ NULL │ │ "bar" │ │ "2.0" │
+/// └───────┘ └───────┘ │────────────────▶ └───────┘ └───────┘ └───────┘ │
+/// │ │
+/// column "c" column "b"│ column "a" column "b" column "c"│
+/// │ Float64 Utf8 │ Int32 Utf8 Utf8
+/// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
+/// Input Record Batch Output Record Batch
+///
+/// Schema { Schema {
+/// "c": Float64, "a": Int32,
+/// "b": Utf8, "b": Utf8,
+/// } "c": Utf8,
+/// }
+/// ```
+/// # Example of using the `DefaultSchemaAdapterFactory` to map [`RecordBatch`]s
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow::datatypes::{DataType, Field, Schema};
+/// # use datafusion::datasource::schema_adapter::{DefaultSchemaAdapterFactory, SchemaAdapterFactory};
+/// # use datafusion_common::record_batch;
+/// // Table has fields "a" and "b"
+/// let table_schema = Schema::new(vec![
+/// Field::new("a", DataType::Int32, true),
+/// Field::new("b", DataType::Utf8, true),
+/// Field::new("c", DataType::Utf8, true),
+/// ]);
+///
+/// // The file provides only fields "b" and "c" that oder
+/// let projected_table_schema = table_schema.project(&[2, 1]).unwrap();
+///
+/// // create an adapter for the table schema and file schema
+/// let adapter = DefaultSchemaAdapterFactory.create(
+/// Arc::new(table_schema),
+/// Arc::new(projected_table_schema)
Review Comment:
I don't really know how the previous version worked, but basically this stemmed from my misunderstanding of how this code should be used. I have updated now with more documentation
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve documentation and examples for `SchemaAdapterFactory`, make `record_batch` "hygenic" [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #13063:
URL: https://github.com/apache/datafusion/pull/13063
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org