Copilot commented on code in PR #13042:
URL: https://github.com/apache/gluten/pull/13042#discussion_r4277871852
##########
gluten-delta/src/main/scala/org/apache/gluten/extension/OffloadDeltaScan.scala:
##########
@@ -35,9 +35,13 @@ case class OffloadDeltaScan(enableNativeDmlRowIndexScan:
Boolean) extends Offloa
// Spark 3.5+ exposes this as
ParquetFileFormat.ROW_INDEX_TEMPORARY_COLUMN_NAME.
private val parquetTemporaryRowIndexColumnName = "_tmp_metadata_row_index"
- // Row-index columns Delta generates as top-level scan outputs.
+ // Row-index columns used by Delta DML with either row-index generation path.
private val generatedRowIndexColumnNames =
Set(DeltaParquetFileFormat.ROW_INDEX_COLUMN_NAME,
parquetTemporaryRowIndexColumnName)
+ private val generatedDeletionVectorMetadataColumnNames =
+ Set(
+ DeltaParquetFileFormat.ROW_INDEX_COLUMN_NAME,
+ DeltaParquetFileFormat.IS_ROW_DELETED_COLUMN_NAME)
Review Comment:
The PR says scans requesting Spark's top-level `_tmp_metadata_row_index`
also fall back when metadata row indexes are disabled, but this set contains
only the two internal Delta names. A non-DML Delta scan requesting only the
temporary field therefore passes this guard and reaches `DeltaScanTransformer`;
neither new suite tests that case. Include the temporary name and test its
fallback, or clarify that native handling of this Parquet row-index field is
intentionally retained. This is a mismatch with the stated fallback scope, not
evidence that the native index is wrong.
--
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]
