You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "yihua (via GitHub)" <gi...@apache.org> on 2026/10/09 01:37:51 UTC
[PR] fix(spark): skip the catalog lookup in HoodieFileIndex unless the read declares a bucket index [hudi]
yihua opened a new pull request, #20240:
URL: https://github.com/apache/hudi/pull/20240
### Describe the issue this Pull Request addresses
issue: #20235, part of #16933
`HoodieFileIndex.getConfigProperties` rebuilds a catalog identifier from `hoodie.table.name` plus `hoodie.database.name`, falling back to the session's current database, and calls `spark.catalog.tableExists` on it to pick up the simple bucket index TBLPROPERTIES. When the table records no database name that guess is usually wrong: a catalog that rejects the session database fails every qualified read, and a same-named table in the session database can hand its bucket count to a different table, which makes bucket pruning drop rows silently.
### Summary and Changelog
The catalog is consulted only when the read already declares `hoodie.index.type=BUCKET` through the datasource options or the session conf. A read with no bucket index declared never calls the catalog. When the lookup does run it is best effort: a catalog error is logged at WARN and the merge is skipped, so a session-wide bucket index setting cannot fail a read either. A declared bucket read against a healthy catalog still merges the entry's properties, and the schema-on-read path still carries TBLPROPERTIES as options.
`TestHoodieFileIndex` gains three cases: a read without a declared bucket index never calls `tableExists` even when the catalog would throw (fails before this change with the thrown exception), a read that declares a bucket index still merges the entry's bucket count, and a declared bucket read whose catalog throws proceeds without the merge.
This is the narrow alternative to #20236. The longer-term fix is to persist the bucket index definition in the table (#14425) and drop both the catalog lookup and the database-name guess (#16933).
### Impact
Reads that do not declare a bucket index stop touching the catalog. A catalog read of a simple bucket index table now needs `hoodie.index.type=BUCKET` on the read or in the session conf to keep bucket pruning, unless the options already carry it.
### Risk Level
low. The behavior change is the condition on the catalog lookup and treating its failure as a skipped merge.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] 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): skip the catalog lookup in HoodieFileIndex unless the read declares a bucket index [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua merged PR #20240:
URL: https://github.com/apache/hudi/pull/20240
--
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): skip the catalog lookup in HoodieFileIndex unless the read declares a bucket index [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #20240:
URL: https://github.com/apache/hudi/pull/20240#discussion_r4225902690
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieFileIndex.scala:
##########
@@ -655,14 +657,20 @@ object HoodieFileIndex extends Logging {
properties.setProperty(PARTITIONPATH_FIELD.key, HoodieTableConfig.getPartitionFieldPropForKeyGenerator(tableConfig).orElse(""))
properties.setProperty(HoodieTableConfig.PARTITION_EXTRACTOR_CLASS.key(), tableConfig.getPartitionExtractorClass.orElse(""))
- // for simple bucket index, we need to set the INDEX_TYPE, BUCKET_INDEX_HASH_FIELD, BUCKET_INDEX_NUM_BUCKETS
- val database = getDatabaseName(tableConfig, spark.catalog.currentDatabase)
- val tableName = tableConfig.getTableName
-
- if (spark.catalog.tableExists(database, tableName)) {
- val tableIdentifier = TableIdentifier(tableName, Some(database))
- val table = HoodieCatalogTable(spark, tableIdentifier)
- table.catalogProperties.foreach(kv => properties.setProperty(kv._1, kv._2))
+ // The catalog entry only completes the simple bucket index settings (hash field and bucket count), which
+ // hoodie.properties does not record, so it is consulted only when the read declares a bucket index.
+ if (isBucketIndexDeclared(properties)) {
+ val database = getDatabaseName(tableConfig, spark.catalog.currentDatabase)
+ val tableName = tableConfig.getTableName
+ try {
+ if (spark.catalog.tableExists(database, tableName)) {
+ val table = HoodieCatalogTable(spark, TableIdentifier(tableName, Some(database)))
+ table.catalogProperties.foreach(kv => properties.setProperty(kv._1, kv._2))
Review Comment:
Done, the merge now copies only the bucket index options (index type, engine type, hash field, bucket count, query pruning flag) from the catalog entry.
--
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): skip the catalog lookup in HoodieFileIndex unless the read declares a bucket index [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20240:
URL: https://github.com/apache/hudi/pull/20240#issuecomment-6072856731
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/20240?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `55.55556%` with `4 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 34.47%. Comparing base ([`4cfba4c`](https://app.codecov.io/gh/apache/hudi/commit/4cfba4ca4079c4abd09348f0826374db78c8ab35?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7a49b1a`](https://app.codecov.io/gh/apache/hudi/commit/7a49b1a4ea821286cc84fb77f80c9fd186483961?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/20240?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...c/main/scala/org/apache/hudi/HoodieFileIndex.scala](https://app.codecov.io/gh/apache/hudi/pull/20240?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fhudi%2FHoodieFileIndex.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmstY29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvaHVkaS9Ib29kaWVGaWxlSW5kZXguc2NhbGE=) | 55.55% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/hudi/pull/20240?src=pr&el=tree&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 #20240 +/- ##
=============================================
- Coverage 80.62% 34.47% -46.16%
+ Complexity 35057 15391 -19666
=============================================
Files 2552 2552
Lines 143390 143393 +3
Branches 17450 17452 +2
=============================================
- Hits 115607 49429 -66178
- Misses 19862 89103 +69241
+ Partials 7921 4861 -3060
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/20240/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/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.07% <ø> (-12.10%)` | :arrow_down: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `35.65% <ø> (-48.15%)` | :arrow_down: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-85.98%)` | :arrow_down: |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.12% <55.55%> (-58.94%)` | :arrow_down: |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `1.40% <ø> (-76.78%)` | :arrow_down: |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.28% <ø> (-55.52%)` | :arrow_down: |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.78% <ø> (-5.34%)` | :arrow_down: |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `6.57% <ø> (-69.69%)` | :arrow_down: |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.13% <ø> (-2.40%)` | :arrow_down: |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.17% <ø> (-11.89%)` | :arrow_down: |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.82% <ø> (-76.18%)` | :arrow_down: |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/20240/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-53.21%)` | :arrow_down: |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/20240/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/20240/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.96% <ø> (-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/20240?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...c/main/scala/org/apache/hudi/HoodieFileIndex.scala](https://app.codecov.io/gh/apache/hudi/pull/20240?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fhudi%2FHoodieFileIndex.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmstY29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvaHVkaS9Ib29kaWVGaWxlSW5kZXguc2NhbGE=) | `44.70% <55.55%> (-38.05%)` | :arrow_down: |
... and [1826 files with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/20240/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</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): skip the catalog lookup in HoodieFileIndex unless the read declares a bucket index [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #20240:
URL: https://github.com/apache/hudi/pull/20240#discussion_r4225866962
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieFileIndex.scala:
##########
@@ -655,14 +657,20 @@ object HoodieFileIndex extends Logging {
properties.setProperty(PARTITIONPATH_FIELD.key, HoodieTableConfig.getPartitionFieldPropForKeyGenerator(tableConfig).orElse(""))
properties.setProperty(HoodieTableConfig.PARTITION_EXTRACTOR_CLASS.key(), tableConfig.getPartitionExtractorClass.orElse(""))
- // for simple bucket index, we need to set the INDEX_TYPE, BUCKET_INDEX_HASH_FIELD, BUCKET_INDEX_NUM_BUCKETS
- val database = getDatabaseName(tableConfig, spark.catalog.currentDatabase)
- val tableName = tableConfig.getTableName
-
- if (spark.catalog.tableExists(database, tableName)) {
- val tableIdentifier = TableIdentifier(tableName, Some(database))
- val table = HoodieCatalogTable(spark, tableIdentifier)
- table.catalogProperties.foreach(kv => properties.setProperty(kv._1, kv._2))
+ // The catalog entry only completes the simple bucket index settings (hash field and bucket count), which
+ // hoodie.properties does not record, so it is consulted only when the read declares a bucket index.
+ if (isBucketIndexDeclared(properties)) {
+ val database = getDatabaseName(tableConfig, spark.catalog.currentDatabase)
+ val tableName = tableConfig.getTableName
+ try {
+ if (spark.catalog.tableExists(database, tableName)) {
+ val table = HoodieCatalogTable(spark, TableIdentifier(tableName, Some(database)))
+ table.catalogProperties.foreach(kv => properties.setProperty(kv._1, kv._2))
Review Comment:
we can just setup the bucket index specific options to narrow down the impact.
--
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