[GLUTEN-12436][VL] Map ORC files by position per-file to match Spark's OrcUtils, and remove the orcUseColumnNames config - #12453
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Aligns Gluten’s ORC/DWRF column mapping behavior with vanilla Spark by enabling per-file mapping decisions (name vs. position) and removing the redundant global orcUseColumnNames flag.
Changes:
- Added regression tests ensuring
_col*ORC files fall back to positional mapping without settingorc.force.positional.evolution, while real-named ORC files in the same session still map by name. - Replaced the removed Velox ORC “use column names” session behavior with forwarding Spark’s
orc.force.positional.evolutioninto a new native Velox flag. - Updated Velox iterator/schema attachment and cleaned up configs/docs/tests referencing
orcUseColumnNames.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| gluten-ut/spark33..41/.../GlutenHiveSQLQuerySuite.scala | Adds a regression test for mixed per-file ORC column mapping modes within one session/query. |
| gluten-substrait/.../GlutenConfig.scala | Forwards Spark positional evolution flag to native via a new Velox config key. |
| cpp/velox/utils/ConfigExtractor.cc | Forces ORC name-mapping default and forwards new positional evolution session property to Velox. |
| cpp/velox/config/VeloxConfig.h | Replaces old ORC config constant with the new positional evolution config key. |
| backends-velox/.../VeloxIteratorApi.scala | Always attaches schema for ORC/DWRF to enable per-file mapping in native reader. |
| backends-velox/.../VeloxConfig.scala | Removes orcUseColumnNames config and accessor. |
| backends-velox/.../VeloxScanSuite.scala | Updates ORC evolution test to use Spark’s orc.force.positional.evolution. |
| backends-velox/.../FallbackSuite.scala | Removes ORC config dimension now that orcUseColumnNames is gone. |
| backends-velox/.../VeloxBackend.scala | Adjusts schema validation gating after ORC orcUseColumnNames removal. |
| docs/velox-configuration.md | Removes orcUseColumnNames from documented configuration list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+167
to
+168
| val colStarLoc = s"file:///$dir/test_orc_colstar" | ||
| val namedLoc = s"file:///$dir/test_orc_named" |
Comment on lines
+152
to
+162
| testGluten( | ||
| "GLUTEN: Hive ORC files with _col* names read by position without positional flag") { | ||
| // Regression for the case where two ORC tables must use OPPOSITE column | ||
| // mapping modes in the same query: one with real column names (by name) and | ||
| // one written by old Hive with placeholder _col* names (by position). The | ||
| // native reader must decide the mode per file (matching vanilla Spark's | ||
| // OrcUtils.requestedColumnIds), so a _col* file reads correctly even though | ||
| // orc.force.positional.evolution is NOT set (ORC is read by name by | ||
| // default). Without the fix the _col* columns would read back as NULL. | ||
| val hiveClient: HiveClient = | ||
| spark.sharedState.externalCatalog.unwrapped.asInstanceOf[HiveExternalCatalog].client |
Comment on lines
57
to
58
| | spark.gluten.sql.columnar.backend.velox.orc.scan.enabled | 🔄 Dynamic | true | Enable velox orc scan. If disabled, vanilla spark orc scan will be used. | | ||
| | spark.gluten.sql.columnar.backend.velox.orcUseColumnNames | 🔄 Dynamic | true | Maps table field names to file field names using names, not indices for ORC files. | | ||
| | spark.gluten.sql.columnar.backend.velox.parquet.dictionaryPageSizeBytes | 🔄 Dynamic | 2MB | The maximum size in bytes for a Parquet dictionary page | |
Comment on lines
595
to
602
| if ( | ||
| backendName == "velox" && | ||
| conf.getOrElse(SPARK_ORC_FORCE_POSITIONAL_EVOLUTION, "false").toBoolean | ||
| ) { | ||
| nativeConfMap.put("spark.gluten.sql.columnar.backend.velox.orcUseColumnNames", "false") | ||
| nativeConfMap.put( | ||
| "spark.gluten.sql.columnar.backend.velox.orcForcePositionalEvolution", | ||
| "true") | ||
| } |
|
Run Gluten Clickhouse CI on x86 |
Comment on lines
235
to
238
| def validateDataSchema(): Option[String] = { | ||
| if (VeloxConfig.get.parquetUseColumnNames && VeloxConfig.get.orcUseColumnNames) { | ||
| if (VeloxConfig.get.parquetUseColumnNames) { | ||
| return None | ||
| } |
|
Run Gluten Clickhouse CI on x86 |
Comment on lines
235
to
238
| def validateDataSchema(): Option[String] = { | ||
| if (VeloxConfig.get.parquetUseColumnNames && VeloxConfig.get.orcUseColumnNames) { | ||
| if (VeloxConfig.get.parquetUseColumnNames) { | ||
| return None | ||
| } |
|
Run Gluten Clickhouse CI on x86 |
Comment on lines
+236
to
238
| if (VeloxConfig.get.parquetUseColumnNames) { | ||
| return None | ||
| } |
Comment on lines
316
to
338
| format => | ||
| Seq("true", "false").foreach { | ||
| parquetUseColumnNames => | ||
| Seq("true", "false").foreach { | ||
| orcUseColumnNames => | ||
| withSQLConf( | ||
| VeloxConfig.PARQUET_USE_COLUMN_NAMES.key -> parquetUseColumnNames, | ||
| VeloxConfig.ORC_USE_COLUMN_NAMES.key -> orcUseColumnNames | ||
| ) { | ||
| withTable("test") { | ||
| spark | ||
| .range(100) | ||
| .selectExpr("to_timestamp_ntz(from_unixtime(id % 3)) as c1", "id as c2") | ||
| .write | ||
| .format(format) | ||
| .saveAsTable("test") | ||
|
|
||
| runQueryAndCompare(query) { | ||
| df => | ||
| val plan = df.queryExecution.executedPlan | ||
| assert(collect(plan) { case g: GlutenPlan => g }.nonEmpty) | ||
| } | ||
| } | ||
| withSQLConf( | ||
| VeloxConfig.PARQUET_USE_COLUMN_NAMES.key -> parquetUseColumnNames | ||
| ) { | ||
| withTable("test") { | ||
| spark | ||
| .range(100) | ||
| .selectExpr("to_timestamp_ntz(from_unixtime(id % 3)) as c1", "id as c2") | ||
| .write | ||
| .format(format) | ||
| .saveAsTable("test") | ||
|
|
||
| runQueryAndCompare(query) { | ||
| df => | ||
| val plan = df.queryExecution.executedPlan | ||
| assert(collect(plan) { case g: GlutenPlan => g }.nonEmpty) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
|
Run Gluten Clickhouse CI on x86 |
Comment on lines
235
to
238
| def validateDataSchema(): Option[String] = { | ||
| if (VeloxConfig.get.parquetUseColumnNames && VeloxConfig.get.orcUseColumnNames) { | ||
| if (VeloxConfig.get.parquetUseColumnNames) { | ||
| return None | ||
| } |
Comment on lines
315
to
+321
| Seq("parquet", "orc").foreach { | ||
| format => | ||
| Seq("true", "false").foreach { | ||
| parquetUseColumnNames => | ||
| Seq("true", "false").foreach { | ||
| orcUseColumnNames => | ||
| withSQLConf( | ||
| VeloxConfig.PARQUET_USE_COLUMN_NAMES.key -> parquetUseColumnNames, | ||
| VeloxConfig.ORC_USE_COLUMN_NAMES.key -> orcUseColumnNames | ||
| ) { | ||
| withTable("test") { | ||
| spark | ||
| .range(100) | ||
| .selectExpr("to_timestamp_ntz(from_unixtime(id % 3)) as c1", "id as c2") | ||
| .write | ||
| .format(format) | ||
| .saveAsTable("test") | ||
|
|
||
| runQueryAndCompare(query) { | ||
| df => | ||
| val plan = df.queryExecution.executedPlan | ||
| assert(collect(plan) { case g: GlutenPlan => g }.nonEmpty) | ||
| } | ||
| } | ||
| withSQLConf( | ||
| VeloxConfig.PARQUET_USE_COLUMN_NAMES.key -> parquetUseColumnNames | ||
| ) { |
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala:238
validateDataSchema()now short-circuits solely onparquetUseColumnNames, butVeloxIteratorApiattaches the table schema for ORC/DWRF unconditionally. That means ORC/DWRF scans may pass unsupported Spark data types down to Velox without this validation (previously guarded byorcUseColumnNames). Consider validating the table schema whenever it will be sent to native: always for ORC/DWRF, and for Parquet only whenparquetUseColumnNames=false.
def validateDataSchema(): Option[String] = {
if (VeloxConfig.get.parquetUseColumnNames) {
return None
}
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala:242
validateDataSchema()now only runs when Parquet is using index-based mapping, butVeloxIteratorApialways attachesdataSchemafor ORC/DWRF scans as well. That means unsupported Spark data types in an ORC/DWRF table schema could reach native without pre-validation/fallback, potentially causing runtime failures that this validator is meant to avoid. Consider triggering the same schema-type validation whenever the scan format is ORC/DWRF (since schema is always forwarded there) in addition to the existing Parquet index-mapping case.
def validateDataSchema(): Option[String] = {
if (VeloxConfig.get.parquetUseColumnNames) {
return None
}
// If we are using column indices for schema evolution, we need to pass the table schema to
// Velox. We need to ensure all types in the table schema are supported.
val validationResults =
…OrcUtils, and remove the orcUseColumnNames config
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala:238
validateDataSchema()now skips validation wheneverparquetUseColumnNamesis true, but this validation also becomes relevant for ORC/DWRF after this PR becauseVeloxIteratorApialways attaches the table schema for ORC/DWRF scans. Skipping validation in mixed-format tables (or pure ORC/DWRF scans) can let unsupported Spark types reach native and fail later at runtime instead of cleanly falling back.
def validateDataSchema(): Option[String] = {
if (VeloxConfig.get.parquetUseColumnNames) {
return None
}
Contributor
Author
rui-mo
reviewed
Jul 29, 2026
rui-mo
left a comment
rui-mo
left a comment
Contributor
There was a problem hiding this comment.
Thanks @beliefer. It looks good to me to bring the ORC reader's position/name selection behavior fully in line with Spark. One concern is that this change could be a breaking change for existing users who rely on this option. Could you add a before-and-after behavior comparison to the PR description?
Contributor
Author
Updated. |
rui-mo
approved these changes
Jul 30, 2026
Contributor
Author
|
@rui-mo Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
Fixies #12436. This PR is related to facebookincubator/velox#18027
Gluten currently chooses ORC/DWRF column mapping (by name vs. by position) globally for a whole session, driven by
spark.gluten.sql.columnar.backend.velox.orcUseColumnNames(defaulttrue, map by name) combined with an override fororc.force.positional.evolution.Vanilla Spark does not work that way.
OrcUtils.requestedColumnIdsdecides the mapping mode per file:Because Gluten only had a single global switch, it could not match Spark when a single query touches ORC tables that require opposite mapping modes — for example a join between:
_col0, _col1, ...(must be read by position).With the global switch, one value is always wrong for one of the tables:
orcUseColumnNames=truereads the_col*table back as NULL (name lookup fails), while settingorc.force.positional.evolution=trueglobally forces the real-named table to be read by position and mis-binds its columns.This PR aligns Gluten with Spark's per-file behavior (backed by a companion Velox change) and, as requested, removes the now-redundant
orcUseColumnNamesconfig and its accessor.How the per-file decision works now
The native (Velox) reader makes the decision per ORC/DWRF file:
_col*placeholder names is mapped by position, regardless of any flag;orc.force.positional.evolution=trueis forwarded to native;For this to work, Gluten must always hand the table (data) schema to the native reader for ORC/DWRF so it has a target to remap file columns to.
Notes
This is the Gluten side of a two-part change. The companion Velox change (see: facebookincubator/velox#18027) adds per-file positional mapping in the DWRF reader: it maps a file by position when the physical schema is all
_col*placeholder names or when theorc.force-positional-evolutionsession property is set. This Gluten PR depends on that Velox change being present in the Velox build.Before / after behavior comparison
The table below compares how an ORC/DWRF file is read before and after this PR. "By name" means table columns are matched to file columns by field name; "by position" means by column index.
orcUseColumnNamesis the removed Gluten config;orc.force.positional.evolutionis the standard Spark/Hive property (unchanged, still honored).orcUseColumnNames)orcUseColumnNames=true)_col*placeholder names, default configorc.force.positional.evolution=true_col*(e.g. a join), default config_col*→ by position)orcUseColumnNames=falseorc.force.positional.evolution=trueto force positionThe only breaking row is the last one: reading a file with real physical names positionally by index. That was a Gluten-only extension with no vanilla Spark equivalent. Users who relied on it should set
orc.force.positional.evolution=true, which produces the same positional mapping through the Spark-compatible path. All other rows are either unchanged or fix a pre-existing correctness bug.Why removing the config is safe
The only capability lost by removing
orcUseColumnNames=falseis "read a file with real physical names positionally by index" — which vanilla Spark does not do. Spark only maps by position when the file is all_col*ororc.force.positional.evolution=true, both of which are still supported (and now per-file). This removes a Gluten-only divergence and makes ORC reads match Spark.How was this patch tested?
gluten-ut(spark33/34/35/40/41)GlutenHiveSQLQuerySuite— new regression test: two ORC tables over the same_col*files (placeholder names) plus a real-named table, all read in one session without setting the positional flag. Asserts the_col*table reads correctly (positional fallback), the real-named table reads correctly by name (opposite mode), and a join of the two returns a non-empty result (the original failure folded the join to an emptyLocalTableScan).VeloxScanSuite— the former "ORC index based schema evolution" test (which relied onorcUseColumnNames=falseto read real-named files by index, a non-Spark behavior) is rewritten as "ORC positional schema evolution" usingorc.force.positional.evolution=true, which produces the same positional mapping via the Spark-compatible path. Expected results are unchanged.FallbackSuite— the "fallback with index based schema evolution" test drops theorcUseColumnNamesdimension (the parquet dimension is retained).Was this patch authored or co-authored using generative AI tooling?
co-authored Claude code.