[GLUTEN-12436][VL] Map ORC files with all-_col* physical names by position even when orcUseColumnNames is true - #12438
[GLUTEN-12436][VL] Map ORC files with all-_col* physical names by position even when orcUseColumnNames is true#12438beliefer wants to merge 1 commit into
_col* physical names by position even when orcUseColumnNames is true#12438Conversation
…hen orcUseColumnNames is true
|
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Pull request overview
This PR addresses incorrect ORC reads in Velox when a single query touches a mix of ORC files with real physical column names and ORC files whose physical field names are Hive placeholders (_col0, _col1, …). It enables the native reader to make a per-file mapping decision (name vs position) by ensuring the requested data schema is always attached to ORC/DWRF splits, and adds a regression test across all supported Spark shims.
Changes:
- Always attach the (requested) data schema to ORC/DWRF
LocalFilesNodesplits inVeloxIteratorApi, even whenorcUseColumnNames=true. - Add a regression test (spark33/34/35/40/41) covering mixed
_col*-schema ORC files + named-schema ORC files in the same session/query.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxIteratorApi.scala | Always attaches schema for ORC/DWRF splits so native reader can perform per-file mapping decisions. |
| gluten-ut/spark33/src/test/scala/org/apache/spark/sql/hive/execution/GlutenHiveSQLQuerySuite.scala | Adds regression coverage for _col* placeholder ORC files + named ORC files in one session. |
| gluten-ut/spark34/src/test/scala/org/apache/spark/sql/hive/execution/GlutenHiveSQLQuerySuite.scala | Same regression test for Spark 3.4 shim. |
| gluten-ut/spark35/src/test/scala/org/apache/spark/sql/hive/execution/GlutenHiveSQLQuerySuite.scala | Same regression test for Spark 3.5 shim. |
| gluten-ut/spark40/src/test/scala/org/apache/spark/sql/hive/execution/GlutenHiveSQLQuerySuite.scala | Same regression test for Spark 4.0 shim. |
| gluten-ut/spark41/src/test/scala/org/apache/spark/sql/hive/execution/GlutenHiveSQLQuerySuite.scala | Same regression test for Spark 4.1 shim. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val joined = sql( | ||
| "select c.name, n.label from test_orc_colstar_renamed c " + | ||
| "join test_orc_named n on c.id = n.uid") | ||
| checkAnswer(joined, Seq(Row("a", "b"))) |
| s"create table test_orc_named(uid int, label string) " + | ||
| s"stored as orc location '$namedLoc'") | ||
| hiveClient.runSqlHive("insert into test_orc_named select 7, 'b'") | ||
|
|
| val joined = sql( | ||
| "select c.name, n.label from test_orc_colstar_renamed c " + | ||
| "join test_orc_named n on c.id = n.uid") | ||
| checkAnswer(joined, Seq(Row("a", "b"))) |
| val joined = sql( | ||
| "select c.name, n.label from test_orc_colstar_renamed c " + | ||
| "join test_orc_named n on c.id = n.uid") | ||
| checkAnswer(joined, Seq(Row("a", "b"))) |
| val joined = sql( | ||
| "select c.name, n.label from test_orc_colstar_renamed c " + | ||
| "join test_orc_named n on c.id = n.uid") | ||
| checkAnswer(joined, Seq(Row("a", "b"))) |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
Because #12453 merged, so I close this one. |
What changes are proposed in this pull request?
Fixies #12436. This PR is related to facebookincubator/velox#18021.
Gluten decides the ORC column-mapping mode from a single global config:
spark.hadoop.orc.force.positional.evolutionflipsorcUseColumnNames, which selectsname-based vs position-based mapping for the whole query. This breaks any query that reads a
mix of ORC tables where some files carry real physical column names and some carry Hive
placeholder names (
_col0,_col1, …):orcUseColumnNames=true(default): the_col*files have no real names to match, so everycolumn reads back null → a filtered join side becomes empty and AQE folds the plan to
LocalTableScan rows=0(silently wrong result).orcUseColumnNames=false(positional): the real-name files are mapped by ordinal and land onthe wrong column, failing with
SCHEMA_MISMATCH(From Kind: INTEGER, To Kind: VARCHAR) or, whena string filter is pushed down,
Filter(BytesValues, ...): testInt64Range() is not supported.No single global value can read both tables correctly in one query.
Vanilla Spark makes this decision per file in
OrcUtils.requestedColumnIds:Gluten already honors the
forcePositionalEvolutionhalf (#12234). The missing half isorcFieldNames.forall(_.startsWith("_col")): even in name mode, an ORC file whose physical schemais entirely
_col*must be mapped by position, because those placeholder names carry no identity —the real names live only in the table schema. That per-file decision can only be made in the native
reader, which is the only layer that sees each file's physical field names.
This PR contains the Gluten-side change; the native-reader change is submitted separately to Velox
(
facebookincubator/velox, see: facebookincubator/velox#18021), which adds the_col*detection toDwrfReaderand falls back to positional mapping in name mode.Gluten change —
VeloxIteratorApi.setFileSchemaForLocalFiles:Always attach the table (data) schema to the split for ORC/DWRF, instead of only when
orcUseColumnNames=false. The native reader needs the table schema in name mode too, so it canremap the
_col*file columns to the real names by position. Parquet behavior is unchanged (stillonly attached when mapping by position). This is additive: the existing positional path and the
explicit
orc.force.positional.evolution=truebehavior are unaffected.How was this patch tested?
Added a regression test to
GlutenHiveSQLQuerySuitefor all supported Spark shims(spark33/34/35/40/41). The test:
_col0/_col1so the physical ORC fieldnames are guaranteed to be placeholders (independent of the embedded Hive version; mirrors Spark's
SPARK-34897 setup), plus a second metastore table with real names over the same files, and a
separate real-name ORC table.
spark.hadoop.orc.force.positional.evolution(defaultorcUseColumnNames=true),asserts that the
_col*table reads correct values via the real column names, that the real-nametable still reads correctly by name in the same session, and that a join of the two returns a
non-empty result (the original failure folded it to an empty
LocalTableScan).checkOperatorMatch[HiveTableScanExecTransformer].Note: the test passes end-to-end only together with the corresponding Velox change; the Gluten-side
change alone attaches the schema but relies on the native reader to perform the
_col*positionalfallback.
Was this patch authored or co-authored using generative AI tooling?
co-authored with Claude Code (Claude Opus 4.8)