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 2026/01/01 20:46:33 UTC
[PR] Record sort order when writing Parquet with WITH ORDER [datafusion]
adriangb opened a new pull request, #19595:
URL: https://github.com/apache/datafusion/pull/19595
## Which issue does this PR close?
Part of #19433
## Rationale for this change
When writing data to a table created with `CREATE EXTERNAL TABLE ... WITH ORDER`, the sorting columns should be recorded in the Parquet file's row group metadata. This allows downstream readers to know the data is sorted and potentially skip sorting operations.
## What changes are included in this PR?
- Add `sort_expr_to_sorting_column()` and `lex_ordering_to_sorting_columns()` functions in `metadata.rs` to convert DataFusion ordering to Parquet `SortingColumn`
- Add `sorting_columns` field to `ParquetSink` with `with_sorting_columns()` builder method
- Update `create_writer_physical_plan()` to pass order requirements to `ParquetSink`
- Update `create_writer_props()` to set sorting columns on `WriterProperties`
- Add test verifying `sorting_columns` metadata is written correctly
## Are these changes tested?
Yes, added `test_create_table_with_order_writes_sorting_columns` that:
1. Creates an external table with `WITH ORDER (a ASC NULLS FIRST, b DESC NULLS LAST)`
2. Inserts data
3. Reads the Parquet file and verifies the `sorting_columns` metadata matches the expected order
## Are there any user-facing changes?
No user-facing API changes. Parquet files written via `INSERT INTO` or `COPY` for tables with `WITH ORDER` will now contain `sorting_columns` metadata in the row group.
🤖 Generated with [Claude Code](https://claude.com/claude-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] Record sort order when writing Parquet with WITH ORDER [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #19595:
URL: https://github.com/apache/datafusion/pull/19595#issuecomment-3719284241
@zhuqi-lucas are you able to review 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-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] Record sort order when writing Parquet with WITH ORDER [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #19595:
URL: https://github.com/apache/datafusion/pull/19595
--
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] Record sort order when writing Parquet with WITH ORDER [datafusion]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #19595:
URL: https://github.com/apache/datafusion/pull/19595#discussion_r2656645780
##########
datafusion/datasource-parquet/src/metadata.rs:
##########
@@ -613,6 +616,40 @@ impl FileMetadata for CachedParquetMetaData {
}
}
+/// Convert a [`PhysicalSortExpr`] to a Parquet [`SortingColumn`].
+///
+/// Returns `Err` if the expression is not a simple column reference.
+pub(crate) fn sort_expr_to_sorting_column(
+ sort_expr: &PhysicalSortExpr,
+) -> Result<SortingColumn> {
+ let column = sort_expr
+ .expr
+ .as_any()
+ .downcast_ref::<Column>()
+ .ok_or_else(|| {
+ DataFusionError::Plan(format!(
+ "Parquet sorting_columns only supports simple column references, \
+ but got expression: {}",
+ sort_expr.expr
+ ))
+ })?;
+
+ Ok(SortingColumn {
+ column_idx: column.index() as i32,
Review Comment:
The conversion from `usize` to `i32` could potentially overflow for schemas with a very large number of columns. While unlikely in practice, consider using `try_into()` with proper error handling to make this conversion safe and explicit.
```suggestion
let column_idx: i32 = column.index().try_into().map_err(|_| {
DataFusionError::Plan(format!(
"Column index {} is too large to be represented as i32",
column.index()
))
})?;
Ok(SortingColumn {
column_idx,
```
--
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