This is an automated email from the ASF dual-hosted git repository.

voonhous pushed a commit to branch release-1.2.1
in repository https://gitbox.apache.org/repos/asf/hudi.git


The following commit(s) were added to refs/heads/release-1.2.1 by this push:
     new 7b411aa2b919 fix(trino): unbind HudiCacheKeyProvider on 1.2.1 (#19985)
7b411aa2b919 is described below

commit 7b411aa2b919da66fdeb3ebf92b8e037bfba8873
Author: voonhous <[email protected]>
AuthorDate: Thu Sep 17 20:50:31 2026 +0800

    fix(trino): unbind HudiCacheKeyProvider on 1.2.1 (#19985)
    
    HudiCacheKeyProvider implements the Trino 483 CacheKeyProvider
    contract. After 483, getCacheKey returns Optional<CacheKey> instead of
    Optional<String>. The erased method descriptor is identical, so a
    connector built at 483 still links in a plugin assembled against a
    newer Trino, then fails with ClassCastException at the first cached
    read. Leave the provider unbound so Trino's default provider (path,
    last-modified time and length) is used; caching stays on.
    
    The testSelectWithFilter and testJoin op-count assertions in
    TestHudiAlluxioCacheFileOperations and TestHudiMemoryCacheFileOperations
    encode the provider's exclusions (table properties and index
    definitions re-read on every query), so they are disabled with the
    reason in the annotation; they return with the binding. Add
    testReadsServedFromMemoryCache so the metadata memory cache keeps
    count-independent coverage alongside testReadsServedFromAlluxioCache.
    
    This is release-only: master is on the new contract and keeps the
    binding.
---
 .../main/java/io/trino/plugin/hudi/HudiModule.java |  8 +++---
 .../hudi/TestHudiAlluxioCacheFileOperations.java   |  3 +++
 .../hudi/TestHudiMemoryCacheFileOperations.java    | 30 ++++++++++++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java 
b/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java
index cd17429b90d3..7dec679db63d 100644
--- a/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java
+++ b/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java
@@ -19,7 +19,6 @@ import com.google.inject.Module;
 import com.google.inject.Provides;
 import com.google.inject.Scopes;
 import com.google.inject.Singleton;
-import io.trino.filesystem.cache.CacheKeyProvider;
 import io.trino.metastore.HiveMetastore;
 import io.trino.plugin.base.metrics.FileFormatDataSourceStats;
 import io.trino.plugin.base.session.SessionPropertiesProvider;
@@ -28,7 +27,6 @@ import io.trino.plugin.hive.HiveNodePartitioningProvider;
 import io.trino.plugin.hive.HiveTransactionHandle;
 import io.trino.plugin.hive.parquet.ParquetReaderConfig;
 import io.trino.plugin.hive.parquet.ParquetWriterConfig;
-import io.trino.plugin.hudi.cache.HudiCacheKeyProvider;
 import io.trino.plugin.hudi.stats.ForHudiTableStatistics;
 import io.trino.spi.connector.ConnectorNodePartitioningProvider;
 import io.trino.spi.connector.ConnectorPageSourceProvider;
@@ -40,7 +38,6 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.function.BiFunction;
 
 import static com.google.inject.multibindings.Multibinder.newSetBinder;
-import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder;
 import static io.airlift.concurrent.Threads.daemonThreadsNamed;
 import static io.airlift.configuration.ConfigBinder.configBinder;
 import static io.airlift.bootstrap.ClosingBinder.closingBinder;
@@ -75,7 +72,10 @@ public class HudiModule
         binder.bind(FileFormatDataSourceStats.class).in(Scopes.SINGLETON);
         
newExporter(binder).export(FileFormatDataSourceStats.class).withGeneratedName();
 
-        newOptionalBinder(binder, 
CacheKeyProvider.class).setBinding().to(HudiCacheKeyProvider.class).in(Scopes.SINGLETON);
+        // HudiCacheKeyProvider is deliberately not bound on release-1.2.1, so 
Trino's default
+        // provider is used. It implements the Trino 483 CacheKeyProvider 
contract, which changed
+        // after 483; binding it breaks a plugin assembled against a newer 
Trino at the first cached
+        // read. Restore the binding when the connector targets a Trino with 
the new contract.
 
         closingBinder(binder).registerExecutor(Key.get(ExecutorService.class, 
ForHudiTableStatistics.class));
         closingBinder(binder).registerExecutor(Key.get(ExecutorService.class, 
ForHudiSplitManager.class));
diff --git 
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
 
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
index 22f569dcf50d..03e579f7bbce 100644
--- 
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
+++ 
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
@@ -24,6 +24,7 @@ import io.trino.testing.AbstractTestQueryFramework;
 import io.trino.testing.DistributedQueryRunner;
 import io.trino.testing.QueryRunner;
 import org.intellij.lang.annotations.Language;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.parallel.Execution;
 import org.junit.jupiter.api.parallel.ExecutionMode;
@@ -82,6 +83,7 @@ public class TestHudiAlluxioCacheFileOperations
     }
 
     @Test
