Skip to content

[SPARK-59609][SQL] Fix Parquet/ORC aggregate push-down reading statistics from the wrong column - #58945

Open
hemanthboyina wants to merge 2 commits into
apache:masterfrom
hemanthboyina:spark-59609-agg-pushdown-schema-merge
Open

hemanthboyina wants to merge 2 commits into
apache:masterfrom
hemanthboyina:spark-59609-agg-pushdown-schema-merge

Conversation

@hemanthboyina

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

When aggregate push-down is enabled, Spark answers MIN/MAX/COUNT from a file's footer statistics instead of reading the data. To locate a column's statistics it used the column's position in the read schema, then applied that position to the file's physical columns. This is only correct when every file's layout matches the read schema. This PR resolves the column against each file's own schema instead:

for Parquet, it looks the column up by name in the file's footer schema;
for ORC, it resolves the column to its position in the file's own schema, following the same name/positional rules ORC already uses in requestedColumnIds.

If a file does not contain the column, MIN/MAX return null and COUNT(col) counts no non-null values, instead of reading the wrong column.

Why are the changes needed?

The old position-based lookup silently returns wrong results when a file's column layout differs from the read schema, e.g. Parquet with mergeSchema=true where some files are missing the column, or ORC files with a different column order.

Does this PR introduce any user-facing change?
Yes. With aggregate push-down enabled (spark.sql.parquet.aggregatePushdown / spark.sql.orc.aggregatePushdown, both false by default), the queries above now return correct results instead of silently wrong ones.

How was this patch tested?

Added tests to FileSourceAggregatePushDownSuite (Parquet mergeSchema with a missing column; ORC with a different column order), run with the vectorized reader on and off. Both fail before the fix and pass after. All existing push-down suites still pass.

Was this patch authored or co-authored using generative AI tooling?

Yes, used Claude code

// Resolve by name in the file's own schema; a positional lookup breaks under mergeSchema.
val fileSchema = footerFileMetaData.getSchema
def fileFieldIndex(colName: String): Int =
if (fileSchema.containsField(colName)) fileSchema.getFieldIndex(colName) else -1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems we don't check case sensitivity here. If so, I believe we need to pass down that flag, same as the ORC reader.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, fixed it now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants