Repository navigation
Conversation
|
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (3)
writetoSparkWrite. Since the method signature acceptsWrite,… · New
What changed in this PR
Enable native Velox writes for Iceberg tables that define a sort order, supporting both global (range-distributed) and local (task-local) sorted writes without falling back to Spark.
Changes:
- Adjust sort-order labeling so writes only advertise a sort order when Spark planned an ordering.
- Gate validation of sorted-table writes behind a backend capability flag (
supportsSortedWrite), enabling Velox sorted writes. - Add documentation and a dedicated test suite covering global/local/partitioned/transformed/nested sort keys and file rotation.
| File | Description |
|---|---|
| gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala | Derives the sort order used for write metadata based on Spark’s required ordering. |
| gluten-iceberg/src/main/scala/org/apache/gluten/execution/IcebergWriteExec.scala | Introduces a capability flag and relaxes sort-order validation for backends that support sorted writes. |
| backends-velox/src-iceberg/main/scala/org/apache/gluten/execution/AbstractIcebergWriteExec.scala | Enables sorted writes for the Velox backend by overriding the capability flag. |
| docs/get-started/VeloxIceberg.md | Documents how to use sorted native writes and relevant table properties. |
| backends-velox/src-iceberg/test/scala/org/apache/gluten/execution/enhanced/VeloxIcebergSuite.scala | Removes the old “sorted table must fallback” test that no longer applies for Velox. |
| backends-velox/src-iceberg/test/scala/org/apache/gluten/execution/enhanced/VeloxIcebergSortOrderSuite.scala | Adds comprehensive tests validating sorted file contents and non-overlapping ranges (global) across multiple scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (write.asInstanceOf[SparkWrite].requiredOrdering().nonEmpty) { | ||
| getTable(write).sortOrder() | ||
| } else { | ||
| SortOrder.unsorted() |
There was a problem hiding this comment.
Validation checks supportsWrite before this helper is reached so this isn't an issue
| } | ||
| } | ||
| if (IcebergWriteUtil.getTable(write).sortOrder().isSorted) { | ||
| if (!supportsSortedWrite && IcebergWriteUtil.getTable(write).sortOrder().isSorted) { |
There was a problem hiding this comment.
fixed, Changed validation to use getSortOrder(write).isSorted
| Supported Parquet writes can offload when | ||
| `spark.gluten.sql.enable.enhancedFeatures` and | ||
| `spark.gluten.sql.columnar.iceberg.enableNativeWrite` are enabled. |
1fab148 to
8400ec6
Compare
6946dbb to
151a6b2
Compare
|
Run Gluten Clickhouse CI on x86 |
151a6b2 to
737be71
Compare
|
Run Gluten Clickhouse CI on x86 |
2 similar comments
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
jinchengchenghh
left a comment
jinchengchenghh
left a comment
There was a problem hiding this comment.
Which PR in Velox supports this feature?
| def getSortOrder(write: Write): SortOrder = { | ||
| getTable(write).sortOrder() | ||
| if (write.asInstanceOf[SparkWrite].requiredOrdering().nonEmpty) { | ||
| getTable(write).sortOrder() |
There was a problem hiding this comment.
What returns when write.asInstanceOf[SparkWrite].requiredOrdering() is empty?
There was a problem hiding this comment.
SortOrder.unsorted() will be returned and the output files will be recorded as unsorted
There was a problem hiding this comment.
What if we return getTable(write).sortOrder() in this case?
There was a problem hiding this comment.
The table’s sort order tells us how the data should be sorted, but this is for the individual write which can be unsorted. So if we return the table's sort order it will just label the files sorted even if they aren't
737be71 to
914c850
Compare
|
Run Gluten Clickhouse CI on x86 |
914c850 to
d73f971
Compare
|
Run Gluten Clickhouse CI on x86 |
Support
INSERT INTOandINSERT OVERWRITEon sorted Iceberg tables using the native Velox writer. Previously, tables with a defined sort order caused writes to fall back to Spark.Users can now perform:
write.distribution-mode=range): range-distribute incoming rows and produce sorted files with non-overlapping sort-key ranges within each Iceberg partition for that write.write.distribution-mode=none): sort rows within each write task without requiring redistribution for the write.Supports ascending/descending order, null placement, transformed and nested sort keys, and preserves ordering across file rotation.
Does not reorganize existing files.