LuciferYang commented on PR #12902:
URL: https://github.com/apache/gluten/pull/12902#issuecomment-5478173953
A correction to my last comment: I blamed my own setup for the
`ClickHouseIcebergHiveTableSupport` failure, and that was wrong. It is a build
bug, and once I identified it I could verify the rest properly.
The `iceberg` and `iceberg-test` profiles both declare a
`build-helper-maven-plugin` execution with the id `add-iceberg-test-sources`,
at `pom.xml:1861` and `pom.xml:1912` on main. Maven merges executions by id, so
when both profiles are active only one `<sources>` list survives.
`help:effective-pom` with `-Pbackends-clickhouse -Pspark-3.5 -Piceberg
-Piceberg-test` resolves that execution to
`src-iceberg-spark35/test/{scala,java}` alone; the `iceberg` profile's
`src-iceberg/test/{scala,java}` and `src-iceberg10/test/{scala,java}` are
dropped.
For ClickHouse that combination is unbuildable by construction.
`ClickHouseIcebergHiveTableSupport` lives in `src-iceberg/test/scala`, and both
Java tests instantiate it, so the class leaves the source path at exactly the
moment the tests join it. Giving the `iceberg-test` execution a distinct id and
changing nothing else makes `test-compile -Pbackends-clickhouse -Pspark-3.5
-Pscala-2.13 -Pspark-ut -Pdelta -Piceberg -Piceberg-test` exit 0 and emit both
`ClickHouseIcebergHiveTableSupport.class` and
`TestPositionDeletesTableGluten.class`. So `TestFlinkUpsert` has not compiled
since #9996 introduced this layout, and neither has its sibling in either copy.
#9996 is also the commit that added the ported 3.5 sibling and left this file
on 3.3.
Velox never hits the collision, which is why it has gone unnoticed: nothing
there needs both directories at once. The `src-iceberg/test` suites run in the
jobs that pass `-Piceberg` alone, and the 57 files under
`src-iceberg-spark34/test/java` run in the three groups that pass both.
With the ids de-conflicted, a straight copy of `TestFlinkUpsert` into
`src-iceberg-spark35` fails on one symbol,
`org.apache.iceberg.flink.MiniClusterResource`, used once for
`DISABLE_CLASSLOADER_CHECK_CONFIG`. So "migrating the test to the 1.19 test
API" was too strong: the file is already JUnit 5, with `@TestTemplate` and
`org.apache.iceberg.Parameters`, and `MiniFlinkClusterExtension` is the
replacement.
I would still keep all of it out of this PR, since fixing the profile ids
changes what three Velox groups compile and would newly run a ClickHouse test
that has never run. Filed as #12933 for the execution-id collision, and #12934
for the ClickHouse side, which covers the CI wiring, the `MiniClusterResource`
port, and the Flink dependency block that has no user today.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
