[CH] Decide the fate of the ClickHouse Iceberg Java tests, which no build compiles today
- Dominant language
- Scala
- Stars
- 1.6k
- Forks
- 657
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 80
Description
### Where things stand
`backends-clickhouse` has two Iceberg Java tests, `TestPositionDeletesTableGluten` and `TestFlinkUpsert`. #9996 introduced the versioned source layout, put both in `src-iceberg-spark33/test/java`, and added a JUnit 5 port of `TestPositionDeletesTableGluten` under `src-iceberg-spark35/test/java` without porting `TestFlinkUpsert`. #12902 removes Spark 3.3 and with it the two 3.3 copies, leaving the 3.5 `TestPositionDeletesTableGluten` as the only survivor.
Neither test has been compiled since #9996. Two independent reasons:
1. `src-iceberg-spark/test/{scala,java}` is added only by the `iceberg-test` profile, and no ClickHouse job passes it. The Jenkins command passes `-Piceberg` and `-Dsurefire.skipTests=false` but not `-Piceberg-test`.
2. Even when it is passed, the build fails, because `iceberg` and `iceberg-test` collide on one build-helper execution id and the collision drops `src-iceberg/test/scala`, where `ClickHouseIcebergHiveTableSupport` lives. Filed as #12933.
The ClickHouse Jenkins job also still hardcodes `-Pspark-3.3`, so it needs an update of its own before any of this runs.
Meanwhile the cost is not zero: the `iceberg` profile in `backends-clickhouse/pom.xml` carries nine Flink artifacts (three `iceberg-flink-*` plus `flink-streaming-java`, `flink-table-api-bridge-base`, `flink-table-api-java`, `flink-table-api-java-bridge`, `flink-test-utils`, `flink-table-planner_2.12`), resolved by every `-Piceberg` ClickHouse build for a test that never compiles. After #12902 no source file in the module references `org.apache.flink` at all.
### Option 1: keep them
In this order, because each step depends on the previous one:
1. Fix the execution-id collision, #12933.
2. Add `-Piceberg-test` to the ClickHouse job, whose `-Pspark-3.3` also has to go.
3. Port `TestFlinkUpsert` to `src-iceberg-spark35`. This is smaller than it looks. The file is already JUnit 5 (`@TestTemplate`, `org.apache.iceberg.Parameters`) and extends `CatalogTestBase`, which `iceberg-flink-1.19-1.10.0-tests.jar` still ships. The only unresolved symbol is `MiniClusterResource`, used once for `DISABLE_CLASSLOADER_CHECK_CONFIG`; `MiniFlinkClusterExtension` replaces it.
4. Run both tests and fix what the run turns up. Nobody has seen either of them execute, so this step is the one with unknown cost.
### Option 2: drop them
Delete `src-iceberg-spark35/test/java/.../TestPositionDeletesTableGluten.java` and the nine Flink dependencies. `TestFlinkUpsert` is already gone with #12902.
### Recommendation
Option 1 if someone will own the ClickHouse CI wiring in step 2, since without it step 3 produces another file that nothing builds. Otherwise option 2: a test no build compiles is worse than no test, because it reads as coverage that does not exist. This is a call for the ClickHouse maintainers rather than something #12902 should decide.
cc @jackylee-ch
Contributor guide
Research direction
Start with backends-clickhouse/pom.xml, the ClickHouse Jenkins job, and the test paths named in the issue. Read #12933 before deciding whether to keep or remove the tests and Flink dependencies. Done means either CI compiles and runs both Iceberg tests after the required wiring and port, or the surviving test and unused dependencies are removed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, scala
- Domain
- build-system, ci-cd, testing
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100