You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2025/07/15 17:19:48 UTC
[PR] add fallback and docs [datafusion]
adriangb opened a new pull request, #16791:
URL: https://github.com/apache/datafusion/pull/16791
https://github.com/apache/datafusion/issues/16235#issuecomment-3067125182
This essentially partially reverts https://github.com/apache/datafusion/pull/16461 by keeping backward compatibility with the existing SchemaAdapter.
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2210136013
##########
docs/source/library-user-guide/upgrading.md:
##########
@@ -120,6 +120,17 @@ SET datafusion.execution.spill_compression = 'zstd';
For more details about this configuration option, including performance trade-offs between different compression codecs, see the [Configuration Settings](../user-guide/configs.md) documentation.
+### Custom `SchemaAdapterFactory` will no longer be used for predicate pushdown
+
+We are moving away from converting data (using `SchemaAdapter`) to converting the expressions themselves (which is more efficient and flexible).
Review Comment:
I think we should also link to a ticket that describes the plan and backstory to move away from SchemaAdpater to rewriting the expressions. This both provides an avenue for feedback as well as additional information for people upgrading
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -106,6 +106,8 @@ pub(crate) struct DatafusionArrowPredicate {
rows_matched: metrics::Count,
/// how long was spent evaluating this predicate
time: metrics::Time,
+ /// used to perform type coercion while filtering rows
Review Comment:
I think it is a bit unclear how the schema mapper and expression rewriter work together -- I think it is the case that the schema is mapped first and then the simplified physical expression is evaluated against the mapped schema rather than the file schema
Maybe we can add a comment explaining how this works
##########
docs/source/library-user-guide/upgrading.md:
##########
@@ -120,6 +120,17 @@ SET datafusion.execution.spill_compression = 'zstd';
For more details about this configuration option, including performance trade-offs between different compression codecs, see the [Configuration Settings](../user-guide/configs.md) documentation.
+### Custom `SchemaAdapterFactory` will no longer be used for predicate pushdown
+
+We are moving away from converting data (using `SchemaAdapter`) to converting the expressions themselves (which is more efficient and flexible).
+The first place this change has taken place is in predicate pushdown for Parquet.
+By default if you do not use a custom `SchemaAdapterFactory` we will use expression conversion instead.
+If you do set a custom `SchemaAdapterFactory` we will continue to use it but emit a warning about that code path being deprecated.
+
+To resolve this you need to implement a custom `PhysicalExprAdapterFactory` and use that instead of a `SchemaAdapterFactory`.
+See the [default values](https://github.com/apache/datafusion/blob/main/datafusion-examples/examples/default_column_values.rs) for an example of how to do this.
+Opting into the new APIs will set you up for future changes since we plan to expand use of `PhysicalExprAdapterFactory` to other areas of DataFusion.
Review Comment:
A link to some description of the future plan would be super helpful here
##########
datafusion/datasource-parquet/src/opener.rs:
##########
@@ -1095,4 +1124,167 @@ mod test {
assert_eq!(num_batches, 0);
assert_eq!(num_rows, 0);
}
+
+ fn get_value(metrics: &MetricsSet, metric_name: &str) -> usize {
+ match metrics.sum_by_name(metric_name) {
+ Some(v) => v.as_usize(),
+ _ => {
+ panic!(
+ "Expected metric not found. Looking for '{metric_name}' in\n\n{metrics:#?}"
+ );
+ }
+ }
+ }
+
+ #[tokio::test]
+ async fn test_custom_schema_adapter_no_rewriter() {
Review Comment:
If possible, I think this test should be of the "end to end" variety (in `core_integration`) that shows how these APIs interact to rewrite predicates / schemas correctly. It is not super clear to me how these low level APIs would be used by users and thus not sure if this test covers the cases correctly
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -140,6 +143,8 @@ impl ArrowPredicate for DatafusionArrowPredicate {
}
fn evaluate(&mut self, batch: RecordBatch) -> ArrowResult<BooleanArray> {
+ let batch = self.schema_mapper.map_batch(batch)?;
Review Comment:
Applying the schema mapper first means the predicate is applied on batches that have been mapped to the table schema, but wasn't the predicate rewritten to be in terms of the file schema?
##########
docs/source/library-user-guide/upgrading.md:
##########
@@ -120,6 +120,17 @@ SET datafusion.execution.spill_compression = 'zstd';
For more details about this configuration option, including performance trade-offs between different compression codecs, see the [Configuration Settings](../user-guide/configs.md) documentation.
+### Custom `SchemaAdapterFactory` will no longer be used for predicate pushdown
Review Comment:
Is the plan to avoid using Schema Adapters for all schema conversion, or just predicate pushdown?
```suggestion
### Deprecating `SchemaAdapterFactory` and `SchemaAdapter`
```
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2211616561
##########
datafusion/core/tests/parquet/schema_adapter.rs:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::sync::Arc;
+
+use arrow::array::{record_batch, RecordBatch};
+use arrow_schema::{DataType, Field, Schema};
+use bytes::{BufMut, BytesMut};
+use datafusion::assert_batches_eq;
+use datafusion::datasource::listing::{ListingTable, ListingTableConfig};
+use datafusion::prelude::SessionContext;
+use datafusion_datasource::schema_adapter::DefaultSchemaAdapterFactory;
+use datafusion_datasource::ListingTableUrl;
+use datafusion_execution::object_store::ObjectStoreUrl;
+use datafusion_physical_expr::schema_rewriter::DefaultPhysicalExprAdapterFactory;
+use object_store::{memory::InMemory, path::Path, ObjectStore};
+use parquet::arrow::ArrowWriter;
+
+async fn write_parquet(batch: RecordBatch, store: Arc<dyn ObjectStore>, path: &str) {
+ let mut out = BytesMut::new().writer();
+ {
+ let mut writer = ArrowWriter::try_new(&mut out, batch.schema(), None).unwrap();
+ writer.write(&batch).unwrap();
+ writer.finish().unwrap();
+ }
+ let data = out.into_inner().freeze();
+ store.put(&Path::from(path), data.into()).await.unwrap();
+}
+
+#[tokio::test]
+async fn single_file() {
+ let batch =
+ record_batch!(("extra", Int64, [1, 2, 3]), ("c1", Int32, [1, 2, 3])).unwrap();
+
+ let store = Arc::new(InMemory::new()) as Arc<dyn ObjectStore>;
+ let store_url = ObjectStoreUrl::parse("memory://").unwrap();
+ let path = "test.parquet";
+ write_parquet(batch, store.clone(), path).await;
+
+ let table_schema = Arc::new(Schema::new(vec![
+ Field::new("c1", DataType::Int64, false),
+ Field::new("c2", DataType::Utf8, true),
+ ]));
+
+ let ctx = SessionContext::new();
+ ctx.register_object_store(store_url.as_ref(), Arc::clone(&store));
+
+ let listing_table_config =
+ ListingTableConfig::new(ListingTableUrl::parse("memory:///").unwrap())
+ .infer_options(&ctx.state())
+ .await
+ .unwrap()
+ .with_schema(table_schema.clone())
+ .with_schema_adapter_factory(Arc::new(DefaultSchemaAdapterFactory))
+ .with_physical_expr_adapter_factory(Arc::new(
Review Comment:
https://github.com/apache/datafusion/pull/16791/commits/a846492631c25c1710b1e0dfb9c79186ce09ae7c#diff-eba504ed30666e1eac14df009713df33d8668156a3fac7b6b7679d9b9cddcb04
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2210213790
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -106,6 +106,8 @@ pub(crate) struct DatafusionArrowPredicate {
rows_matched: metrics::Count,
/// how long was spent evaluating this predicate
time: metrics::Time,
+ /// used to perform type coercion while filtering rows
Review Comment:
> I think it is a bit unclear how the schema mapper and expression rewriter work together
If you have an expression adapter you map the expression and the expression is now evaluated against the physical file schema. So there is no longer a need / point of having a SchemaAdapter. It will still be there but it becomes a no-op because it's adapting between identical schemas.
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2211267704
##########
docs/source/library-user-guide/upgrading.md:
##########
@@ -120,6 +120,17 @@ SET datafusion.execution.spill_compression = 'zstd';
For more details about this configuration option, including performance trade-offs between different compression codecs, see the [Configuration Settings](../user-guide/configs.md) documentation.
+### Custom `SchemaAdapterFactory` will no longer be used for predicate pushdown
+
+We are moving away from converting data (using `SchemaAdapter`) to converting the expressions themselves (which is more efficient and flexible).
Review Comment:
```suggestion
We are moving away from converting data (using `SchemaAdapter`) to converting the expressions themselves (which is more efficient and flexible).
See [issue #16800](https://github.com/apache/datafusion/issues/16800) for more information
```
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#issuecomment-3078342870
> In particular I am not sure about the intended behavior when they are both present
If you have an expression adapter you map the expression and the expression is now evaluated against the physical file schema. So there is no longer a need / point of having a SchemaAdapter. It will still be there but it becomes a no-op because it's adapting between identical schemas.
I'm sorry I did not give more detail in the PR description. The idea is that there are one of 4 scenarios that users will fall into:
1. Custom SchemaAdapter, no custom PhysicalExprAdapter -> use SchemaAdapter / old code path
2. Default SchemaAdapter, no custom PhysicalExprAdapter -> use default PhysicalExprAdapter
3. Custom SchemaAdapter, custom PhysicalExprAdapter -> use the custom PhysicalExprAdapter (making the SchemaAdapter a no-op for predicate pushdown).
4. Default SchemaAdapter, custom PhysicalExprAdapter -> use custom PhysicalExprAdapter.
This makes it completely backwards compatible: anyone using a custom SchemaAdapter has to make no code changes to continue using it. If they want to opt into the new mechanism they can either stop setting a custom SchemaAdapter or also set a custom PhysicalExprAdapter.
A SchemaAdapter, custom or the default, is still used for projections.
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2211108359
##########
datafusion/datasource-parquet/src/opener.rs:
##########
@@ -1095,4 +1124,167 @@ mod test {
assert_eq!(num_batches, 0);
assert_eq!(num_rows, 0);
}
+
+ fn get_value(metrics: &MetricsSet, metric_name: &str) -> usize {
+ match metrics.sum_by_name(metric_name) {
+ Some(v) => v.as_usize(),
+ _ => {
+ panic!(
+ "Expected metric not found. Looking for '{metric_name}' in\n\n{metrics:#?}"
+ );
+ }
+ }
+ }
+
+ #[tokio::test]
+ async fn test_custom_schema_adapter_no_rewriter() {
Review Comment:
I added a test in https://github.com/apache/datafusion/pull/16791/files#diff-eba504ed30666e1eac14df009713df33d8668156a3fac7b6b7679d9b9cddcb04
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2211280296
##########
datafusion/datasource-parquet/src/source.rs:
##########
@@ -468,10 +468,50 @@ impl FileSource for ParquetSource {
let projection = base_config
.file_column_projection_indices()
.unwrap_or_else(|| (0..base_config.file_schema.fields().len()).collect());
- let schema_adapter_factory = self
- .schema_adapter_factory
- .clone()
- .unwrap_or_else(|| Arc::new(DefaultSchemaAdapterFactory));
+
+ if self.schema_adapter_factory.is_some() {
+ log::warn!("The SchemaAdapter API will be removed from ParquetSource in a future release. \
+ Use PhysicalExprAdapterFactory API instead. \
+ See https://github.com/apache/datafusion/issues/16800 for discussion and https://datafusion.apache.org/library-user-guide/upgrading.html#datafusion-49-0-0 for upgrade instructions.");
+ }
+
+ let (expr_adapter_factory, schema_adapter_factory) = match (
Review Comment:
I'm a bit torn: we don't yet have an alternative for projections. We kind of want to mark it as deprecated for predicates but not for projections, which is weird. It may be in a bit of a limbo state for a release or two.
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#issuecomment-3078933391
I opened https://github.com/apache/datafusion/issues/16800 to track the big picture
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2210208739
##########
docs/source/library-user-guide/upgrading.md:
##########
@@ -120,6 +120,17 @@ SET datafusion.execution.spill_compression = 'zstd';
For more details about this configuration option, including performance trade-offs between different compression codecs, see the [Configuration Settings](../user-guide/configs.md) documentation.
+### Custom `SchemaAdapterFactory` will no longer be used for predicate pushdown
Review Comment:
For all schema conversions. Projection pushdown will also use the same expression rewrite machinery.
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#issuecomment-3075481633
> I'd like to add a unit test that confirms the custom schema adapter factory will be used if specified.
done
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #16791:
URL: https://github.com/apache/datafusion/pull/16791
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2210616673
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -140,6 +143,8 @@ impl ArrowPredicate for DatafusionArrowPredicate {
}
fn evaluate(&mut self, batch: RecordBatch) -> ArrowResult<BooleanArray> {
+ let batch = self.schema_mapper.map_batch(batch)?;
Review Comment:
This code basically becomes a no-op if a predicate rewriter is used. But instead of making it Option<SchemaAdapter> or something like that I think it's easier to minimize the version on version diff and code paths to leave it be.
It's a no-op because when the schema mapping is calculated it will be from the physical file schema to the physical file schema.
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2211266501
##########
datafusion/datasource-parquet/src/source.rs:
##########
@@ -468,10 +468,50 @@ impl FileSource for ParquetSource {
let projection = base_config
.file_column_projection_indices()
.unwrap_or_else(|| (0..base_config.file_schema.fields().len()).collect());
- let schema_adapter_factory = self
- .schema_adapter_factory
- .clone()
- .unwrap_or_else(|| Arc::new(DefaultSchemaAdapterFactory));
+
+ if self.schema_adapter_factory.is_some() {
+ log::warn!("The SchemaAdapter API will be removed from ParquetSource in a future release. \
+ Use PhysicalExprAdapterFactory API instead. \
+ See https://github.com/apache/datafusion/issues/16800 for discussion and https://datafusion.apache.org/library-user-guide/upgrading.html#datafusion-49-0-0 for upgrade instructions.");
+ }
+
+ let (expr_adapter_factory, schema_adapter_factory) = match (
Review Comment:
this is good.
Should we mark `SchemaAdapter` a deprecated as well? Maybe as a follow on PR?
##########
datafusion/core/tests/parquet/schema_adapter.rs:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::sync::Arc;
+
+use arrow::array::{record_batch, RecordBatch};
+use arrow_schema::{DataType, Field, Schema};
+use bytes::{BufMut, BytesMut};
+use datafusion::assert_batches_eq;
+use datafusion::datasource::listing::{ListingTable, ListingTableConfig};
+use datafusion::prelude::SessionContext;
+use datafusion_datasource::schema_adapter::DefaultSchemaAdapterFactory;
+use datafusion_datasource::ListingTableUrl;
+use datafusion_execution::object_store::ObjectStoreUrl;
+use datafusion_physical_expr::schema_rewriter::DefaultPhysicalExprAdapterFactory;
+use object_store::{memory::InMemory, path::Path, ObjectStore};
+use parquet::arrow::ArrowWriter;
+
+async fn write_parquet(batch: RecordBatch, store: Arc<dyn ObjectStore>, path: &str) {
+ let mut out = BytesMut::new().writer();
+ {
+ let mut writer = ArrowWriter::try_new(&mut out, batch.schema(), None).unwrap();
+ writer.write(&batch).unwrap();
+ writer.finish().unwrap();
+ }
+ let data = out.into_inner().freeze();
+ store.put(&Path::from(path), data.into()).await.unwrap();
+}
+
+#[tokio::test]
+async fn single_file() {
+ let batch =
+ record_batch!(("extra", Int64, [1, 2, 3]), ("c1", Int32, [1, 2, 3])).unwrap();
+
+ let store = Arc::new(InMemory::new()) as Arc<dyn ObjectStore>;
+ let store_url = ObjectStoreUrl::parse("memory://").unwrap();
+ let path = "test.parquet";
+ write_parquet(batch, store.clone(), path).await;
+
+ let table_schema = Arc::new(Schema::new(vec![
+ Field::new("c1", DataType::Int64, false),
+ Field::new("c2", DataType::Utf8, true),
+ ]));
+
+ let ctx = SessionContext::new();
+ ctx.register_object_store(store_url.as_ref(), Arc::clone(&store));
+
+ let listing_table_config =
+ ListingTableConfig::new(ListingTableUrl::parse("memory:///").unwrap())
+ .infer_options(&ctx.state())
+ .await
+ .unwrap()
+ .with_schema(table_schema.clone())
+ .with_schema_adapter_factory(Arc::new(DefaultSchemaAdapterFactory))
+ .with_physical_expr_adapter_factory(Arc::new(
Review Comment:
is the idea to show the default being used? Or did you mean to also provide a custom factory that changed the schema?
##########
docs/source/library-user-guide/upgrading.md:
##########
@@ -120,6 +120,17 @@ SET datafusion.execution.spill_compression = 'zstd';
For more details about this configuration option, including performance trade-offs between different compression codecs, see the [Configuration Settings](../user-guide/configs.md) documentation.
+### Custom `SchemaAdapterFactory` will no longer be used for predicate pushdown
+
+We are moving away from converting data (using `SchemaAdapter`) to converting the expressions themselves (which is more efficient and flexible).
Review Comment:
```suggestion
We are moving away from converting data (using `SchemaAdapter`) to converting the expressions themselves (which is more efficient and flexible).
See [isseu #16800](https://github.com/apache/datafusion/issues/16800) for more information
```
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#discussion_r2211279282
##########
datafusion/core/tests/parquet/schema_adapter.rs:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::sync::Arc;
+
+use arrow::array::{record_batch, RecordBatch};
+use arrow_schema::{DataType, Field, Schema};
+use bytes::{BufMut, BytesMut};
+use datafusion::assert_batches_eq;
+use datafusion::datasource::listing::{ListingTable, ListingTableConfig};
+use datafusion::prelude::SessionContext;
+use datafusion_datasource::schema_adapter::DefaultSchemaAdapterFactory;
+use datafusion_datasource::ListingTableUrl;
+use datafusion_execution::object_store::ObjectStoreUrl;
+use datafusion_physical_expr::schema_rewriter::DefaultPhysicalExprAdapterFactory;
+use object_store::{memory::InMemory, path::Path, ObjectStore};
+use parquet::arrow::ArrowWriter;
+
+async fn write_parquet(batch: RecordBatch, store: Arc<dyn ObjectStore>, path: &str) {
+ let mut out = BytesMut::new().writer();
+ {
+ let mut writer = ArrowWriter::try_new(&mut out, batch.schema(), None).unwrap();
+ writer.write(&batch).unwrap();
+ writer.finish().unwrap();
+ }
+ let data = out.into_inner().freeze();
+ store.put(&Path::from(path), data.into()).await.unwrap();
+}
+
+#[tokio::test]
+async fn single_file() {
+ let batch =
+ record_batch!(("extra", Int64, [1, 2, 3]), ("c1", Int32, [1, 2, 3])).unwrap();
+
+ let store = Arc::new(InMemory::new()) as Arc<dyn ObjectStore>;
+ let store_url = ObjectStoreUrl::parse("memory://").unwrap();
+ let path = "test.parquet";
+ write_parquet(batch, store.clone(), path).await;
+
+ let table_schema = Arc::new(Schema::new(vec![
+ Field::new("c1", DataType::Int64, false),
+ Field::new("c2", DataType::Utf8, true),
+ ]));
+
+ let ctx = SessionContext::new();
+ ctx.register_object_store(store_url.as_ref(), Arc::clone(&store));
+
+ let listing_table_config =
+ ListingTableConfig::new(ListingTableUrl::parse("memory:///").unwrap())
+ .infer_options(&ctx.state())
+ .await
+ .unwrap()
+ .with_schema(table_schema.clone())
+ .with_schema_adapter_factory(Arc::new(DefaultSchemaAdapterFactory))
+ .with_physical_expr_adapter_factory(Arc::new(
Review Comment:
I'm going to follow up with a commit that uses several combinations of custom factories
--
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] Restore custom SchemaAdapter functionality for Parquet [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #16791:
URL: https://github.com/apache/datafusion/pull/16791#issuecomment-3074657503
I'd like to add a unit test that confirms the custom schema adapter factory will be used if specified.
--
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