Skip to content

Commit 47ecc65

Browse files
committed
Add presto-native-tests module
1 parent 053d93d commit 47ecc65

18 files changed

+3031
-4
lines changed

.circleci/continue_config.yml

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ workflows:
2929
run_linux_tests: << pipeline.parameters.run_linux_tests >>
3030
requires:
3131
- linux-build-and-unit-test
32+
- linux-presto-native-tests:
33+
run_linux_tests: << pipeline.parameters.run_linux_tests >>
34+
requires:
35+
- linux-build-and-unit-test
3236
- linux-spark-e2e-tests:
3337
run_linux_tests: << pipeline.parameters.run_linux_tests >>
3438
requires:
@@ -198,6 +202,54 @@ jobs:
198202
- store_artifacts:
199203
path: '/tmp/PrestoNativeQueryRunnerUtils'
200204

205+
linux-presto-native-tests:
206+
executor: build
207+
parameters:
208+
run_linux_tests:
209+
type: boolean
210+
default: false
211+
parallelism: 5
212+
steps:
213+
- run: echo "Run Linux tests is << parameters.run_linux_tests >>"
214+
- when:
215+
condition: << parameters.run_linux_tests >>
216+
steps:
217+
- checkout
218+
- attach_workspace:
219+
at: presto-native-execution
220+
- maven_install:
221+
maven_install_opts: ${MAVEN_INSTALL_OPTS}
222+
maven_fast_install: ${MAVEN_FAST_INSTALL}
223+
- run:
224+
name: 'Run Presto native tests'
225+
command: |
226+
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/usr/local/lib:/usr/local/lib64"
227+
export PRESTO_SERVER_PATH="${HOME}/project/presto-native-execution/_build/debug/presto_cpp/main/presto_server"
228+
export TEMP_PATH="/tmp"
229+
TESTFILES=$(circleci tests glob "presto-native-tests/src/test/**/Test*.java" | circleci tests split --split-by=timings)
230+
# Convert file paths to comma separated class names
231+
export TESTCLASSES=
232+
for test_file in $TESTFILES
233+
do
234+
tmp=${test_file##*/}
235+
test_class=${tmp%%\.*}
236+
export TESTCLASSES="${TESTCLASSES},$test_class"
237+
done
238+
export TESTCLASSES=${TESTCLASSES#,}
239+
240+
if [ ! -z $TESTCLASSES ]; then
241+
mvn test \
242+
${MAVEN_TEST} \
243+
-pl 'presto-native-tests' \
244+
-Dtest="${TESTCLASSES}" \
245+
-DPRESTO_SERVER=${PRESTO_SERVER_PATH} \
246+
-DDATA_DIR=${TEMP_PATH} \
247+
-Duser.timezone=America/Bahia_Banderas \
248+
-T1C
249+
fi
250+
- store_artifacts:
251+
path: '/tmp/PrestoNativeQueryRunnerUtils'
252+
201253
linux-presto-native-sidecar-tests:
202254
executor: build
203255
parameters:

.github/workflows/test-other-modules.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ jobs:
6464
run: |
6565
./mvnw test -T 1 ${MAVEN_TEST} -pl '
6666
!presto-tests,
67+
!presto-native-tests,
6768
!presto-accumulo,
6869
!presto-cassandra,
6970
!presto-hive,

pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@
195195
<module>presto-test-coverage</module>
196196
<module>presto-hudi</module>
197197
<module>presto-native-execution</module>
198+
<module>presto-native-tests</module>
198199
<module>presto-router</module>
199200
<module>presto-open-telemetry</module>
200201
<module>redis-hbo-provider</module>

presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@
4343
import java.nio.file.Files;
4444
import java.nio.file.Path;
4545
import java.nio.file.Paths;
46+
import java.util.Collections;
4647
import java.util.List;
48+
import java.util.Map;
4749
import java.util.Optional;
4850
import java.util.OptionalInt;
4951
import java.util.UUID;
@@ -121,7 +123,7 @@ public static QueryRunner createQueryRunner(
121123

122124
defaultQueryRunner.close();
123125

124-
return createNativeQueryRunner(dataDirectory.get().toString(), prestoServerPath.get(), workerCount, cacheMaxSize, true, Optional.empty(), storageFormat, addStorageFormatToPath, false, isCoordinatorSidecarEnabled);
126+
return createNativeQueryRunner(dataDirectory.get().toString(), prestoServerPath.get(), workerCount, cacheMaxSize, true, Optional.empty(), storageFormat, addStorageFormatToPath, false, isCoordinatorSidecarEnabled, Collections.emptyMap(), Collections.emptyMap());
125127
}
126128

127129
public static QueryRunner createJavaQueryRunner()
@@ -321,7 +323,9 @@ public static QueryRunner createNativeQueryRunner(
321323
String storageFormat,
322324
boolean addStorageFormatToPath,
323325
Boolean failOnNestedLoopJoin,
324-
boolean isCoordinatorSidecarEnabled)
326+
boolean isCoordinatorSidecarEnabled,
327+
Map<String, String> extraProperties,
328+
Map<String, String> extraCoordinatorProperties)
325329
throws Exception
326330
{
327331
// The property "hive.allow-drop-table" needs to be set to true because security is always "legacy" in NativeQueryRunner.
@@ -339,8 +343,11 @@ public static QueryRunner createNativeQueryRunner(
339343
.put("experimental.internal-communication.thrift-transport-enabled", String.valueOf(useThrift))
340344
.putAll(getNativeWorkerSystemProperties())
341345
.putAll(isCoordinatorSidecarEnabled ? getNativeSidecarProperties() : ImmutableMap.of())
346+
.putAll(extraProperties)
347+
.build(),
348+
ImmutableMap.<String, String>builder()
349+
.putAll(extraCoordinatorProperties)
342350
.build(),
343-
ImmutableMap.of(),
344351
"legacy",
345352
hiveProperties,
346353
workerCount,
@@ -397,6 +404,27 @@ public static QueryRunner createNativeQueryRunner(String remoteFunctionServerUds
397404
return createNativeQueryRunner(false, DEFAULT_STORAGE_FORMAT, Optional.ofNullable(remoteFunctionServerUds), false, false);
398405
}
399406

407+
public static QueryRunner createNativeQueryRunner(Map<String, String> extraProperties, Map<String, String> extraCoordinatorProperties,
408+
String storageFormat)
409+
throws Exception
410+
{
411+
int cacheMaxSize = 0;
412+
NativeQueryRunnerParameters nativeQueryRunnerParameters = getNativeQueryRunnerParameters();
413+
return createNativeQueryRunner(
414+
nativeQueryRunnerParameters.dataDirectory.toString(),
415+
nativeQueryRunnerParameters.serverBinary.toString(),
416+
nativeQueryRunnerParameters.workerCount,
417+
cacheMaxSize,
418+
true,
419+
Optional.empty(),
420+
storageFormat,
421+
true,
422+
false,
423+
false,
424+
extraProperties,
425+
extraCoordinatorProperties);
426+
}
427+
400428
public static QueryRunner createNativeQueryRunner(boolean useThrift)
401429
throws Exception
402430
{
@@ -430,7 +458,9 @@ public static QueryRunner createNativeQueryRunner(boolean useThrift, String stor
430458
storageFormat,
431459
true,
432460
failOnNestedLoopJoin,
433-
isCoordinatorSidecarEnabled);
461+
isCoordinatorSidecarEnabled,
462+
Collections.emptyMap(),
463+
Collections.emptyMap());
434464
}
435465

436466
// Start the remote function server. Return the UDS path used to communicate with it.

presto-native-tests/README.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Presto Native Tests
2+
3+
This module contains end-to-end tests that run queries from test classes in
4+
the `presto-tests` module with Presto C++ workers. Please build the module
5+
`presto-native-execution` first.
6+
7+
The following command can be used to run all tests in this module:
8+
```
9+
mvn test
10+
-pl 'presto-native-tests'
11+
-Dtest="com.facebook.presto.nativetests.Test*"
12+
-Duser.timezone=America/Bahia_Banderas
13+
-DPRESTO_SERVER=${PRESTO_HOME}/presto-native-execution/cmake-build-debug/presto_cpp/main/presto_server
14+
-DWORKER_COUNT=${WORKER_COUNT} -T1C
15+
```
16+
Please update JVM argument `PRESTO_SERVER` to point to the Presto C++ worker
17+
binary `presto_server`.
18+
19+
## Adding new tests
20+
21+
Presto C++ currently does not have the same behavior as Presto for certain
22+
queries. This could be because of missing types, missing function signatures,
23+
among other reasons. Tests with these unsupported queries are therefore
24+
expected to fail and the test asserts the error message is as expected.
25+
26+
Issues should also be created for the failing queries, so they are documented
27+
and fixed. Please add the tag `presto-native-tests` for these issues.
28+
Once all the failures in a testcase are fixed, the overriden test in this
29+
module should be removed and the testcase in the corresponding base class in
30+
`presto-tests` would be the single source of truth for Presto SQL coverage
31+
tests.

presto-native-tests/pom.xml

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<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">
3+
<modelVersion>4.0.0</modelVersion>
4+
5+
<parent>
6+
<groupId>com.facebook.presto</groupId>
7+
<artifactId>presto-root</artifactId>
8+
<version>0.291-SNAPSHOT</version>
9+
</parent>
10+
11+
<artifactId>presto-native-tests</artifactId>
12+
<name>presto-native-tests</name>
13+
<description>Presto Native Tests</description>
14+
15+
<properties>
16+
<air.main.basedir>${project.parent.basedir}</air.main.basedir>
17+
</properties>
18+
19+
<dependencies>
20+
<dependency>
21+
<groupId>org.testng</groupId>
22+
<artifactId>testng</artifactId>
23+
</dependency>
24+
25+
<dependency>
26+
<groupId>com.facebook.presto</groupId>
27+
<artifactId>presto-native-execution</artifactId>
28+
<version>${project.version}</version>
29+
<type>test-jar</type>
30+
<scope>test</scope>
31+
</dependency>
32+
33+
<dependency>
34+
<groupId>com.facebook.presto</groupId>
35+
<artifactId>presto-common</artifactId>
36+
</dependency>
37+
38+
<!-- Presto SPI -->
39+
<dependency>
40+
<groupId>com.facebook.presto</groupId>
41+
<artifactId>presto-spi</artifactId>
42+
</dependency>
43+
44+
<dependency>
45+
<groupId>com.facebook.presto</groupId>
46+
<artifactId>presto-main</artifactId>
47+
</dependency>
48+
49+
<dependency>
50+
<groupId>com.facebook.presto</groupId>
51+
<artifactId>presto-tests</artifactId>
52+
<version>${project.version}</version>
53+
</dependency>
54+
55+
<dependency>
56+
<groupId>com.google.guava</groupId>
57+
<artifactId>guava</artifactId>
58+
</dependency>
59+
60+
<dependency>
61+
<groupId>com.facebook.presto</groupId>
62+
<artifactId>presto-tpcds</artifactId>
63+
<scope>test</scope>
64+
</dependency>
65+
66+
<dependency>
67+
<groupId>org.jetbrains</groupId>
68+
<artifactId>annotations</artifactId>
69+
<scope>test</scope>
70+
</dependency>
71+
</dependencies>
72+
73+
<build>
74+
<plugins>
75+
<!-- Disable git-commit-id-plugin plugin to allow for running tests without
76+
a git checkout -->
77+
<plugin>
78+
<groupId>pl.project13.maven</groupId>
79+
<artifactId>git-commit-id-plugin</artifactId>
80+
<configuration>
81+
<skip>true</skip>
82+
</configuration>
83+
</plugin>
84+
<plugin>
85+
<groupId>org.basepom.maven</groupId>
86+
<artifactId>duplicate-finder-maven-plugin</artifactId>
87+
<configuration>
88+
<ignoredResourcePatterns>
89+
<ignoredResourcePattern>parquet.thrift</ignoredResourcePattern>
90+
<ignoredResourcePattern>about.html</ignoredResourcePattern>
91+
<ignoredResourcePattern>mozilla/public-suffix-list.txt</ignoredResourcePattern>
92+
<ignoredResourcePattern>iceberg-build.properties</ignoredResourcePattern>
93+
<ignoredResourcePattern>org.apache.avro.data/Json.avsc</ignoredResourcePattern>
94+
</ignoredResourcePatterns>
95+
<ignoredClassPatterns>
96+
<ignoredClassPattern>com.esotericsoftware.kryo.*</ignoredClassPattern>
97+
<ignoredClassPattern>com.esotericsoftware.minlog.Log</ignoredClassPattern>
98+
<ignoredClassPattern>com.esotericsoftware.reflectasm.*</ignoredClassPattern>
99+
<ignoredClassPattern>module-info</ignoredClassPattern>
100+
<ignoredClassPattern>META-INF.versions.9.module-info</ignoredClassPattern>
101+
<ignoredClassPattern>org.apache.avro.*</ignoredClassPattern>
102+
<ignoredClassPattern>com.github.benmanes.caffeine.*</ignoredClassPattern>
103+
<ignoredClassPattern>org.roaringbitmap.*</ignoredClassPattern>
104+
</ignoredClassPatterns>
105+
</configuration>
106+
</plugin>
107+
<plugin>
108+
<groupId>org.apache.maven.plugins</groupId>
109+
<artifactId>maven-surefire-plugin</artifactId>
110+
<configuration>
111+
<argLine>-Xms4g -Xmx4g</argLine>
112+
<forkCount>1</forkCount>
113+
<reuseForks>false</reuseForks>
114+
<excludedGroups>remote-function,textfile_reader</excludedGroups>
115+
<systemPropertyVariables>
116+
<PRESTO_SERVER>/root/project/build/debug/presto_cpp/main/presto_server</PRESTO_SERVER>
117+
</systemPropertyVariables>
118+
</configuration>
119+
</plugin>
120+
</plugins>
121+
</build>
122+
</project>

0 commit comments

Comments
 (0)