apache / apache/gluten

[CH] Decide the fate of the ClickHouse Iceberg Java tests, which no build compiles today

Open
#12,934 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.