Copilot commented on code in PR #13199:
URL: https://github.com/apache/gluten/pull/13199#discussion_r4185525702
##########
docs/get-started/VeloxIceberg.md:
##########
@@ -30,17 +30,28 @@ Fallback
Fallback
## Writing
-Fallback
+Supported Parquet writes can offload when
+`spark.gluten.sql.enable.enhancedFeatures` and
+`spark.gluten.sql.columnar.iceberg.enableNativeWrite` are enabled.
Review Comment:
The SQL example uses a 4-backtick fence (````) while the later example uses
triple backticks with a `sql` language tag. Using consistent triple backticks
(and ` ```sql `) improves Markdown rendering and syntax highlighting across
common viewers.
##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/IcebergWriteUtil.scala:
##########
@@ -114,7 +114,14 @@ object IcebergWriteUtil {
}
def getSortOrder(write: Write): SortOrder = {
- getTable(write).sortOrder()
+ // SparkWrite requests the table sort order (with partition clustering)
unless the write
+ // disables distribution and ordering. In that case, do not label unsorted
files with the
+ // table's default sort order merely because the table defines one.
+ if (write.asInstanceOf[SparkWrite].requiredOrdering().nonEmpty) {
+ getTable(write).sortOrder()
+ } else {
+ SortOrder.unsorted()
Review Comment:
This unconditionally casts `write` to `SparkWrite`. Since the method
signature accepts `Write`, this can throw `ClassCastException` if a
non-`SparkWrite` implementation is passed. Prefer a safe pattern match (e.g.,
`write match { case sw: SparkWrite => ...; case _ => ... }`) or change the API
to accept `SparkWrite` if that’s the only supported caller.
##########
gluten-iceberg/src/main/scala/org/apache/gluten/execution/IcebergWriteExec.scala:
##########
@@ -130,7 +133,7 @@ trait IcebergWriteExec extends ColumnarV2TableWriteExec {
"Not support write unsupported partition type, or is nested
partition column")
}
}
- if (IcebergWriteUtil.getTable(write).sortOrder().isSorted) {
+ if (!supportsSortedWrite &&
IcebergWriteUtil.getTable(write).sortOrder().isSorted) {
Review Comment:
Validation checks the table’s declared sort order rather than the effective
sort order for this write. With the new `IcebergWriteUtil.getSortOrder(write)`
behavior (returning `unsorted` when Spark planned no ordering), this can still
force fallback even for intentionally unsorted writes into a sorted table. To
align behavior, validate against Spark’s planned ordering (e.g.,
`IcebergWriteUtil.getSortOrder(write).isSorted`) or directly use
`SparkWrite.requiredOrdering().nonEmpty` when `supportsSortedWrite` is false.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
