From 8d9545c0f0cfdb5f05de20c602b36d97d49d8163 Mon Sep 17 00:00:00 2001 From: Jialiang Tan Date: Tue, 16 Sep 2025 21:16:35 -0700 Subject: [PATCH] [pos] Fix connector caused test failures --- .../AbstractTestNativeGeneralQueries.java | 12 +++++- .../PrestoSparkNativeQueryRunnerUtils.java | 42 +++++++++++++++++-- .../TestPrestoSparkNativeGeneralQueries.java | 13 +++--- ...PrestoSparkNativeTpchConnectorQueries.java | 10 ----- 4 files changed, 56 insertions(+), 21 deletions(-) diff --git a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/AbstractTestNativeGeneralQueries.java b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/AbstractTestNativeGeneralQueries.java index 1549ccddbaaef..b0db6a4c0a7cf 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/AbstractTestNativeGeneralQueries.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/AbstractTestNativeGeneralQueries.java @@ -141,7 +141,17 @@ public void testCatalogWithCacheEnabled() .put("hive.pushdown-filter-enabled", "true") .build(); - getQueryRunner().createCatalog("hivecached", "hive", hiveProperties); + try { + getQueryRunner().createCatalog("hivecached", "hive", hiveProperties); + } + catch (IllegalArgumentException e) { + if (e.getMessage().contains("A catalog already exists")) { + System.out.println("Catalog 'hivecached' already exists, skipping creation"); + } + else { + throw e; + } + } Session actualSession = Session.builder(getSession()) .setCatalog("hivecached") diff --git a/presto-native-execution/src/test/java/com/facebook/presto/spark/PrestoSparkNativeQueryRunnerUtils.java b/presto-native-execution/src/test/java/com/facebook/presto/spark/PrestoSparkNativeQueryRunnerUtils.java index a0856f8c6a530..3a386faece33a 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/spark/PrestoSparkNativeQueryRunnerUtils.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/spark/PrestoSparkNativeQueryRunnerUtils.java @@ -31,6 +31,7 @@ import java.nio.file.Paths; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; import static com.facebook.airlift.log.Level.WARN; import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.getNativeWorkerHiveProperties; @@ -106,6 +107,39 @@ public static PrestoSparkQueryRunner createHiveRunner() return queryRunner; } + /** + * Similar to createHiveRunner(), but also add additional specified catalogs and their + * corresponding properties. This method exists because unlike Java, native execution does not + * allow adding catalogs in the tests after process starts. So any tests that need additional + * catalogs need to add them upon runner creation. + */ + public static PrestoSparkQueryRunner createHiveRunner(Map> additionalCatalogs) + { + // Add connectors on the native side to make them available during execution. + ImmutableMap.Builder> catalogBuilder = ImmutableMap.builder(); + catalogBuilder.put("hive", ImmutableMap.of("connector.name", "hive")) + .putAll(additionalCatalogs); + PrestoSparkQueryRunner queryRunner = createRunner("hive", new NativeExecutionModule( + Optional.of(catalogBuilder.build()))); + + // Add connectors on the Java side to make them visible during planning. + additionalCatalogs.entrySet().stream().forEach(entry -> { + queryRunner.createCatalog( + entry.getKey(), + entry.getValue().get("connector.name"), + entry.getValue().entrySet().stream().filter(propertyEntry -> { + // Add all properties except for "connector.name" as it is not applicable + // for Java config (it's already fed in as the above function parameter). + return !propertyEntry.getKey().equals("connector.name"); + }).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); + }); + + PrestoNativeQueryRunnerUtils.setupJsonFunctionNamespaceManager(queryRunner, + "external_functions.json", "json"); + + return queryRunner; + } + private static PrestoSparkQueryRunner createRunner(String defaultCatalog, NativeExecutionModule nativeExecutionModule) { // Increases log level to reduce log spamming while running test. @@ -122,10 +156,10 @@ private static PrestoSparkQueryRunner createRunner(String defaultCatalog, Native public static PrestoSparkQueryRunner createTpchRunner() { return createRunner( - "tpchstandard", - new NativeExecutionModule( - Optional.of( - ImmutableMap.of("hive", ImmutableMap.of("connector.name", "tpch"))))); + "tpchstandard", + new NativeExecutionModule( + Optional.of( + ImmutableMap.of("tpchstandard", ImmutableMap.of("connector.name", "tpch"))))); } public static PrestoSparkQueryRunner createRunner(String defaultCatalog, Optional baseDir, Map additionalConfigProperties, Map additionalSparkProperties, ImmutableList nativeModules) diff --git a/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeGeneralQueries.java b/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeGeneralQueries.java index d67dba20dfa93..33f7d76a61b57 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeGeneralQueries.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeGeneralQueries.java @@ -19,6 +19,7 @@ import com.facebook.presto.scalar.sql.SqlInvokedFunctionsPlugin; import com.facebook.presto.testing.ExpectedQueryRunner; import com.facebook.presto.testing.QueryRunner; +import com.google.common.collect.ImmutableMap; import org.testng.annotations.Ignore; import java.util.ArrayList; @@ -32,7 +33,12 @@ public class TestPrestoSparkNativeGeneralQueries @Override protected QueryRunner createQueryRunner() { - QueryRunner queryRunner = PrestoSparkNativeQueryRunnerUtils.createHiveRunner(); + // Adding additional catalog needed in some tests in the suite. + QueryRunner queryRunner = PrestoSparkNativeQueryRunnerUtils.createHiveRunner( + ImmutableMap.of("hivecached", + ImmutableMap.of("connector.name", "hive", + "hive.storage-format", "DWRF", + "hive.pushdown-filter-enabled", "true"))); // Install plugins needed for extra array functions. queryRunner.installPlugin(new SqlInvokedFunctionsPlugin()); @@ -115,9 +121,4 @@ public void testRowWiseExchange() {} @Override @Ignore public void testAnalyzeStatsOnDecimals() {} - - // VeloxRuntimeError: it != connectors().end() Connector with ID 'hivecached' not registered - @Override - @Ignore - public void testCatalogWithCacheEnabled() {} } diff --git a/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeTpchConnectorQueries.java b/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeTpchConnectorQueries.java index 05d194f6f6754..0ffdd2a05a797 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeTpchConnectorQueries.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/spark/TestPrestoSparkNativeTpchConnectorQueries.java @@ -16,7 +16,6 @@ import com.facebook.presto.nativeworker.AbstractTestNativeTpchConnectorQueries; import com.facebook.presto.testing.ExpectedQueryRunner; import com.facebook.presto.testing.QueryRunner; -import org.testng.annotations.Ignore; public class TestPrestoSparkNativeTpchConnectorQueries extends AbstractTestNativeTpchConnectorQueries @@ -39,13 +38,4 @@ public void testMissingTpchConnector() { super.testMissingTpchConnector(".*Catalog tpch does not exist*"); } - - @Override - @Ignore - public void testTpchTinyTables() {} - - // VeloxRuntimeError: it != connectors().end() Connector with ID 'tpchstandard' not registered - @Override - @Ignore - public void testTpchDateFilter() {} }