prestodb · pramodsatya · Sep 10, 2023
@@ -29,6 +29,10 @@ workflows:
           run_linux_tests: << pipeline.parameters.run_linux_tests >>
           requires:
             - linux-build-and-unit-test
+      - linux-presto-native-tests:
+          run_linux_tests: << pipeline.parameters.run_linux_tests >>
+          requires:
+            - linux-build-and-unit-test
       - linux-spark-e2e-tests:
           run_linux_tests: << pipeline.parameters.run_linux_tests >>
           requires:
@@ -193,6 +197,54 @@ jobs:
             - store_artifacts:
                 path: '/tmp/PrestoNativeQueryRunnerUtils'
 
+  linux-presto-native-tests:
+    executor: build
+    parameters:
+      run_linux_tests:
+        type: boolean
+        default: false
+    parallelism: 5
+    steps:
+      - run: echo "Run Linux tests is << parameters.run_linux_tests >>"
+      - when:
+          condition: << parameters.run_linux_tests >>
+          steps:
+            - checkout
+            - attach_workspace:
+                at: presto-native-tests
+            - maven_install:
+                maven_install_opts: ${MAVEN_INSTALL_OPTS}
+                maven_fast_install: ${MAVEN_FAST_INSTALL}
+            - run:
+                name: 'Run Presto native tests'
+                command: |
+                  export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/usr/local/lib:/usr/local/lib64"
+                  export PRESTO_SERVER_PATH="${HOME}/project/presto-native-execution/_build/debug/presto_cpp/main/presto_server"
+                  export TEMP_PATH="/tmp"
+                  TESTFILES=$(circleci tests glob "presto-native-tests/src/test/**/Test*.java" | circleci tests split --split-by=timings)
+                  # Convert file paths to comma separated class names
+                  export TESTCLASSES=
+                  for test_file in $TESTFILES
+                  do
+                    tmp=${test_file##*/}
+                    test_class=${tmp%%\.*}
+                    export TESTCLASSES="${TESTCLASSES},$test_class"
+                  done
+                  export TESTCLASSES=${TESTCLASSES#,}
+
+                  if [ ! -z $TESTCLASSES ]; then
+                    mvn test \
+                      ${MAVEN_TEST} \
+                      -pl 'presto-native-tests' \
+                      -Dtest="${TESTCLASSES}" \
+                      -DPRESTO_SERVER=${PRESTO_SERVER_PATH} \
+                      -DDATA_DIR=${TEMP_PATH} \
+                      -Duser.timezone=America/Bahia_Banderas \
+                      -T1C
+                  fi
+            - store_artifacts:
+                path: '/tmp/PrestoNativeQueryRunnerUtils'
+
   linux-spark-e2e-tests:
     executor: build
     parameters:

@@ -64,6 +64,7 @@ jobs:
         run: |
           ./mvnw test -T 1 ${MAVEN_TEST} -pl '
             !presto-tests, 
+            !presto-native-tests,
             !presto-accumulo,
             !presto-cassandra,
             !presto-hive,

@@ -193,6 +193,7 @@
         <module>presto-test-coverage</module>
         <module>presto-hudi</module>
         <module>presto-native-execution</module>
+        <module>presto-native-tests</module>
         <module>presto-router</module>
         <module>presto-open-telemetry</module>
         <module>redis-hbo-provider</module>

@@ -30,6 +30,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.Map;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.UUID;
@@ -88,7 +90,7 @@ public static QueryRunner createQueryRunner(
 
         defaultQueryRunner.close();
 
-        return createNativeQueryRunner(dataDirectory.get().toString(), prestoServerPath.get(), workerCount, cacheMaxSize, true, Optional.empty(), storageFormat, addStorageFormatToPath, false);
+        return createNativeQueryRunner(dataDirectory.get().toString(), prestoServerPath.get(), workerCount, cacheMaxSize, true, Optional.empty(), storageFormat, addStorageFormatToPath, false, Collections.emptyMap(), Collections.emptyMap());
     }
 
     public static QueryRunner createJavaQueryRunner()
@@ -265,7 +267,9 @@ public static QueryRunner createNativeQueryRunner(
             Optional<String> remoteFunctionServerUds,
             String storageFormat,
             boolean addStorageFormatToPath,
-            Boolean failOnNestedLoopJoin)
+            Boolean failOnNestedLoopJoin,
+            Map<String, String> extraProperties,
+            Map<String, String> extraCoordinatorProperties)
             throws Exception
     {
         // The property "hive.allow-drop-table" needs to be set to true because security is always "legacy" in NativeQueryRunner.
@@ -282,8 +286,11 @@ public static QueryRunner createNativeQueryRunner(
                         .put("http-server.http.port", "8081")
                         .put("experimental.internal-communication.thrift-transport-enabled", String.valueOf(useThrift))
                         .putAll(getNativeWorkerSystemProperties())
+                        .putAll(extraProperties)
+                        .build(),
+                ImmutableMap.<String, String>builder()
+                        .putAll(extraCoordinatorProperties)
                         .build(),
-                ImmutableMap.of(),
                 "legacy",
                 hiveProperties,
                 workerCount,
@@ -340,6 +347,26 @@ public static QueryRunner createNativeQueryRunner(String remoteFunctionServerUds
         return createNativeQueryRunner(false, DEFAULT_STORAGE_FORMAT, Optional.ofNullable(remoteFunctionServerUds), false);
     }
 
+    public static QueryRunner createNativeQueryRunner(Map<String, String> extraProperties, Map<String, String> extraCoordinatorProperties,
+                                                      String storageFormat)
+            throws Exception
+    {
+        int cacheMaxSize = 0;
+        NativeQueryRunnerParameters nativeQueryRunnerParameters = getNativeQueryRunnerParameters();
+        return createNativeQueryRunner(
+                nativeQueryRunnerParameters.dataDirectory.toString(),
+                nativeQueryRunnerParameters.serverBinary.toString(),
+                nativeQueryRunnerParameters.workerCount,
+                cacheMaxSize,
+                true,
+                Optional.empty(),
+                storageFormat,
+                true,
+                false,
+                extraProperties,
+                extraCoordinatorProperties);
+    }
+
     public static QueryRunner createNativeQueryRunner(boolean useThrift)
             throws Exception
     {
@@ -372,7 +399,9 @@ public static QueryRunner createNativeQueryRunner(boolean useThrift, String stor
                 remoteFunctionServerUds,
                 storageFormat,
                 true,
-                failOnNestedLoopJoin);
+                failOnNestedLoopJoin,
+                Collections.emptyMap(),
+                Collections.emptyMap());
     }
 
     // Start the remote function server. Return the UDS path used to communicate with it.

@@ -0,0 +1,31 @@
+# Presto Native Tests
+
+This module contains end-to-end tests that run queries from test classes in 
+the `presto-tests` module with Presto C++ workers. Please build the module
+`presto-native-execution` first. 
+
+The following command can be used to run all tests in this module:
+```
+mvn test 
+    -pl 'presto-native-tests' 
+    -Dtest="com.facebook.presto.nativetests.Test*" 
+    -Duser.timezone=America/Bahia_Banderas 
+    -DPRESTO_SERVER=${PRESTO_HOME}/presto-native-execution/cmake-build-debug/presto_cpp/main/presto_server 
+    -DWORKER_COUNT=${WORKER_COUNT} -T1C
+```
+Please update JVM argument `PRESTO_SERVER` to point to the Presto C++ worker
+binary `presto_server`. 
+
+## Adding new tests
+
+Presto C++ currently does not have the same behavior as Presto for certain 
+queries. This could be because of missing types, missing function signatures,
+among other reasons. Tests with these unsupported queries are therefore 
+expected to fail and the test asserts the error message is as expected. 
+
+Issues should also be created for the failing queries, so they are documented
+and fixed. Please add the tag `presto-native-tests` for these issues. 
+Once all the failures in a testcase are fixed, the overriden test in this 
+module should be removed and the testcase in the corresponding base class in
+`presto-tests` would be the single source of truth for Presto SQL coverage 
+tests.  
@@ -0,0 +1,121 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<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>com.facebook.presto</groupId>
+        <artifactId>presto-root</artifactId>
+        <version>0.290-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>presto-native-tests</artifactId>
+    <name>presto-native-tests</name>
+    <description>Presto Native Tests</description>
+
+    <properties>
+        <air.main.basedir>${project.parent.basedir}</air.main.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.testng</groupId>
+            <artifactId>testng</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-native-execution</artifactId>
+            <version>0.290-SNAPSHOT</version>
+            <type>test-jar</type>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-common</artifactId>
+        </dependency>
+
+        <!-- Presto SPI -->
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-spi</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-main</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-tests</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>com.google.guava</groupId>
+            <artifactId>guava</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-tpcds</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.jetbrains</groupId>
+            <artifactId>annotations</artifactId>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <!-- Disable git-commit-id-plugin plugin to allow for running tests without
+            a git checkout -->
+            <plugin>
+                <groupId>pl.project13.maven</groupId>
+                <artifactId>git-commit-id-plugin</artifactId>
+                <configuration>
+                    <skip>true</skip>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.basepom.maven</groupId>
+                <artifactId>duplicate-finder-maven-plugin</artifactId>
+                <configuration>
+                    <ignoredResourcePatterns>
+                        <ignoredResourcePattern>parquet.thrift</ignoredResourcePattern>
+                        <ignoredResourcePattern>about.html</ignoredResourcePattern>
+                        <ignoredResourcePattern>mozilla/public-suffix-list.txt</ignoredResourcePattern>
+                        <ignoredResourcePattern>iceberg-build.properties</ignoredResourcePattern>
+                        <ignoredResourcePattern>org.apache.avro.data/Json.avsc</ignoredResourcePattern>
+                    </ignoredResourcePatterns>
+                    <ignoredClassPatterns>
+                        <ignoredClassPattern>com.esotericsoftware.kryo.*</ignoredClassPattern>
+                        <ignoredClassPattern>com.esotericsoftware.minlog.Log</ignoredClassPattern>
+                        <ignoredClassPattern>com.esotericsoftware.reflectasm.*</ignoredClassPattern>
+                        <ignoredClassPattern>module-info</ignoredClassPattern>
+                        <ignoredClassPattern>META-INF.versions.9.module-info</ignoredClassPattern>
+                        <ignoredClassPattern>org.apache.avro.*</ignoredClassPattern>
+                        <ignoredClassPattern>com.github.benmanes.caffeine.*</ignoredClassPattern>
+                        <ignoredClassPattern>org.roaringbitmap.*</ignoredClassPattern>
+                    </ignoredClassPatterns>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <argLine>-Xms4g -Xmx4g</argLine>
+                    <forkCount>1</forkCount>
+                    <reuseForks>false</reuseForks>
+                    <excludedGroups>remote-function,textfile_reader</excludedGroups>
+                    <systemPropertyVariables>
+                        <PRESTO_SERVER>/root/project/build/debug/presto_cpp/main/presto_server</PRESTO_SERVER>
+                    </systemPropertyVariables>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+</project>