wombatu-kun commented on code in PR #19217: URL: https://github.com/apache/hudi/pull/19217#discussion_r3710301424
########## hudi-integ-test/src/test/java/org/apache/hudi/integ2/testcontainers/trino/ITTestTrinoCustomType.java: ########## @@ -0,0 +1,376 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.integ2.testcontainers.trino; + +import org.apache.hudi.integ2.testcontainers.ITTestBaseTestcontainers; +import org.apache.hudi.integ2.testcontainers.ITTestCustomTypeHiveSync; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import static org.apache.hudi.integ2.testcontainers.TestcontainersConfig.Paths; + +/** + * Trino read coverage for Hudi's custom logical types (BLOB struct, VECTOR + * fixed_len_byte_array, VARIANT), complementing {@link ITTestCustomTypeHiveSync} + * which asserts the same fixtures round-trip through the Hive serde. A flip in + * either direction (e.g. VECTOR decoded as array<float> instead of + * binary, BLOB struct field projection broken, VARIANT row count off) shows up + * here. + * + * <p>This test reuses the same {@code sparksql-*-sql.commands} fixtures that + * {@code ITTestCustomTypeHiveSync} drives, so the two tests can run in either + * order without cross-contamination (each has its own {@code @BeforeAll} that + * re-seeds, and an {@code @AfterAll} that cleans up). + * + * <p>VARIANT seeding and tests only fire on a Spark 4.x compose; on Spark 3.5 Review Comment: Only the `spark402` compose pair defines `trinocoordinator`, so on a Spark 3.5 prefix this class either skips entirely or fails in `initializeServices` - the BLOB and VECTOR fallback described here cannot run, and `isSpark4Compose()` is always true when the class does. Drop this sentence and the `assumeSpark4Compose` guards, or is a Spark 3.5 trino service planned? ########## hudi-integ-test/src/test/java/org/apache/hudi/integ2/testcontainers/trino/ITTestTrinoCustomType.java: ########## @@ -0,0 +1,376 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.integ2.testcontainers.trino; + +import org.apache.hudi.integ2.testcontainers.ITTestBaseTestcontainers; +import org.apache.hudi.integ2.testcontainers.ITTestCustomTypeHiveSync; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import static org.apache.hudi.integ2.testcontainers.TestcontainersConfig.Paths; + +/** + * Trino read coverage for Hudi's custom logical types (BLOB struct, VECTOR + * fixed_len_byte_array, VARIANT), complementing {@link ITTestCustomTypeHiveSync} + * which asserts the same fixtures round-trip through the Hive serde. A flip in + * either direction (e.g. VECTOR decoded as array<float> instead of + * binary, BLOB struct field projection broken, VARIANT row count off) shows up + * here. + * + * <p>This test reuses the same {@code sparksql-*-sql.commands} fixtures that + * {@code ITTestCustomTypeHiveSync} drives, so the two tests can run in either + * order without cross-contamination (each has its own {@code @BeforeAll} that + * re-seeds, and an {@code @AfterAll} that cleans up). + * + * <p>VARIANT seeding and tests only fire on a Spark 4.x compose; on Spark 3.5 + * the BLOB and VECTOR coverage still runs. + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class ITTestTrinoCustomType extends ITTestBaseTestcontainers { + + private static final String BLOB_TEST_PATH = "/user/hive/warehouse/blob_test"; + private static final String BLOB_TEST_DF_PATH = "/user/hive/warehouse/blob_test_df"; + private static final String VECTOR_TEST_PATH = "/user/hive/warehouse/vector_test"; + private static final String VARIANT_TEST_PATH = "/user/hive/warehouse/variant_test"; + private static final String SPARKSQL_BLOB_TYPE_SQL_COMMANDS = + Paths.DEMO_DIR + "/sparksql-blob-type-sql.commands"; + private static final String SPARKSQL_BLOB_TYPE_DF_COMMANDS = + Paths.DEMO_DIR + "/sparksql-blob-type-df.commands"; + private static final String SPARKSQL_VECTOR_TYPE_SQL_COMMANDS = + Paths.DEMO_DIR + "/sparksql-vector-type-sql.commands"; + private static final String SPARKSQL_VARIANT_TYPE_SQL_COMMANDS = + Paths.DEMO_DIR + "/sparksql-variant-type-sql.commands"; + + @BeforeAll + public void setupOnce() throws Exception { + assumeTrinoProfile(); + initializeServices(); + waitForHdfs(); + sparkAdhoc1.executeShellCommand("/bin/bash " + Paths.DEMO_SETUP).expectToSucceed(); + sparkAdhoc1.executeSQLFile(SPARKSQL_BLOB_TYPE_SQL_COMMANDS) + .expectToSucceed() + .assertStdOutContainsLine("BLOB_SQL_TEST_SUCCESS"); + // The DF fixture writes blob_test_df with the INLINE branch of the BLOB struct + // (data field non-null, reference null). The SQL fixture exercises only the + // OUT_OF_LINE branch, so seeding both gives Trino read coverage of both shapes. + sparkAdhoc1.executeSQLFile(SPARKSQL_BLOB_TYPE_DF_COMMANDS) + .expectToSucceed() + .assertStdOutContainsLine("BLOB_DF_TEST_SUCCESS"); + sparkAdhoc1.executeSQLFile(SPARKSQL_VECTOR_TYPE_SQL_COMMANDS) + .expectToSucceed() + .assertStdOutContainsLine("VECTOR_SQL_TEST_SUCCESS"); + if (isSpark4Compose()) { + // VARIANT type is Spark 4.x only - guard the seed so the BLOB/VECTOR + // coverage still runs on a Spark 3.5 stack. + sparkAdhoc1.executeSQLFile(SPARKSQL_VARIANT_TYPE_SQL_COMMANDS) + .expectToSucceed() + .assertStdOutContainsLine("VARIANT_SQL_TEST_SUCCESS"); + } + trino.waitUntilReady(); + } + + @AfterAll + public void clean() throws Exception { + // JUnit runs @AfterAll even when the @BeforeAll assumption aborted setupOnce() + // before initializeServices(); nothing was seeded then, so nothing to clean. + if (sparkAdhoc1 == null) { + return; + } + // -f silently skips non-existent paths so the variant_test cleanup is safe + // even on Spark 3.5 runs where the table was never created. + sparkAdhoc1.executeShellCommand("hdfs dfs -rm -R -f " + + BLOB_TEST_PATH + " " + BLOB_TEST_DF_PATH + " " + + VECTOR_TEST_PATH + " " + VARIANT_TEST_PATH).expectToSucceed(); + } + + // ---------- BLOB OUT_OF_LINE (blob_test) ---------- + + @Test + public void testTrinoCountBlob() throws Exception { + // Post-DELETE state of sparksql-blob-type-sql.commands is 2 rows (id=1 updated, + // id=2 merged, id=3 inserted then deleted) - parity with the Hive count assertion + // in ITTestCustomTypeHiveSync#testBlobTypeWithHiveSyncSQL. + trino.execute("SELECT count(*) FROM blob_test") + .expectToSucceed() + .assertStdOutContains("2"); + } + + @Test + public void testTrinoProjectsBlobUpdatedRow() throws Exception { + // Full per-row shape for id=1 (post-UPDATE state): type discriminator + + // every reference subfield + the OUT_OF_LINE invariant that data IS NULL. + // One query, one substring assertion - catches column-order shifts, + // nested-struct field renames, and per-field decoding bugs. + trino.execute("SELECT blob_data.type, blob_data.data IS NULL, " + + "blob_data.reference.external_path, blob_data.reference.offset, " + + "blob_data.reference.length, blob_data.reference.managed " + + "FROM blob_test WHERE id = 1") + .expectToSucceed() + .assertStdOutContains("OUT_OF_LINE,true,blobs/updated-1,10,100,true"); + } + + @Test + public void testTrinoProjectsBlobMergedRow() throws Exception { + // id=2 was MATCHED by the MERGE clause and rewritten to 'blobs/merged-2'. + // Same full-shape assertion as id=1 - confirms both UPDATE and MERGE write + // paths land at an identical on-disk OUT_OF_LINE shape. + trino.execute("SELECT blob_data.type, blob_data.data IS NULL, " + + "blob_data.reference.external_path, blob_data.reference.offset, " + + "blob_data.reference.length, blob_data.reference.managed " + + "FROM blob_test WHERE id = 2") + .expectToSucceed() + .assertStdOutContains("OUT_OF_LINE,true,blobs/merged-2,20,200,true"); + } + + @Test + public void testTrinoBlobDeletedRowAbsent() throws Exception { + // id=3 was MERGE-inserted into dt=2024-01-02 then DELETEd. A DELETE that + // leaves the row visible (e.g. tombstone not honored on read) shows up as + // count = 1 here. Pairs with testTrinoCountBlob = 2 (total post-delete) + // to catch the case where DELETE silently no-ops. + trino.execute("SELECT count(*) FROM blob_test WHERE id = 3") + .expectToSucceed() + .assertStdOutContains("0"); + } + + @Test + public void testTrinoBlobEmptiedPartitionInvisible() throws Exception { Review Comment: `GROUP BY dt` only emits groups for partitions that still have rows, so this cannot tell a connector that prunes empty partitions from one that lists them - and `ITTestCustomTypeHiveSync` shows `dt=2024-01-02` is still registered in the metastore. Drop the partition-pruning claim from the comment here and in `testTrinoVariantEmptiedPartitionInvisible`; follow-up, not a blocker. ########## hudi-integ-test/src/test/java/org/apache/hudi/integ2/testcontainers/trino/ITTestTrinoCustomType.java: ########## @@ -0,0 +1,376 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.integ2.testcontainers.trino; + +import org.apache.hudi.integ2.testcontainers.ITTestBaseTestcontainers; +import org.apache.hudi.integ2.testcontainers.ITTestCustomTypeHiveSync; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import static org.apache.hudi.integ2.testcontainers.TestcontainersConfig.Paths; + +/** + * Trino read coverage for Hudi's custom logical types (BLOB struct, VECTOR + * fixed_len_byte_array, VARIANT), complementing {@link ITTestCustomTypeHiveSync} + * which asserts the same fixtures round-trip through the Hive serde. A flip in + * either direction (e.g. VECTOR decoded as array<float> instead of + * binary, BLOB struct field projection broken, VARIANT row count off) shows up + * here. + * + * <p>This test reuses the same {@code sparksql-*-sql.commands} fixtures that Review Comment: The javadoc credits `ITTestCustomTypeHiveSync` with a re-seeding `@BeforeAll` and a cleaning `@AfterAll`, but it seeds inside each `@Test` and cleans in `@AfterEach`. Reword this to point at `tearDownDockerCompose` dropping the whole stack between classes, which is what actually keeps the two from contaminating each other. -- 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]