+    @Disabled("Asserts the exclusions of HudiCacheKeyProvider (table 
properties and index definitions are re-read from storage on every query). 
release-1.2.1 does not bind that provider, so Trino's default provider caches 
those files and the counts no longer hold. Re-enable together with the binding 
in HudiModule.")
     public void testSelectWithFilter()
     {
         @Language("SQL") String query = "SELECT * FROM " + 
HUDI_MULTI_FG_PT_V8_MOR + " WHERE country='SG'";
@@ -103,6 +105,7 @@ public class TestHudiAlluxioCacheFileOperations
     }
 
     @Test
+    @Disabled("Asserts the exclusions of HudiCacheKeyProvider (table 
properties and index definitions are re-read from storage on every query). 
release-1.2.1 does not bind that provider, so Trino's default provider caches 
those files and the counts no longer hold. Re-enable together with the binding 
in HudiModule.")
     public void testJoin()
     {
         @Language("SQL") String query = "SELECT t1.id, t1.name, t1.price, 
t1.ts FROM " +
diff --git 
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
 
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
index b5ada5741411..03cd41ee4c8d 100644
--- 
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
+++ 
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
@@ -23,6 +23,7 @@ import io.trino.testing.AbstractTestQueryFramework;
 import io.trino.testing.DistributedQueryRunner;
 import io.trino.testing.QueryRunner;
 import org.intellij.lang.annotations.Language;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.parallel.Execution;
 import org.junit.jupiter.api.parallel.ExecutionMode;
@@ -40,6 +41,7 @@ import static 
io.trino.plugin.hudi.util.FileOperationUtils.FileType.METADATA_TAB
 import static 
io.trino.plugin.hudi.util.FileOperationUtils.FileType.TABLE_PROPERTIES;
 import static io.trino.testing.MultisetAssertions.assertMultisetsEqual;
 import static java.util.stream.Collectors.toCollection;
+import static org.assertj.core.api.Assertions.assertThat;
 
 @ResourceLock("HUDI_CACHE_SYSTEM")
 @Execution(ExecutionMode.SAME_THREAD)
@@ -70,6 +72,7 @@ public class TestHudiMemoryCacheFileOperations
     }
 
     @Test
+    @Disabled("Asserts the exclusions of HudiCacheKeyProvider in the metadata 
memory cache (table properties and index definitions are re-read from storage 
on every query). release-1.2.1 does not bind that provider, so Trino's default 
provider caches those files and the counts no longer hold. Re-enable together 
with the binding in HudiModule.")
     public void testSelectWithFilter()
     {
         @Language("SQL") String query = "SELECT * FROM " + 
HUDI_MULTI_FG_PT_V8_MOR + " WHERE country='SG'";
@@ -93,6 +96,7 @@ public class TestHudiMemoryCacheFileOperations
     }
 
     @Test
+    @Disabled("Asserts the exclusions of HudiCacheKeyProvider in the metadata 
memory cache (table properties and index definitions are re-read from storage 
on every query). release-1.2.1 does not bind that provider, so Trino's default 
provider caches those files and the counts no longer hold. Re-enable together 
with the binding in HudiModule.")
     public void testJoin()
     {
         @Language("SQL") String query = "SELECT t1.id, t1.name, t1.price, 
t1.ts FROM " +
@@ -117,6 +121,32 @@ public class TestHudiMemoryCacheFileOperations
                         .build());
     }
 
+    @Test
+    public void testReadsServedFromMemoryCache()
+    {
+        // Count-independent coverage that the memory cache serves reads, 
whichever CacheKeyProvider is
+        // bound: once a query has warmed the cache, repeating it still goes 
through the cache for data
+        // files (FileSystemCache.cacheInput) but opens none of them from 
storage (InputFile.newInput is
+        // only issued when the cache loads a missing entry).
+        @Language("SQL") String query = "SELECT * FROM " + 
HUDI_MULTI_FG_PT_V8_MOR;
+        DistributedQueryRunner queryRunner = getDistributedQueryRunner();
+        queryRunner.executeWithPlan(queryRunner.getDefaultSession(), query);
+
+        queryRunner.executeWithPlan(queryRunner.getDefaultSession(), query);
+        Multiset<FileOperation> dataOperations = 
queryRunner.getSpans().stream()
+                .filter(span -> 
span.getName().equals("FileSystemCache.cacheInput") || 
span.getName().equals("InputFile.newInput"))
+                .filter(span -> 
!isTrinoSchemaOrPermissions(getFileLocation(span)))
+                .map(FileOperation::create)
+                .filter(operation -> operation.fileType() == DATA)
+                .collect(toCollection(HashMultiset::create));
+        assertThat(dataOperations.count(new 
FileOperation("FileSystemCache.cacheInput", DATA)))
+                .as("data file reads through the memory cache")
+                .isGreaterThanOrEqualTo(1);
+        assertThat(dataOperations.count(new 
FileOperation("InputFile.newInput", DATA)))
+                .as("data files opened from storage after the cache was 
warmed")
+                .isZero();
+    }
+
     private void assertFileSystemAccesses(@Language("SQL") String query, 
Multiset<FileOperation> expectedCacheAccesses)
     {
         DistributedQueryRunner queryRunner = getDistributedQueryRunner();

Reply via email to