[BUILD] Give iceberg-test its own build-helper execution ids

[LuciferYang]

What changes are proposed in this pull request?

The iceberg and iceberg-test profiles each declared a build-helper-maven-plugin execution named add-iceberg-test-sources, and another named add-iceberg-test-resources. Maven merges plugin executions by id, so activating both profiles kept only one <sources> list per id. This gives the iceberg-test pair its own ids and changes nothing else.

Fixes #12933.

Before the change, help:effective-pom with -Pbackends-clickhouse -Pspark-3.5 -Pspark-ut -Pdelta -Piceberg -Piceberg-test resolved add-iceberg-test-sources to src-iceberg-spark35/test/{scala,java} alone. The iceberg profile's src-iceberg/test/{scala,java} and src-iceberg${iceberg.binary.version}/test/{scala,java} were dropped with no warning. After the change all four executions survive in both backends.

For ClickHouse the old behaviour made that profile combination unbuildable. ClickHouseIcebergHiveTableSupport lives in src-iceberg/test/scala and the Java test under src-iceberg-spark35/test/java instantiates it, so the class left the source path at exactly the moment the test joined it:

$ ./build/mvn -Pbackends-clickhouse -Pspark-3.5 -Pscala-2.13 -Pspark-ut -Pdelta \
      -Piceberg -Piceberg-test -DskipTests test-compile -pl backends-clickhouse -am
[ERROR] .../src-iceberg-spark35/test/java/.../TestPositionDeletesTableGluten.java:[41,18] cannot find symbol
[ERROR]   symbol:   class ClickHouseIcebergHiveTableSupport

Velox was unaffected because nothing there needed both directories at once: the src-iceberg/test suites run in the jobs that pass -Piceberg alone, and the files under src-iceberg-spark34/test run in the three groups that pass both.

The one behaviour change a reviewer should weigh is that those three Spark 3.4 groups, velox_backend_x86.yml:817, :821 and :825, plus the Spark 4.0 test-compile at :1277, now also compile src-iceberg/test/**. Group 1 picks up VeloxIcebergSuite and VeloxTPCHIcebergSuite through its org.apache.gluten wildcard, so those two suites now also run on Spark 3.4, where they did not run before; the -Piceberg-only jobs at :941 and :1013 run them on Spark 3.5. -Piceberg cannot be dropped from the three commands, because the src-iceberg-spark34/test sources depend on classes and dependencies that only that profile provides.

src-iceberg${iceberg.binary.version}/test and every src-iceberg*/test/resources directory are empty in all modules today, so the resources rename and that half of the sources rename are latent fixes with no effect on the current tree. Only backends-velox and backends-clickhouse hold files under src-iceberg*/test, so nothing outside those two modules is touched.

While checking for the same shape elsewhere in the root pom, the only other cross-profile id reuse is enforce-java-17+ and enforce-scala-213, shared by the spark-4.0 and spark-4.1 profiles. Those two are mutually exclusive, so the merge never happens and I left them alone.

How was this patch tested?

All four runs are on e0dce586b with only this change applied, JDK 17.

what	result
ClickHouse test-compile, -Piceberg -Piceberg-test, before the change	fails on ClickHouseIcebergHiveTableSupport
the same command after the change	exit 0, emits ClickHouseIcebergHiveTableSupport.class and TestPositionDeletesTableGluten.class
Velox test-compile with the profile set the three x86 groups use (-Pjava-17 -Pspark-3.4 -Pspark-ut -Piceberg -Piceberg-test -Pdelta -Phudi -Ppaimon)	BUILD SUCCESS
help:effective-pom for both backends after the change	all four executions present, six test source directories each

The Velox run is the one that matters for regressions, since it is the combination that newly compiles five more files. It emitted VeloxIcebergSuite, VeloxTPCHIcebergSuite, enhanced/VeloxIcebergSuite and TestIcebergNestedFieldVisitor while keeping the existing src-iceberg-spark34/test classes.

I did not run the ClickHouse tests themselves. TestPositionDeletesTableGluten compiles for the first time here but no job passes -Piceberg-test to a ClickHouse build, so it still does not execute; #12934 covers whether it should.

Was this patch authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Opus.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Resolves Maven build-helper-maven-plugin execution-id collisions between the iceberg and iceberg-test profiles so both profiles can be activated together without losing test source/resource paths.

Changes:

