You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "prashantwason (via GitHub)" <gi...@apache.org> on 2026/09/22 20:13:09 UTC
[PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
prashantwason opened a new pull request, #20029:
URL: https://github.com/apache/hudi/pull/20029
HoodieFileGroupReaderBasedFileFormat.shortName() returned "HudiFileGroup" (mixed case), which failed to match Spark's lowercased allowlist in DataSourceUtils.supportNestedPredicatePushdown(). This silently disabled nested struct predicate pushdown (e.g., msg.field = 'value'), causing PushedFilters: [] in the physical plan and forcing full row group reads.
Fix:
- Change shortName() to return "hoodiefilegroup" (lowercase) so it can match the allowlist after Spark lowercases it.
- Add "hoodiefilegroup" to the supportedFileSources config in HoodieSparkSessionExtension via an injected optimizer rule.
- Add unit test verifying nested struct predicates appear in PushedFilters.
Impact: On tables with nested struct columns, this was causing up to 88x more data reads (27.4 TB vs 309 GB observed in production benchmarks). Affects all Hudi users on any Spark 3.x.
### Describe the issue this Pull Request addresses
<!-- Either describe the issue inline here with motivation behind the changes
(or) link to an issue by including `Closes #<issue-number>` for context.
If this PR includes changes to the storage format, public APIs,
or has breaking changes, use `!` (e.g., feat!: ...) -->
### Summary and Changelog
<!-- Short, plain-English summary of what users gain or what changed in behavior.
Followed by a detailed log of all the changes. Highlight if any code was copied. -->
### Impact
<!-- Describe any public API or user-facing feature change or any performance impact. -->
### Risk Level
<!-- Accepted values: none, low, medium or high. Other than `none`, explain the risk.
If medium or high, explain what verification was done to mitigate the risks. -->
### Documentation Update
<!-- Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".
- The config description must be updated if new configs are added or the default value of the configs are changed.
- Any new feature or user-facing change requires updating the Hudi website. Please follow the
[instruction](https://hudi.apache.org/contribute/developer-setup#website) to make changes to the website. -->
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Enough context is provided in the sections above
- [ ] Adequate tests were added if applicable
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-6022111559
@voonhous I've replied on each of your four inline threads (non-ASCII columns under schema-on-read, rewrite vs. drop on rename, MOR log files, ORC e2e) and left them open for you. Could you take another look and resolve them if the fixes look right? The branch is also merged with the latest master, so there are no conflicts now.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5784467233
non-blocking: once nested filters reach the reader again, they also flow through `ParquetSchemaEvolutionUtils.rebuildFilterFromParquet` when `hoodie.schema.on.read.enable` is set. That path calls `InternalSchemaUtils.reBuildFilterName`, which throws `IllegalArgumentException` if `querySchema.findIdByName(name)` misses, and Spark hands nested columns over as the dotted multipart name (`item.name`, backtick-quoted only if a part is not a plain identifier). Avro naming rules should keep Hudi field names quote-free so I expect the dotted form to resolve, but it would be good to add a leg to the new test with schema-on-read enabled and a renamed nested column, to confirm the filter is rewritten to the file-side name rather than dropped to `AlwaysTrue` or thrown.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #20029:
URL: https://github.com/apache/hudi/pull/20029#discussion_r4076537718
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +299,39 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val selectDF = spark.sql(s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5")
+
+ val fileScan = findFileScan(selectDF.queryExecution.executedPlan)
+ val planString = fileScan.toString()
+
+ assertTrue(fileScan.dataFilters.nonEmpty,
+ s"[$tableType] dataFilters should contain nested predicates but was empty")
+ assertFalse(planString.contains("PushedFilters: []"),
+ s"[$tableType] PushedFilters should not be empty — nested predicate pushdown is not working.\n" +
+ s"Verify that shortName() returns a lowercase value present in " +
+ s"spark.sql.optimizer.nestedPredicatePushdown.supportedFileSources.\nPlan:\n$planString")
+ assertTrue(planString.contains("PushedFilters: ["),
Review Comment:
I think this test passes on master without the fix: Spark infers `IsNotNull(item)` from `item.name = 'a1'` (GetStructField is null-intolerant), and that top-level filter is pushed regardless of the allowlist, so `PushedFilters` is never `[]` here. Could you assert on the nested predicates themselves instead, e.g. `fileScan.metadata("PushedFilters")` contains `EqualTo(item.name,a1)` and `GreaterThan(item.price,5)` (and, ideally, a negative leg with the extension's conf change undone showing they are absent)?
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -45,6 +49,8 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
extensions.injectOptimizerRule(ruleBuilder(_))
}
+ extensions.injectOptimizerRule(HoodieNestedPredicatePushdownConfig(_))
Review Comment:
This only helps sessions that load HoodieSparkSessionExtension; a plain `spark.read.format("hudi")` or Spark Connect/Thrift session without `spark.sql.extensions` still ends up with `hoodiefilegroup` against the default `parquet,orc` allowlist. Have you considered having `shortName()` return the base file format (`parquet` / `orc` via `tableConfig.getBaseFileFormat`) instead? That restores the 0.x / 1.0.x behaviour on every read path without touching session state, and ORC base files get the same treatment.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua merged PR #20029:
URL: https://github.com/apache/hudi/pull/20029
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5884340926
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1766c930421c273bcf9cbae7c452518fd127b414",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242",
"triggerID" : "1766c930421c273bcf9cbae7c452518fd127b414",
"triggerType" : "PUSH"
}, {
"hash" : "0c24ffdd3a5a8d3047e7903f751470888a9e5a83",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17346",
"triggerID" : "0c24ffdd3a5a8d3047e7903f751470888a9e5a83",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1766c930421c273bcf9cbae7c452518fd127b414 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242)
* 0c24ffdd3a5a8d3047e7903f751470888a9e5a83 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17346)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5785106000
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1766c930421c273bcf9cbae7c452518fd127b414",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242",
"triggerID" : "1766c930421c273bcf9cbae7c452518fd127b414",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1766c930421c273bcf9cbae7c452518fd127b414 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5884079537
Thanks @yihua for the detailed review. Agreed on all points; reworked in 0c24ffd:
- `shortName()` now returns the base file format short name (`parquet` / `orc` from the table's base file format; `parquet` for multiple base file formats), restoring the pre-#13632 behavior on every read path (DataFrame API, Spark Connect, Thrift) without touching session state.
- Dropped the `HoodieNestedPredicatePushdownConfig` optimizer rule entirely.
- Fixed the test to assert the nested predicates (`EqualTo(item.name,a1)`, `GreaterThan(item.price,5)`) appear in `PushedFilters` rather than relying on `PushedFilters` being non-empty (which the inferred `IsNotNull(item)` satisfies on master), with a negative leg (empty allowlist), plus a unit test pinning `shortName()` per base file format.
- Added a schema-on-read leg with a renamed nested column to verify the filter is rewritten to the file-side name.
Will backport to release-1.1.x / release-1.2.x once this lands.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-6020282168
@yihua @voonhous thanks for the reviews. I've addressed all the comments, please take another look.
- **yihua: assert on the nested predicates.** The tests now check that `EqualTo(item.name,a1)` and `GreaterThan(item.price,5)` show up in `PushedFilters`, with a negative leg (empty allowlist) showing they're absent. `shortName()` now returns the base file format instead of mutating session config (0c24ffd).
- **voonhous: non-ASCII columns under schema-on-read.** You were right. Once nested pushdown is on, Spark backtick-quotes non-identifier names (`` `名字` ``, ``item.`my-name` ``), and `rebuildFilterFromParquet` threw `cannot find filter col name`. The fix parses the name into its parts for the internal-schema lookup, then quotes the file-side name back the way `ParquetFilters` expects (08101e3). I added `WHERE` filters to `TestSpark3DDL` "Test Chinese table" plus a COW/MOR test with renamed non-ASCII top-level and nested columns. Both failed with that error before the fix.
- **voonhous: rewrite vs. drop on rename.** Added a nested case to `TestParquetSchemaEvolutionUtils` asserting `item.title` → `item.name`, not `AlwaysTrue`. It also covers quoted, dotted and hyphenated names.
- **voonhous: MOR without log files.** The MOR legs now assert log files exist. The pushdown test includes a log update that moves a record off the filtered value.
- **Also:** Vortex now reports its own `shortName` instead of falling through to `parquet` (f75f8b4). I fixed the description too: EXPLAIN's `Format` comes from `toString`, so it doesn't change.
I didn't add an ORC end-to-end test (your nit), per your note that it can wait.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #20029:
URL: https://github.com/apache/hudi/pull/20029#discussion_r4184207126
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val query = s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5"
+ // NOTE: Spark infers a top-level IsNotNull(item) from the nested predicates and pushes it
+ // regardless of the allowlist, so assert on the nested predicates themselves.
+ val nestedFilters = Seq("EqualTo(item.name,a1)", "GreaterThan(item.price,5)")
+
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertTrue(pushedFilters.contains(f), s"[$tableType] $f should be pushed down, got $pushedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+
+ // Negative leg: with an empty allowlist the nested predicates must not be pushed.
+ withSQLConf(SQLConf.NESTED_PREDICATE_PUSHDOWN_FILE_SOURCE_LIST.key -> "") {
+ val prunedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertFalse(prunedFilters.contains(f), s"[$tableType] $f should not be pushed down, got $prunedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+ }
+ }
+ }
+ }
+
+ test("Nested struct predicate pushdown with schema on read and a renamed nested column") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ withSQLConf("hoodie.schema.on.read.enable" -> "true") {
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+ spark.sql(s"ALTER TABLE $tableName RENAME COLUMN item.name TO title")
+
+ // The base file still stores item.name, so the pushed filter must be rewritten to the
+ // file-side name rather than dropped or failing the read.
+ val query = s"SELECT id FROM $tableName WHERE item.title = 'a1'"
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
Review Comment:
Can you please check, i think the SoR rename operation cannot tell `rewritten` from `dropped`.
Maybe we can add a nested case to `testRebuildFilterFromParquetRespellsFiltersOntoFileNames`, for example:
`EqualTo("item.title")` becomes `EqualTo("item.name")`.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
Review Comment:
Can't find a place to inline this, so will add this here.
`schema-on-read` filtering on a non-ASCII column will now throw. IIRC, `DataSourceStrategy` will backtick-quote non-ascii haracters.
We currently have tests for Mandarin characters, but it does not trigger the path that we need because the test below has no `WHERE` predicate filter.
`TestSpark3DDL "Test Chinese table"`
Let's add one and try to test it.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val query = s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5"
+ // NOTE: Spark infers a top-level IsNotNull(item) from the nested predicates and pushes it
+ // regardless of the allowlist, so assert on the nested predicates themselves.
+ val nestedFilters = Seq("EqualTo(item.name,a1)", "GreaterThan(item.price,5)")
+
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertTrue(pushedFilters.contains(f), s"[$tableType] $f should be pushed down, got $pushedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+
+ // Negative leg: with an empty allowlist the nested predicates must not be pushed.
+ withSQLConf(SQLConf.NESTED_PREDICATE_PUSHDOWN_FILE_SOURCE_LIST.key -> "") {
+ val prunedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertFalse(prunedFilters.contains(f), s"[$tableType] $f should not be pushed down, got $prunedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+ }
+ }
+ }
+ }
+
+ test("Nested struct predicate pushdown with schema on read and a renamed nested column") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
Review Comment:
We are testing MOR here but no LOG files are generated.
Can you please do an assertion check to ensure that there are log files for the MOR case?
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val query = s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5"
+ // NOTE: Spark infers a top-level IsNotNull(item) from the nested predicates and pushes it
+ // regardless of the allowlist, so assert on the nested predicates themselves.
+ val nestedFilters = Seq("EqualTo(item.name,a1)", "GreaterThan(item.price,5)")
+
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertTrue(pushedFilters.contains(f), s"[$tableType] $f should be pushed down, got $pushedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+
+ // Negative leg: with an empty allowlist the nested predicates must not be pushed.
+ withSQLConf(SQLConf.NESTED_PREDICATE_PUSHDOWN_FILE_SOURCE_LIST.key -> "") {
+ val prunedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertFalse(prunedFilters.contains(f), s"[$tableType] $f should not be pushed down, got $prunedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+ }
+ }
+ }
+ }
+
+ test("Nested struct predicate pushdown with schema on read and a renamed nested column") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ withSQLConf("hoodie.schema.on.read.enable" -> "true") {
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+ spark.sql(s"ALTER TABLE $tableName RENAME COLUMN item.name TO title")
+
+ // The base file still stores item.name, so the pushed filter must be rewritten to the
+ // file-side name rather than dropped or failing the read.
+ val query = s"SELECT id FROM $tableName WHERE item.title = 'a1'"
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ assertTrue(pushedFilters.contains("EqualTo(item.title,a1)"),
+ s"[$tableType] EqualTo(item.title,a1) should be pushed down, got $pushedFilters")
+ checkAnswer(query)(Seq(1))
+ checkAnswer(s"SELECT id FROM $tableName WHERE item.title = 'a2'")()
+ }
+ }
+ }
+ }
+
+ test("shortName matches the base file format") {
+ val unusedSchema = StructType.fromDDL("id int, ts long")
+ def formatFor(fileFormat: HoodieFileFormat, multipleBaseFileFormats: Boolean = false) =
+ new HoodieFileGroupReaderBasedFileFormat(
+ "unused", HoodieTableSchema(unusedSchema,
+ HoodieSchemaConversionUtils.convertStructTypeToHoodieSchema(unusedSchema, "unused", "hoodie.test")),
+ "unused", "000", Seq.empty, isMOR = false, isBootstrap = false, isIncremental = false,
+ "", shouldUseRecordPosition = false, Seq.empty, multipleBaseFileFormats, fileFormat)
+
+ assertEquals("parquet", formatFor(HoodieFileFormat.PARQUET).shortName())
+ assertEquals("orc", formatFor(HoodieFileFormat.ORC).shortName())
Review Comment:
Nit: I don't think we have a ORC end-to-end test (feel free to ignore). If it's not high priority, we can ignore it and focus on parquet only.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5783542888
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1766c930421c273bcf9cbae7c452518fd127b414",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "1766c930421c273bcf9cbae7c452518fd127b414",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1766c930421c273bcf9cbae7c452518fd127b414 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5784035259
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/20029?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 80.35%. Comparing base ([`319afce`](https://app.codecov.io/gh/apache/hudi/commit/319afce31cc3677abbb3a5b97a8d3add141ef534?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1766c93`](https://app.codecov.io/gh/apache/hudi/commit/1766c930421c273bcf9cbae7c452518fd127b414?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #20029 +/- ##
============================================
- Coverage 80.35% 80.35% -0.01%
+ Complexity 34781 34780 -1
============================================
Files 2545 2545
Lines 142646 142645 -1
Branches 17327 17327
============================================
- Hits 114623 114622 -1
Misses 20120 20120
Partials 7903 7903
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.86% <ø> (ø)` | |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.40% <ø> (ø)` | |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.69% <ø> (ø)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.79% <ø> (-0.01%)` | :arrow_down: |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.18% <ø> (ø)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.99% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.98% <ø> (ø)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.02% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.48% <ø> (ø)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.34% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/20029/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/20029/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/20029/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.85% <ø> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/20029?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...parquet/HoodieFileGroupReaderBasedFileFormat.scala](https://app.codecov.io/gh/apache/hudi/pull/20029?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fspark%2Fsql%2Fexecution%2Fdatasources%2Fparquet%2FHoodieFileGroupReaderBasedFileFormat.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmstY29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2V4ZWN1dGlvbi9kYXRhc291cmNlcy9wYXJxdWV0L0hvb2RpZUZpbGVHcm91cFJlYWRlckJhc2VkRmlsZUZvcm1hdC5zY2FsYQ==) | `84.24% <ø> (-0.06%)` | :arrow_down: |
| [...e/spark/sql/hudi/HoodieSparkSessionExtension.scala](https://app.codecov.io/gh/apache/hudi/pull/20029?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fspark%2Fsql%2Fhudi%2FHoodieSparkSessionExtension.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9zcGFyay9zcWwvaHVkaS9Ib29kaWVTcGFya1Nlc3Npb25FeHRlbnNpb24uc2NhbGE=) | `91.66% <ø> (ø)` | |
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5784221538
Suggested direction for this PR: **return the base file format's short name from `shortName()` (`parquet` / `orc` via `tableConfig.getBaseFileFormat`), drop the optimizer rule, and keep the test but assert on the nested predicates themselves.**
Why this is preferable to `hoodiefilegroup` + conf injection:
- **Covers every read path.** The conf injection only runs in sessions that load `HoodieSparkSessionExtension`. `spark.read.format("hudi")`, catalog reads without `spark.sql.extensions`, Spark Connect and Thrift Server deployments stay regressed. A base-format short name matches Spark's default `parquet,orc` allowlist everywhere, with zero user action.
- **No session-state mutation from the planner.** Optimizer rules are expected to be pure; `spark.conf.set` from inside a fixed-point batch is surprising, and the one-shot `applied` flag means a later `RESET` / `spark.conf.unset` switches pushdown off for the rest of the session.
- **Restores prior behaviour rather than inventing new.** 0.x (`Spark3xLegacyHoodieParquetFileFormat`) and 1.0.x (`HoodieFileGroupReaderBasedParquetFileFormat`) both inherited `parquet`; `HudiFileGroup` only arrived with HUDI-8746 (#13632). Users migrating from those versions get the same `Format: parquet` in EXPLAIN they had before.
- **ORC is handled by the same change.** Both `ParquetFilters` and `OrcFilters` resolve nested (quoted multipart) column names, and the allowlist only gates translation, so `parquet` / `orc` per base format is enough; for multi-format tables returning `parquet` is fine for the same reason.
- **One thing to keep in sync instead of two.** With `hoodiefilegroup` the short name and the injected conf value must agree forever; with the base-format name there is one string, and a small unit test pinning `shortName()` per base format prevents the next accidental rename.
- **No change to the other Spark gates.** `SchemaPruning` and `PushVariantIntoScan` match on `isInstanceOf[ParquetFileFormat]`, which is unaffected either way.
The cost is losing the Hudi-specific string in the scan node's `Format`; `HoodieFileIndex` in the same node still identifies the table as Hudi, so I think that is acceptable.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-6027669824
@voonhous, I'm going to merge this PR in so @prashantwason can start cherry-picking on top of the minor release. Feel free to raise additional comments if there is any, which will be followed up in a separate PR.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on code in PR #20029:
URL: https://github.com/apache/hudi/pull/20029#discussion_r4197642891
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
Review Comment:
You were right: with nested pushdown on, Spark backtick-quotes non-identifier names (```名字```, ``item.`my-name```), and `rebuildFilterFromParquet` threw `cannot find filter col name`. Fixed in 08101e3: the name is parsed into its parts for the internal-schema lookup, and the file-side name is quoted back the way `ParquetFilters` keys its columns. I added `WHERE` filters to `TestSpark3DDL` "Test Chinese table", plus a COW/MOR test with renamed non-ASCII top-level and nested columns. Both failed with that error before the fix.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val query = s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5"
+ // NOTE: Spark infers a top-level IsNotNull(item) from the nested predicates and pushes it
+ // regardless of the allowlist, so assert on the nested predicates themselves.
+ val nestedFilters = Seq("EqualTo(item.name,a1)", "GreaterThan(item.price,5)")
+
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertTrue(pushedFilters.contains(f), s"[$tableType] $f should be pushed down, got $pushedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+
+ // Negative leg: with an empty allowlist the nested predicates must not be pushed.
+ withSQLConf(SQLConf.NESTED_PREDICATE_PUSHDOWN_FILE_SOURCE_LIST.key -> "") {
+ val prunedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertFalse(prunedFilters.contains(f), s"[$tableType] $f should not be pushed down, got $prunedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+ }
+ }
+ }
+ }
+
+ test("Nested struct predicate pushdown with schema on read and a renamed nested column") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ withSQLConf("hoodie.schema.on.read.enable" -> "true") {
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+ spark.sql(s"ALTER TABLE $tableName RENAME COLUMN item.name TO title")
+
+ // The base file still stores item.name, so the pushed filter must be rewritten to the
+ // file-side name rather than dropped or failing the read.
+ val query = s"SELECT id FROM $tableName WHERE item.title = 'a1'"
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
Review Comment:
Agreed, the e2e test couldn't tell those apart. Added `testRebuildFilterFromParquetRespellsNestedFilters` to `TestParquetSchemaEvolutionUtils`: it asserts `EqualTo("item.title")` becomes `EqualTo("item.name")` and a nested column the file lacks becomes `AlwaysTrue`. A sibling test covers quoted, dotted and hyphenated names (08101e3).
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val query = s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5"
+ // NOTE: Spark infers a top-level IsNotNull(item) from the nested predicates and pushes it
+ // regardless of the allowlist, so assert on the nested predicates themselves.
+ val nestedFilters = Seq("EqualTo(item.name,a1)", "GreaterThan(item.price,5)")
+
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertTrue(pushedFilters.contains(f), s"[$tableType] $f should be pushed down, got $pushedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+
+ // Negative leg: with an empty allowlist the nested predicates must not be pushed.
+ withSQLConf(SQLConf.NESTED_PREDICATE_PUSHDOWN_FILE_SOURCE_LIST.key -> "") {
+ val prunedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertFalse(prunedFilters.contains(f), s"[$tableType] $f should not be pushed down, got $prunedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+ }
+ }
+ }
+ }
+
+ test("Nested struct predicate pushdown with schema on read and a renamed nested column") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
Review Comment:
Done (08101e3). The MOR legs now assert log files exist. The pushdown test does an update that moves id 1 off the filtered value (base matches, log doesn't); the rename tests merge a log written under the new name over a base file under the old one.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +301,84 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val query = s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5"
+ // NOTE: Spark infers a top-level IsNotNull(item) from the nested predicates and pushes it
+ // regardless of the allowlist, so assert on the nested predicates themselves.
+ val nestedFilters = Seq("EqualTo(item.name,a1)", "GreaterThan(item.price,5)")
+
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertTrue(pushedFilters.contains(f), s"[$tableType] $f should be pushed down, got $pushedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+
+ // Negative leg: with an empty allowlist the nested predicates must not be pushed.
+ withSQLConf(SQLConf.NESTED_PREDICATE_PUSHDOWN_FILE_SOURCE_LIST.key -> "") {
+ val prunedFilters = pushedFiltersOf(spark.sql(query))
+ nestedFilters.foreach { f =>
+ assertFalse(prunedFilters.contains(f), s"[$tableType] $f should not be pushed down, got $prunedFilters")
+ }
+ checkAnswer(query)(Seq(1))
+ }
+ }
+ }
+ }
+
+ test("Nested struct predicate pushdown with schema on read and a renamed nested column") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ withSQLConf("hoodie.schema.on.read.enable" -> "true") {
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+ spark.sql(s"ALTER TABLE $tableName RENAME COLUMN item.name TO title")
+
+ // The base file still stores item.name, so the pushed filter must be rewritten to the
+ // file-side name rather than dropped or failing the read.
+ val query = s"SELECT id FROM $tableName WHERE item.title = 'a1'"
+ val pushedFilters = pushedFiltersOf(spark.sql(query))
+ assertTrue(pushedFilters.contains("EqualTo(item.title,a1)"),
+ s"[$tableType] EqualTo(item.title,a1) should be pushed down, got $pushedFilters")
+ checkAnswer(query)(Seq(1))
+ checkAnswer(s"SELECT id FROM $tableName WHERE item.title = 'a2'")()
+ }
+ }
+ }
+ }
+
+ test("shortName matches the base file format") {
+ val unusedSchema = StructType.fromDDL("id int, ts long")
+ def formatFor(fileFormat: HoodieFileFormat, multipleBaseFileFormats: Boolean = false) =
+ new HoodieFileGroupReaderBasedFileFormat(
+ "unused", HoodieTableSchema(unusedSchema,
+ HoodieSchemaConversionUtils.convertStructTypeToHoodieSchema(unusedSchema, "unused", "hoodie.test")),
+ "unused", "000", Seq.empty, isMOR = false, isBootstrap = false, isIncremental = false,
+ "", shouldUseRecordPosition = false, Seq.empty, multipleBaseFileFormats, fileFormat)
+
+ assertEquals("parquet", formatFor(HoodieFileFormat.PARQUET).shortName())
+ assertEquals("orc", formatFor(HoodieFileFormat.ORC).shortName())
Review Comment:
Leaving ORC e2e out of this PR as you suggested. The `shortName()` unit test pins `orc` for ORC tables, and `OrcFilters` resolves nested names natively.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on code in PR #20029:
URL: https://github.com/apache/hudi/pull/20029#discussion_r4197639422
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -57,3 +63,26 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
sparkAdapter.injectPlannerStrategies(extensions)
}
}
+
+/**
+ * Ensures that Spark's nested predicate pushdown allowlist includes "hoodiefilegroup"
+ * so that nested struct field predicates (e.g., msg.field = 'value') are pushed down
+ * to the Parquet reader when using HoodieFileGroupReaderBasedFileFormat.
+ */
+case class HoodieNestedPredicatePushdownConfig(spark: SparkSession) extends Rule[LogicalPlan] with Logging {
Review Comment:
Outdated: the session-extension approach is gone. `shortName()` now returns the base file format's name (`parquet` / `orc`), so the default allowlist matches on every read path, including plain `spark.read.format("hudi")`, with no config mutation (0c24ffd).
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -57,3 +63,26 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
sparkAdapter.injectPlannerStrategies(extensions)
}
}
+
+/**
+ * Ensures that Spark's nested predicate pushdown allowlist includes "hoodiefilegroup"
+ * so that nested struct field predicates (e.g., msg.field = 'value') are pushed down
+ * to the Parquet reader when using HoodieFileGroupReaderBasedFileFormat.
+ */
+case class HoodieNestedPredicatePushdownConfig(spark: SparkSession) extends Rule[LogicalPlan] with Logging {
+ @volatile private var applied = false
+
+ override def apply(plan: LogicalPlan): LogicalPlan = {
+ if (!applied) {
+ val key = "spark.sql.optimizer.nestedPredicatePushdown.supportedFileSources"
+ val current = spark.conf.get(key, "parquet,orc")
+ if (!current.toLowerCase.contains("hoodiefilegroup")) {
Review Comment:
Outdated: nothing touches `spark.sql.optimizer.nestedPredicatePushdown.supportedFileSources` any more (0c24ffd). A user who empties it to turn nested pushdown off now gets exactly that, and the negative leg in `TestNestedSchemaPruningOptimization` covers it.
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -57,3 +63,26 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
sparkAdapter.injectPlannerStrategies(extensions)
}
}
+
+/**
+ * Ensures that Spark's nested predicate pushdown allowlist includes "hoodiefilegroup"
+ * so that nested struct field predicates (e.g., msg.field = 'value') are pushed down
+ * to the Parquet reader when using HoodieFileGroupReaderBasedFileFormat.
+ */
+case class HoodieNestedPredicatePushdownConfig(spark: SparkSession) extends Rule[LogicalPlan] with Logging {
+ @volatile private var applied = false
+
+ override def apply(plan: LogicalPlan): LogicalPlan = {
Review Comment:
Outdated: the rule and the `@volatile applied` flag were removed. `shortName()` returning the base file format replaces them (0c24ffd).
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestNestedSchemaPruningOptimization.scala:
##########
@@ -299,6 +299,39 @@ class TestNestedSchemaPruningOptimization extends HoodieSparkSqlTestBase {
}
}
+ test("Nested struct predicate pushdown") {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val tableName = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$tableName"
+
+ createTableWithNestedStructSchema(tableType, tableName, tablePath)
+
+ val selectDF = spark.sql(s"SELECT id FROM $tableName WHERE item.name = 'a1' AND item.price > 5")
+
+ val fileScan = findFileScan(selectDF.queryExecution.executedPlan)
+ val planString = fileScan.toString()
+
+ assertTrue(fileScan.dataFilters.nonEmpty,
+ s"[$tableType] dataFilters should contain nested predicates but was empty")
+ assertFalse(planString.contains("PushedFilters: []"),
+ s"[$tableType] PushedFilters should not be empty — nested predicate pushdown is not working.\n" +
+ s"Verify that shortName() returns a lowercase value present in " +
+ s"spark.sql.optimizer.nestedPredicatePushdown.supportedFileSources.\nPlan:\n$planString")
+ assertTrue(planString.contains("PushedFilters: ["),
Review Comment:
Good catch, thanks. The test now asserts on the nested predicates themselves (`EqualTo(item.name,a1)`, `GreaterThan(item.price,5)`) in `PushedFilters`, with a negative leg (empty allowlist) showing they're absent. The MOR leg also merges a log file now (08101e3).
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -45,6 +49,8 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
extensions.injectOptimizerRule(ruleBuilder(_))
}
+ extensions.injectOptimizerRule(HoodieNestedPredicatePushdownConfig(_))
Review Comment:
Agreed, done in 0c24ffd: `shortName()` returns the base file format (`parquet` / `orc`; multi-format tables report `parquet`), restoring the 0.x / 1.0.x behaviour on every read path. Lance and Vortex report their own names, so they stay off the allowlist (f75f8b4).
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5783644498
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1766c930421c273bcf9cbae7c452518fd127b414",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242",
"triggerID" : "1766c930421c273bcf9cbae7c452518fd127b414",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1766c930421c273bcf9cbae7c452518fd127b414 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #20029:
URL: https://github.com/apache/hudi/pull/20029#discussion_r4076089508
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -57,3 +63,26 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
sparkAdapter.injectPlannerStrategies(extensions)
}
}
+
+/**
+ * Ensures that Spark's nested predicate pushdown allowlist includes "hoodiefilegroup"
+ * so that nested struct field predicates (e.g., msg.field = 'value') are pushed down
+ * to the Parquet reader when using HoodieFileGroupReaderBasedFileFormat.
+ */
+case class HoodieNestedPredicatePushdownConfig(spark: SparkSession) extends Rule[LogicalPlan] with Logging {
+ @volatile private var applied = false
+
+ override def apply(plan: LogicalPlan): LogicalPlan = {
+ if (!applied) {
+ val key = "spark.sql.optimizer.nestedPredicatePushdown.supportedFileSources"
+ val current = spark.conf.get(key, "parquet,orc")
+ if (!current.toLowerCase.contains("hoodiefilegroup")) {
Review Comment:
🤖 If a user has explicitly set this config to empty to turn nested pushdown off (a known escape hatch for Parquet nested-filter issues), this appends and silently re-enables it for Hudi — is that intended? And once `applied` is set, a later `RESET`/`SET` by the user is never reconciled. Minor: `spark.conf.get(key)` returns the registered entry's default, so hard-coding `"parquet,orc"` isn't needed.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -57,3 +63,26 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
sparkAdapter.injectPlannerStrategies(extensions)
}
}
+
+/**
+ * Ensures that Spark's nested predicate pushdown allowlist includes "hoodiefilegroup"
+ * so that nested struct field predicates (e.g., msg.field = 'value') are pushed down
+ * to the Parquet reader when using HoodieFileGroupReaderBasedFileFormat.
+ */
+case class HoodieNestedPredicatePushdownConfig(spark: SparkSession) extends Rule[LogicalPlan] with Logging {
+ @volatile private var applied = false
+
+ override def apply(plan: LogicalPlan): LogicalPlan = {
Review Comment:
🤖 nit: this mutates a session-wide, comma-separated SQLConf string from inside an optimizer rule, using an `@volatile applied` flag to only run once — that's a fairly surprising place to do config setup. Might be worth moving this to `HoodieSparkSessionExtension.apply` itself (setting the conf once at extension-injection time) rather than as a `Rule[LogicalPlan]` that runs on every plan until it flips a flag.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/HoodieSparkSessionExtension.scala:
##########
@@ -57,3 +63,26 @@ class HoodieSparkSessionExtension extends (SparkSessionExtensions => Unit)
sparkAdapter.injectPlannerStrategies(extensions)
}
}
+
+/**
+ * Ensures that Spark's nested predicate pushdown allowlist includes "hoodiefilegroup"
+ * so that nested struct field predicates (e.g., msg.field = 'value') are pushed down
+ * to the Parquet reader when using HoodieFileGroupReaderBasedFileFormat.
+ */
+case class HoodieNestedPredicatePushdownConfig(spark: SparkSession) extends Rule[LogicalPlan] with Logging {
Review Comment:
🤖 This only takes effect when `HoodieSparkSessionExtension` is configured, so sessions using just `spark.read.format("hudi")` (no `spark.sql.extensions`) still get `PushedFilters: []`. Since the class extends `ParquetFileFormat` (inherited `shortName` = `parquet`, already on Spark's default allowlist) and nothing in the repo references the name, would simply removing the `shortName()` override work — that was the pre-#13632 behavior and needs no config mutation? Alternatively, `DefaultSource`'s existing `SparkSession.getActiveSession.foreach` block runs for both SQL and DataFrame entry points if you'd rather keep a distinct name.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5783990844
For context: the `shortName()` override to `"HudiFileGroup"` came in with HUDI-8746 (#13632), so this regression affects 1.1.0, 1.1.1, 1.2.0 and the 1.2.1 RC on every Spark version (1.0.x is unaffected since `HoodieFileGroupReaderBasedParquetFileFormat` still inherited `"parquet"`). Worth backporting the fix to release-1.1.x and release-1.2.x once it lands.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5884268892
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1766c930421c273bcf9cbae7c452518fd127b414",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242",
"triggerID" : "1766c930421c273bcf9cbae7c452518fd127b414",
"triggerType" : "PUSH"
}, {
"hash" : "0c24ffdd3a5a8d3047e7903f751470888a9e5a83",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "0c24ffdd3a5a8d3047e7903f751470888a9e5a83",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1766c930421c273bcf9cbae7c452518fd127b414 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242)
* 0c24ffdd3a5a8d3047e7903f751470888a9e5a83 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5885625133
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1766c930421c273bcf9cbae7c452518fd127b414",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17242",
"triggerID" : "1766c930421c273bcf9cbae7c452518fd127b414",
"triggerType" : "PUSH"
}, {
"hash" : "0c24ffdd3a5a8d3047e7903f751470888a9e5a83",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17346",
"triggerID" : "0c24ffdd3a5a8d3047e7903f751470888a9e5a83",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0c24ffdd3a5a8d3047e7903f751470888a9e5a83 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17346)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(Spark): Fix nested struct predicate pushdown for HoodieFileGroupReaderBasedFileFormat [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5994964130
@prashantwason Can you please help to address the comments above? Thanks.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org