infvg commented on code in PR #12726:
URL: https://github.com/apache/gluten/pull/12726#discussion_r4184663212
##########
backends-velox/src-delta33/main/scala/org/apache/spark/sql/delta/perf/GlutenDeltaOptimizedWriterExec.scala:
##########
@@ -84,7 +85,7 @@ case class GlutenDeltaOptimizedWriterExec(
@transient private lazy val mapTracker = SparkEnv.get.mapOutputTracker
private lazy val columnarShufflePlan = {
- val resolver =
org.apache.spark.sql.catalyst.analysis.caseInsensitiveResolution
+ val resolver = SQLConf.get.resolver
Review Comment:
I tested this with native Delta writes enabled and
`spark.sql.caseSensitive=true`. For an existing table partitioned by `Part`,
appending a column named `part` fails:
```scala
spark.range(4, 8, 1, 2)
.selectExpr("id", "cast(id % 2 as int) AS part")
.write.format("delta").mode("append")
.option("optimizeWrite", "true").save(path)
```
Expected: append succeeds
Actual:
```text
[DELTA_FAILED_FIND_PARTITION_COLUMN_IN_OUTPUT_PLAN] Could not find Part in
output plan.
```
##########
gluten-substrait/src/main/scala/org/apache/gluten/execution/BasicScanExecTransformer.scala:
##########
@@ -117,15 +118,28 @@ trait BasicScanExecTransformer extends
LeafTransformSupport with BaseDataSource
val metadataFromSpark = getMetadataColumns().map(_.name)
- val inputFileRelatedMetadataKeys = Seq(
- InputFileName().prettyName,
- InputFileBlockStart().prettyName,
- InputFileBlockLength().prettyName)
-
- val neededInputFileRelatedMetadataKeys =
- inputFileRelatedMetadataKeys.filter(k => output.exists(_.name == k))
+ // In addition to the "proper" Spark metadata columns
(FileSourceConstantMetadataAttribute),
+ // PreOffload may have injected AttributeReferences to carry input-file
function values.
+ // These attributes are identified by isInjectedInputFileAttr (presence of
+ // GLUTEN_INPUT_FILE_COL_ATTR_KEY in metadata) rather than by name,
because under
+ // caseSensitive=false the schema name may have been mangled (e.g.
+ // "__gluten_input_file_col__input_file_name__") to avoid colliding with a
user column that
+ // lowercases to the same name. The canonical prettyName is recovered via
+ // injectedInputFileCanonName and used as the key in the Velox split
infoColumns map, while
+ // the schema name (attr.name) is the key under which Velox looks up the
value.
+ //
+ // When no mangling occurred (caseSensitive=true or no collision) schema
name == canonical
+ // prettyName and both lookups use the same string.
+ //
+ // IMPORTANT: do NOT use normalizeColName here. A user column like
"Input_File_Name" must
+ // never match -- we identify injected attrs exclusively via their
metadata key.
+ val injectedInputFileCols: Seq[(String, String)] = output.collect {
+ case a if PushDownInputFileExpression.isInjectedInputFileAttr(a) =>
+ // (schemaName, canonicalPrettyName)
+ a.name -> PushDownInputFileExpression.injectedInputFileCanonName(a)
+ }
Review Comment:
I tested this locally with `caseSensitive=false` and a Parquet table
`t(id, Input_File_Name)` containing `(1, 'user-a'), (2, 'user-b')`:
```sql
SELECT id, input_file_name() AS fname
FROM (SELECT * FROM t UNION ALL SELECT * FROM t) u;
```
Expected: file paths for all four rows.
Actual:
```text
1, file:///...parquet
1, user-a
2, file:///...parquet
2, user-b
```
--
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]