• Renames build-helper-maven-plugin execution ids to avoid Maven merging executions across profiles.
• Preserves separate add-test-source and add-test-resource executions so all intended test directories remain on the effective POM/classpath.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[LuciferYang]

The ids already differ from the iceberg profile's add-iceberg-test-sources / add-iceberg-test-resources, which is exactly the collision this PR fixes: Maven merges plugin executions by id, so a shared id silently drops one profile's sources whenever both profiles are active (which is how every in-repo build runs). The spark token marks these as the spark-side iceberg test sources, and the inline comment documents why they must stay distinct. Happy to switch to a specific naming scheme if you prefer one.

[LuciferYang]

The ids already differ from the iceberg profile's add-iceberg-test-sources / add-iceberg-test-resources, which is exactly the collision this PR fixes: Maven merges plugin executions by id, so a shared id silently drops one profile's sources whenever both profiles are active (which is how every in-repo build runs). The spark token marks these as the spark-side iceberg test sources, and the inline comment documents why they must stay distinct. Happy to switch to a specific naming scheme if you prefer one.

[github-actions]

Run Gluten Clickhouse CI on x86

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

🟢 Approval recommended

The change is narrowly scoped to Maven execution IDs, resolves a verified effective-POM merge bug, and does not alter the actual source/resource directory lists.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[LuciferYang]

Good prompt, though I landed somewhere slightly different.

I kept the ids naming what the execution adds (src-iceberg-spark${spark.plain.version}/test/...) rather than which profile owns it, because the profile-based scheme is what created the confusion in the first place: the iceberg profile's own test execution is already called add-iceberg-test-sources, and that is the name that reads as if it belonged to iceberg-test. Renaming by owner would mean renaming that one too, which is four more executions in a profile the collision does not otherwise touch.

What the comment is really after, though, is that the reason for the difference should be visible at the point where someone might undo it. So I said it out loud instead of encoding it in the id:

<!-- These ids must stay different from the iceberg profile's
     add-iceberg-test-sources / add-iceberg-test-resources: Maven merges plugin
     executions by id, so sharing one would drop the other profile's sources
     whenever both profiles are active, which is how every in-repo build runs. -->

That also covers the future-collision half of your point better than a naming convention would, since nothing enforces a convention.

Also merged current main, which moved the two colliding executions from pom.xml:1861 and pom.xml:1912 to :1833 and :1884 after #12902 dropped the spark-3.3 profile. #12933's line numbers are updated (the description never carried them). Re-ran the check that matters on the new base: test-compile -Pbackends-clickhouse -Pspark-3.5 -Pspark-ut -Pdelta -Piceberg -Piceberg-test exits 0 and emits both ClickHouseIcebergHiveTableSupport.class and TestPositionDeletesTableGluten.class, which is the pair that cannot compile without this fix.

[jackylee-ch]

The merge-executions-by-id bug is real and the fix is correct — I confirmed from the diff that the iceberg and iceberg-test sources/resources executions now keep distinct ids, so neither profile's list is dropped when both are active. One question on the CI-time cost below.

[jackylee-ch]

With both sources executions now surviving, the three velox_backend_x86 groups that pass -Piceberg -Piceberg-test compile and run VeloxIcebergSuite/VeloxTPCHIcebergSuite twice across the matrix (here, and again in the -Piceberg-only jobs). You flagged this in the description and kept -Piceberg to preserve dependency resolution — given the ongoing effort to reduce the project's GHA usage, is the dependency difference from dropping -Piceberg on those three commands actually load-bearing, or could we drop it there to avoid the double run?

[LuciferYang]

-Piceberg is load-bearing there: the src-iceberg-spark34/test sources that -Piceberg-test adds import org.apache.gluten.TestConfUtil from src-iceberg/main/java and need the gluten-iceberg module and iceberg-spark-runtime, all of which only -Piceberg supplies. Dropping it would break test-compile for those three groups.

I also overstated the cost in the description. These three groups are Spark 3.4, and before this fix VeloxIcebergSuite / VeloxTPCHIcebergSuite never ran on 3.4 at all (the collision dropped src-iceberg/test, and the 3.4 slow jobs only run tagged suites). The -Piceberg-only runs at :941 and :1013 are Spark 3.5. So the change adds 3.4 coverage for those two suites in group 1, rather than running the same thing twice. I've corrected the description.

[jackylee-ch]

LGTM