voonhous commented on code in PR #19217: URL: https://github.com/apache/hudi/pull/19217#discussion_r3701775467
########## docker/trino/shim/pom.xml: ########## @@ -0,0 +1,157 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<!-- + In-repo mirror of the RFC-105 Trino-side shim (trinodb/trino plugin/trino-hudi), + which is not yet released upstream. It exists so this repo can assemble the + deployable Trino plugin directory (hudi-trino jar + runtime deps + service + descriptor) for the E2E docker image without a trinodb/trino checkout, and it + stays useful afterwards: CI must test hudi-trino at HEAD while the upstream shim + pins released versions. + + Standalone project - NOT part of the Hudi reactor and never deployed. + + Build (JDK 25, after installing hudi-trino into the local m2): + mvn -f docker/trino/shim/pom.xml clean package -DskipTests + Output plugin dir: target/trino-hudi-<trino.version>/ + Use `package`, never `install`: installing would shadow the real + io.trino:trino-hudi release coordinates in the local repository. + + The parent version below must stay in sync with trino.version in the root pom, + the TRINO_VERSION ARG in docker/trino/Dockerfile, and the compose image tag. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + + <parent> + <groupId>io.trino</groupId> + <artifactId>trino-root</artifactId> + <version>481</version> + <!-- Resolve the parent from Maven Central, never the file system. --> + <relativePath /> + </parent> + + <artifactId>trino-hudi</artifactId> + <packaging>trino-plugin</packaging> + <description>Trino - Hudi connector plugin assembly (in-repo E2E shim mirroring the upstream plugin/trino-hudi shim planned by RFC-105; never deployed)</description> + + <properties> + <!-- Hudi connector under test; override with -Ddep.hudi.version=... --> + <dep.hudi.version>1.3.0-SNAPSHOT</dep.hudi.version> + <!-- Assembly-only project: skip airbase/basepom checks (license header + style, checkstyle, enforcer, modernizer, duplicate-finder, dependency + analysis). ASF headers are enforced separately by + scripts/release/validate_source_copyright.sh. --> + <air.check.skip-all>true</air.check.skip-all> + <!-- Test infra only: never deploy/release this artifact. --> + <maven.deploy.skip>true</maven.deploy.skip> Review Comment: Added `<maven.install.skip>true</maven.install.skip>` next to the deploy skip, and updated the pom comment to say install is hard-disabled. ########## hudi-integ-test/src/test/java/org/apache/hudi/integ2/testcontainers/trino/ITTestTrinoCustomType.java: ########## @@ -0,0 +1,369 @@ +/* + * 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 { + // -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 testTrinoBlobBothPartitionsVisible() throws Exception { Review Comment: Did both: renamed to `testTrinoBlobEmptiedPartitionInvisible` / `testTrinoVariantEmptiedPartitionInvisible` and pinned the claim with `assertStdOutContains("2024-01-02", 0)`. -- 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]
