You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "databird07 (via GitHub)" <gi...@apache.org> on 2026/10/16 04:10:14 UTC
[PR] test(spark): share column-stats tables across read-only cases and write real table versions in TestColumnStatsIndexWithSQL [hudi]
databird07 opened a new pull request, #20361:
URL: https://github.com/apache/hudi/pull/20361
### Describe the issue this Pull Request addresses
`TestColumnStatsIndexWithSQL` (82 invocations, ~816s in CI) rebuilt the same 3-commit table for every parameter case and then only ran SQL and data-skipping assertions against it. The parameter case also carries `tableVersion` (6/8/9/10), but the class never put it into the write options, so every case wrote a v10 table.
### Summary and Changelog
- Writes now set `WRITE_TABLE_VERSION`, `HoodieTableConfig.VERSION` and the timeline layout from the case's table version, and every write asserts that the table has the requested version.
- Read-only methods share one cached table per recipe and write options. Each version gets its own table. Mutating methods get a private copy.
- Isolation guards run after each test. They check files (path, size, mtime), completed instants and MDT instants, a fresh SparkSession, and SQLConf/view cleanup. A changed table is evicted, and only the offending test fails.
- All 82 invocations still run, and nothing is skipped or loosened.
### Impact
Test-only. Same-window local median 681s -> 555s (ratio 0.82). Lines covered in every run went from 28,209 to 30,174, and none were lost. The new lines are mostly older-version timeline, MDT writer and log-format paths. 20/20 runs passed, 10 of them in random method order. Measured locally on JDK 17 with 4 pinned CPUs and JaCoCo on; full numbers are in the commit message.
### Risk Level
low. Test-only. Shared state is guarded and checked after every test.
### 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] test(spark): share column-stats tables across read-only cases and write real table versions in TestColumnStatsIndexWithSQL [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20361:
URL: https://github.com/apache/hudi/pull/20361#issuecomment-6208692706
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/20361?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.63%. Comparing base ([`e9da226`](https://app.codecov.io/gh/apache/hudi/commit/e9da226db3a90747cc51d13fec56ebd1704c1699?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7f7f7f4`](https://app.codecov.io/gh/apache/hudi/commit/7f7f7f4d470f15e1ca536714a139eeb8a821a755?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 18 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #20361 +/- ##
============================================
- Coverage 80.70% 80.63% -0.07%
- Complexity 35124 35218 +94
============================================
Files 2553 2553
Lines 143524 144052 +528
Branches 17474 17569 +95
============================================
+ Hits 115830 116157 +327
- Misses 19746 19911 +165
- Partials 7948 7984 +36
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/20361/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/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.90% <ø> (-0.25%)` | :arrow_down: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.94% <ø> (ø)` | |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.06% <ø> (ø)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.18% <ø> (ø)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.17% <ø> (ø)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.80% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.91% <ø> (-0.25%)` | :arrow_down: |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.58% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.88% <ø> (+0.35%)` | :arrow_up: |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.41% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/20361/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/20361/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `54.48% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/20361/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/20361/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.94% <ø> (+<0.01%)` | :arrow_up: |
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.
[see 9 files with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/20361/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] test(spark): share column-stats tables across read-only cases and write real table versions in TestColumnStatsIndexWithSQL [hudi]
Posted by "databird07 (via GitHub)" <gi...@apache.org>.
databird07 commented on PR #20361:
URL: https://github.com/apache/hudi/pull/20361#issuecomment-6208216386
Combined into #20364 so the total impact can be reviewed in one place; this commit is included there unchanged. Closing.
--
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] test(spark): share column-stats tables across read-only cases and write real table versions in TestColumnStatsIndexWithSQL [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #20361:
URL: https://github.com/apache/hudi/pull/20361#discussion_r4290877670
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestColumnStatsIndexWithSQL.scala:
##########
@@ -42,17 +43,133 @@ import org.apache.spark.sql.catalyst.expressions.{And, AttributeReference, Bitwi
import org.apache.spark.sql.catalyst.expressions.Literal.TrueLiteral
import org.apache.spark.sql.hudi.DataSkippingUtils
import org.apache.spark.sql.types.StringType
-import org.junit.jupiter.api.{Tag, Test}
+import org.junit.jupiter.api.{AfterEach, BeforeEach, Tag, Test}
import org.junit.jupiter.api.Assertions.{assertEquals, assertFalse, assertTrue}
+import org.junit.jupiter.api.extension.{AfterAllCallback, ExtendWith, ExtensionContext}
+import org.junit.jupiter.api.parallel.{Execution, ExecutionMode}
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import java.io.File
+import java.lang.ref.WeakReference
+import java.nio.file.{Files, Path, Paths, StandardCopyOption}
import scala.collection.JavaConverters._
+import scala.collection.mutable
@Tag("functional-b")
+@ExtendWith(Array(classOf[TestColumnStatsIndexWithSQL.SharedTablesCleanup]))
+// the shared table cache in the companion object is not thread-safe
+@Execution(ExecutionMode.SAME_THREAD)
class TestColumnStatsIndexWithSQL extends ColumnStatIndexTestBase {
+ import TestColumnStatsIndexWithSQL._
+
+ // Shared tables this test reads or copies; each one is checked for changes after the test.
+ private var sharedTablesInUse: Seq[SharedTable] = Seq()
+ private var initialSqlConf: Map[String, String] = Map()
+
+ @BeforeEach
+ override def setUp(): Unit = {
+ super.setUp()
+ sharedTablesInUse = Seq()
+ // Every test must start from a fresh SparkSession: no SQL conf, temp views or cached plans
+ // left over from a previous test that read the same shared table.
+ assertTrue(lastSparkSession.get() ne spark, "SparkSession reused across tests")
+ lastSparkSession = new WeakReference(spark)
+ assertFalse(spark.catalog.tableExists("tbl"))
+ initialSqlConf = spark.conf.getAll
+ }
+
+ @AfterEach
+ override def tearDown(): Unit = {
+ try {
+ if (spark != null) {
+ spark.catalog.dropTempView("tbl")
+ spark.catalog.clearCache()
+ val sqlConf = spark.sessionState.conf
+ val currentConf = sqlConf.getAllConfs
+ (currentConf.keySet -- initialSqlConf.keySet).foreach(sqlConf.unsetConf)
+ initialSqlConf.foreach { case (k, v) => if (!currentConf.get(k).contains(v)) sqlConf.setConfString(k, v) }
+ }
+ // A test must never change a shared table (data files, timeline or metadata table timeline);
+ // tests that write work on their own copy.
+ sharedTablesInUse.foreach { table =>
+ val state = captureTableState(table.path)
+ if (state != table.state) {
+ // evict it so that later tests rebuild a clean table and only this test fails
+ sharedTables.retain((_, cached) => cached ne table)
+ }
+ assertEquals(table.state, state, s"Shared table ${table.path} was modified by the test")
+ }
+ } finally {
+ super.tearDown()
+ }
+ }
+
+ /**
+ * Points the test at a table built by `build`, building it only the first time a test asks for it.
+ * Tables are keyed by `recipe` plus the exact write options, so parameter cases that write the
+ * same data with the same options (e.g. ones that only differ in how the index is read) share one table.
+ * `recipe` must name every other input of `build` that affects the table or its validation.
+ * With `copy = true` the test gets a private copy it may write to.
+ */
Review Comment:
🤖 nit: could you replace the free-form `recipe` strings (e.g. "setupTable(shouldValidate = false)" repeated across tests) with shared constants or derive the key from the setupTable arguments? A typo or a drifting copy would silently give two tests different tables, or worse, the wrong shared one.
<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] test(spark): share column-stats tables across read-only cases and write real table versions in TestColumnStatsIndexWithSQL [hudi]
Posted by "databird07 (via GitHub)" <gi...@apache.org>.
databird07 closed pull request #20361: test(spark): share column-stats tables across read-only cases and write real table versions in TestColumnStatsIndexWithSQL
URL: https://github.com/apache/hudi/pull/20361
--
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