Skip to content

Commit 41e2705

Browse files
pdabre12Pratik Joseph Dabre
pdabre12
authored and
Pratik Joseph Dabre
committed
[native] Introduce presto-native-sql-invoked-functions-plugin for sidecar enabled clusters
 Adds a new plugin : presto-native-sql-invoked-functions-plugin that contains all inlined SQL functions except those with overridden native implementations. This plugin is intended to be loaded only in sidecar enabled clusters.
1 parent 3257215 commit 41e2705

File tree

17 files changed

+394
-17
lines changed

17 files changed

+394
-17
lines changed

‎.github/workflows/prestocpp-linux-build-and-unit-test.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ jobs:
370370
# Use different Maven options to install.
371371
MAVEN_OPTS: "-Xmx2G -XX:+ExitOnOutOfMemoryError"
372372
run: |
373-
for i in $(seq 1 3); do ./mvnw clean install $MAVEN_FAST_INSTALL -pl 'presto-native-execution' -am && s=0 && break || s=$? && sleep 10; done; (exit $s)
373+
for i in $(seq 1 3); do ./mvnw clean install $MAVEN_FAST_INSTALL -pl 'presto-native-sidecar-plugin' -am && s=0 && break || s=$? && sleep 10; done; (exit $s)
374374
375375
- name: Run presto-native sidecar tests
376376
if: |

‎pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@
224224
<module>presto-router-example-plugin-scheduler</module>
225225
<module>presto-plan-checker-router-plugin</module>
226226
<module>presto-sql-invoked-functions-plugin</module>
227+
<module>presto-native-sql-invoked-functions-plugin</module>
227228
</modules>
228229

229230
<dependencyManagement>

‎presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/KeyBasedSampler.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package com.facebook.presto.sql.planner.optimizations;
1515

1616
import com.facebook.presto.Session;
17-
import com.facebook.presto.common.QualifiedObjectName;
1817
import com.facebook.presto.common.function.OperatorType;
1918
import com.facebook.presto.common.type.Type;
2019
import com.facebook.presto.common.type.Varchars;
@@ -55,7 +54,6 @@
5554
import static com.facebook.presto.common.type.BooleanType.BOOLEAN;
5655
import static com.facebook.presto.common.type.DoubleType.DOUBLE;
5756
import static com.facebook.presto.common.type.VarcharType.VARCHAR;
58-
import static com.facebook.presto.metadata.BuiltInTypeAndFunctionNamespaceManager.JAVA_BUILTIN_NAMESPACE;
5957
import static com.facebook.presto.metadata.CastType.CAST;
6058
import static com.facebook.presto.spi.StandardErrorCode.FUNCTION_NOT_FOUND;
6159
import static com.facebook.presto.spi.StandardWarningCode.SAMPLED_FIELDS;
@@ -150,7 +148,7 @@ private PlanNode addSamplingFilter(PlanNode tableScanNode, Optional<VariableRefe
150148
try {
151149
sampledArg = call(
152150
functionAndTypeManager,
153-
QualifiedObjectName.valueOf(JAVA_BUILTIN_NAMESPACE, getKeyBasedSamplingFunction(session)),
151+
getKeyBasedSamplingFunction(session),
154152
DOUBLE,
155153
ImmutableList.of(arg));
156154
}

‎presto-main-base/src/main/java/com/facebook/presto/sql/relational/Expressions.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
*/
1414
package com.facebook.presto.sql.relational;
1515

16-
import com.facebook.presto.common.QualifiedObjectName;
1716
import com.facebook.presto.common.function.OperatorType;
1817
import com.facebook.presto.common.type.Type;
1918
import com.facebook.presto.metadata.CastType;
@@ -154,12 +153,6 @@ public static CallExpression call(FunctionAndTypeManager functionAndTypeManager,
154153
return call(name, functionHandle, returnType, arguments);
155154
}
156155

157-
public static CallExpression call(FunctionAndTypeManager functionAndTypeManager, QualifiedObjectName qualifiedObjectName, Type returnType, List<RowExpression> arguments)
158-
{
159-
FunctionHandle functionHandle = functionAndTypeManager.lookupFunction(qualifiedObjectName, fromTypes(arguments.stream().map(RowExpression::getType).collect(toImmutableList())));
160-
return call(String.valueOf(qualifiedObjectName), functionHandle, returnType, arguments);
161-
}
162-
163156
public static CallExpression call(FunctionAndTypeResolver functionAndTypeResolver, String name, Type returnType, RowExpression... arguments)
164157
{
165158
FunctionHandle functionHandle = functionAndTypeResolver.lookupFunction(name, fromTypes(Arrays.stream(arguments).map(RowExpression::getType).collect(toImmutableList())));

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ private NativeQueryRunnerUtils() {}
2929
public static Map<String, String> getNativeWorkerHiveProperties()
3030
{
3131
return ImmutableMap.of("hive.parquet.pushdown-filter-enabled", "true",
32-
"hive.orc-compression-codec", "ZSTD", "hive.storage-format", "DWRF");
32+
"hive.orc-compression-codec", "ZSTD", "hive.storage-format", "DWRF");
3333
}
3434

3535
public static Map<String, String> getNativeWorkerIcebergProperties()
@@ -59,6 +59,8 @@ public static Map<String, String> getNativeSidecarProperties()
5959
.put("coordinator-sidecar-enabled", "true")
6060
.put("exclude-invalid-worker-session-properties", "true")
6161
.put("presto.default-namespace", "native.default")
62+
// inline-sql-functions is overridden to be true in sidecar enabled native clusters.
63+
.put("inline-sql-functions", "true")
6264
.build();
6365
}
6466

‎presto-native-sidecar-plugin/pom.xml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,25 @@
260260
</exclusion>
261261
</exclusions>
262262
</dependency>
263+
263264
<dependency>
264265
<groupId>com.facebook.presto</groupId>
265266
<artifactId>presto-built-in-worker-function-tools</artifactId>
267+
<version>${project.version}</version>
268+
</dependency>
269+
270+
<dependency>
271+
<groupId>com.facebook.presto</groupId>
272+
<artifactId>presto-native-sql-invoked-functions-plugin</artifactId>
273+
<version>${project.version}</version>
274+
<scope>test</scope>
275+
</dependency>
276+
277+
<dependency>
278+
<groupId>com.facebook.presto</groupId>
279+
<artifactId>presto-sql-invoked-functions-plugin</artifactId>
280+
<version>${project.version}</version>
281+
<scope>test</scope>
266282
</dependency>
267283
</dependencies>
268284

‎presto-native-sidecar-plugin/src/test/java/com/facebook/presto/sidecar/NativeSidecarPluginQueryRunnerUtils.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
*/
1414
package com.facebook.presto.sidecar;
1515

16+
import com.facebook.presto.scalar.sql.NativeSqlInvokedFunctionsPlugin;
1617
import com.facebook.presto.sidecar.functionNamespace.NativeFunctionNamespaceManagerFactory;
1718
import com.facebook.presto.sidecar.sessionpropertyproviders.NativeSystemSessionPropertyProviderFactory;
1819
import com.facebook.presto.sidecar.typemanager.NativeTypeManagerFactory;
@@ -37,5 +38,6 @@ public static void setupNativeSidecarPlugin(QueryRunner queryRunner)
3738
"function-implementation-type", "CPP"));
3839
queryRunner.loadTypeManager(NativeTypeManagerFactory.NAME);
3940
queryRunner.loadPlanCheckerProviderManager("native", ImmutableMap.of());
41+
queryRunner.installPlugin(new NativeSqlInvokedFunctionsPlugin());
4042
}
4143
}

‎presto-native-sidecar-plugin/src/test/java/com/facebook/presto/sidecar/TestNativeSidecarPlugin.java‎

Lines changed: 123 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import com.facebook.airlift.units.DataSize;
1717
import com.facebook.presto.Session;
1818
import com.facebook.presto.nativeworker.PrestoNativeQueryRunnerUtils;
19+
import com.facebook.presto.scalar.sql.NativeSqlInvokedFunctionsPlugin;
20+
import com.facebook.presto.scalar.sql.SqlInvokedFunctionsPlugin;
1921
import com.facebook.presto.sidecar.functionNamespace.FunctionDefinitionProvider;
2022
import com.facebook.presto.sidecar.functionNamespace.NativeFunctionDefinitionProvider;
2123
import com.facebook.presto.sidecar.functionNamespace.NativeFunctionNamespaceManager;
@@ -46,8 +48,12 @@
4648

4749
import static com.facebook.airlift.units.DataSize.Unit.MEGABYTE;
4850
import static com.facebook.presto.SystemSessionProperties.REMOVE_MAP_CAST;
51+
import static com.facebook.presto.SystemSessionProperties.INLINE_SQL_FUNCTIONS;
52+
import static com.facebook.presto.SystemSessionProperties.KEY_BASED_SAMPLING_ENABLED;
53+
import static com.facebook.presto.SystemSessionProperties.REMOVE_MAP_CAST;
4954
import static com.facebook.presto.common.Utils.checkArgument;
5055
import static com.facebook.presto.common.type.BigintType.BIGINT;
56+
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createCustomer;
5157
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createLineitem;
5258
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createNation;
5359
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createOrders;
@@ -65,6 +71,7 @@ public class TestNativeSidecarPlugin
6571
private static final String REGEX_FUNCTION_NAMESPACE = "native.default.*";
6672
private static final String REGEX_SESSION_NAMESPACE = "Native Execution only.*";
6773
private static final long SIDECAR_HTTP_CLIENT_MAX_CONTENT_SIZE_MB = 128;
74+
private static final int INLINED_SQL_FUNCTIONS_COUNT = 7;
6875

6976
@Override
7077
protected void createTables()
@@ -75,6 +82,7 @@ protected void createTables()
7582
createOrders(queryRunner);
7683
createOrdersEx(queryRunner);
7784
createRegion(queryRunner);
85+
createCustomer(queryRunner);
7886
}
7987

8088
@Override
@@ -93,9 +101,11 @@ protected QueryRunner createQueryRunner()
93101
protected QueryRunner createExpectedQueryRunner()
94102
throws Exception
95103
{
96-
return PrestoNativeQueryRunnerUtils.javaHiveQueryRunnerBuilder()
104+
QueryRunner queryRunner = PrestoNativeQueryRunnerUtils.javaHiveQueryRunnerBuilder()
97105
.setAddStorageFormatToPath(true)
98106
.build();
107+
queryRunner.installPlugin(new SqlInvokedFunctionsPlugin());
108+
return queryRunner;
99109
}
100110

101111
public static void setupNativeSidecarPlugin(QueryRunner queryRunner)
@@ -113,6 +123,7 @@ public static void setupNativeSidecarPlugin(QueryRunner queryRunner)
113123
"sidecar.http-client.max-content-length", SIDECAR_HTTP_CLIENT_MAX_CONTENT_SIZE_MB + "MB"));
114124
queryRunner.loadTypeManager(NativeTypeManagerFactory.NAME);
115125
queryRunner.loadPlanCheckerProviderManager("native", ImmutableMap.of());
126+
queryRunner.installPlugin(new NativeSqlInvokedFunctionsPlugin());
116127
}
117128

118129
@Test
@@ -163,6 +174,7 @@ public void testSetNativeWorkerSessionProperty()
163174
@Test
164175
public void testShowFunctions()
165176
{
177+
int inlinedSQLFunctionsCount = 0;
166178
@Language("SQL") String sql = "SHOW FUNCTIONS";
167179
MaterializedResult actualResult = computeActual(sql);
168180
List<MaterializedRow> actualRows = actualResult.getMaterializedRows();
@@ -176,11 +188,17 @@ public void testShowFunctions()
176188

177189
// function namespace should be present.
178190
String fullFunctionName = row.get(5).toString();
179-
if (Pattern.matches(REGEX_FUNCTION_NAMESPACE, fullFunctionName)) {
180-
continue;
191+
if (!Pattern.matches(REGEX_FUNCTION_NAMESPACE, fullFunctionName)) {
192+
// If no namespace match found, check if it's an inlined SQL Invoked function.
193+
String language = row.get(9).toString();
194+
if (language.equalsIgnoreCase("SQL")) {
195+
inlinedSQLFunctionsCount++;
196+
continue;
197+
}
198+
fail(format("No namespace match found for row: %s", row));
181199
}
182-
fail(format("No namespace match found for row: %s", row));
183200
}
201+
assertEquals(inlinedSQLFunctionsCount, INLINED_SQL_FUNCTIONS_COUNT);
184202
}
185203

186204
@Test
@@ -321,7 +339,7 @@ public void testApproxPercentile()
321339
public void testInformationSchemaTables()
322340
{
323341
assertQuery("select lower(table_name) from information_schema.tables "
324-
+ "where table_name = 'lineitem' or table_name = 'LINEITEM' ");
342+
+ "where table_name = 'lineitem' or table_name = 'LINEITEM' ");
325343
}
326344

327345
@Test
@@ -423,6 +441,106 @@ public void testRemoveMapCast()
423441
"values 0.5, 0.1");
424442
}
425443

444+
@Test
445+
public void testOverriddenInlinedSqlInvokedFunctions()
446+
{
447+
// String functions
448+
assertQuery("SELECT trail(comment, cast(nationkey as integer)) FROM nation");
449+
assertQuery("SELECT name, comment, replace_first(comment, 'iron', 'gold') from nation");
450+
451+
// Array functions
452+
assertQuery("SELECT array_intersect(ARRAY['apple', 'banana', 'cherry'], ARRAY['apple', 'mango', 'fig'])");
453+
assertQuery("SELECT array_frequency(split(comment, '')) from nation");
454+
assertQuery("SELECT array_duplicates(ARRAY[regionkey]), array_duplicates(ARRAY[comment]) from nation");
455+
assertQuery("SELECT array_has_duplicates(ARRAY[custkey]) from orders");
456+
assertQuery("SELECT array_max_by(ARRAY[comment], x -> length(x)) from orders");
457+
assertQuery("SELECT array_min_by(ARRAY[ROW('USA', 1), ROW('INDIA', 2), ROW('UK', 3)], x -> x[2])");
458+
assertQuery("SELECT array_sort_desc(map_keys(map_union(quantity_by_linenumber))) FROM orders_ex");
459+
assertQuery("SELECT remove_nulls(ARRAY[CAST(regionkey AS VARCHAR), comment, NULL]) from nation");
460+
assertQuery("SELECT array_top_n(ARRAY[CAST(nationkey AS VARCHAR)], 3) from nation");
461+
assertQuerySucceeds("SELECT array_sort_desc(quantities, x -> abs(x)) FROM orders_ex");
462+
463+
// Map functions
464+
assertQuery("SELECT map_normalize(MAP(ARRAY['a', 'b', 'c'], ARRAY[1, 4, 5]))");
465+
assertQuery("SELECT map_normalize(MAP(ARRAY['a', 'b', 'c'], ARRAY[1, 0, -1]))");
466+
assertQuery("SELECT name, map_normalize(MAP(ARRAY['regionkey', 'length'], ARRAY[regionkey, length(comment)])) from nation");
467+
assertQuery("SELECT name, map_remove_null_values(map(ARRAY['region', 'comment', 'nullable'], " +
468+
"ARRAY[CAST(regionkey AS VARCHAR), comment, NULL])) from nation");
469+
assertQuery("SELECT name, map_key_exists(map(ARRAY['nation', 'comment'], ARRAY[CAST(nationkey AS VARCHAR), comment]), 'comment') from nation");
470+
assertQuery("SELECT map_keys_by_top_n_values(MAP(ARRAY[orderkey], ARRAY[custkey]), 2) from orders");
471+
assertQuery("SELECT map_top_n(MAP(ARRAY[CAST(nationkey AS VARCHAR)], ARRAY[comment]), 3) from nation");
472+
assertQuery("SELECT map_top_n_keys(MAP(ARRAY[orderkey], ARRAY[custkey]), 3) from orders");
473+
assertQuery("SELECT map_top_n_values(MAP(ARRAY[orderkey], ARRAY[custkey]), 3) from orders");
474+
assertQuery("SELECT all_keys_match(MAP(ARRAY[comment], ARRAY[custkey]), k -> length(k) > 5) from orders");
475+
assertQuery("SELECT any_keys_match(MAP(ARRAY[comment], ARRAY[custkey]), k -> starts_with(k, 'abc')) from orders");
476+
assertQuery("SELECT any_values_match(MAP(ARRAY[orderkey], ARRAY[totalprice]), k -> abs(k) > 20) from orders");
477+
assertQuery("SELECT no_values_match(MAP(ARRAY[orderkey], ARRAY[comment]), k -> length(k) > 2) from orders");
478+
assertQuery("SELECT no_keys_match(MAP(ARRAY[comment], ARRAY[custkey]), k -> ends_with(k, 'a')) from orders");
479+
}
480+
481+
@Test
482+
public void testNonOverriddenInlinedSqlInvokedFunctionsWhenConfigEnabled()
483+
{
484+
// Array functions
485+
assertQuery("SELECT array_split_into_chunks(split(comment, ''), 2) from nation");
486+
assertQuery("SELECT array_least_frequent(quantities) from orders_ex");
487+
assertQuery("SELECT array_least_frequent(split(comment, ''), 5) from nation");
488+
assertQuerySucceeds("SELECT array_top_n(ARRAY[orderkey], 25, (x, y) -> if (x < y, cast(1 as bigint), if (x > y, cast(-1 as bigint), cast(0 as bigint)))) from orders");
489+
490+
// Map functions
491+
assertQuerySucceeds("SELECT map_top_n_values(MAP(ARRAY[comment], ARRAY[nationkey]), 2, (x, y) -> if (x < y, cast(1 as bigint), if (x > y, cast(-1 as bigint), cast(0 as bigint)))) from nation");
492+
assertQuerySucceeds("SELECT map_top_n_keys(MAP(ARRAY[regionkey], ARRAY[nationkey]), 5, (x, y) -> if (x < y, cast(1 as bigint), if (x > y, cast(-1 as bigint), cast(0 as bigint)))) from nation");
493+
494+
Session sessionWithKeyBasedSampling = Session.builder(getSession())
495+
.setSystemProperty(KEY_BASED_SAMPLING_ENABLED, "true")
496+
.build();
497+
498+
@Language("SQL") String query = "select count(1) FROM lineitem l left JOIN orders o ON l.orderkey = o.orderkey JOIN customer c ON o.custkey = c.custkey";
499+
500+
assertQuery(query, "select cast(60175 as bigint)");
501+
assertQuery(sessionWithKeyBasedSampling, query, "select cast(16185 as bigint)");
502+
}
503+
504+
@Test
505+
public void testNonOverriddenInlinedSqlInvokedFunctionsWhenConfigDisabled()
506+
{
507+
// When inline_sql_functions is set to false, the below queries should fail as the implementations don't exist on the native worker
508+
Session session = Session.builder(getSession())
509+
.setSystemProperty(KEY_BASED_SAMPLING_ENABLED, "true")
510+
.setSystemProperty(INLINE_SQL_FUNCTIONS, "false")
511+
.build();
512+
513+
// Array functions
514+
assertQueryFails(session,
515+
"SELECT array_split_into_chunks(split(comment, ''), 2) from nation",
516+
".*Scalar function name not registered: native.default.array_split_into_chunks.*");
517+
assertQueryFails(session,
518+
"SELECT array_least_frequent(quantities) from orders_ex",
519+
".*Scalar function name not registered: native.default.array_least_frequent.*");
520+
assertQueryFails(session,
521+
"SELECT array_least_frequent(split(comment, ''), 2) from nation",
522+
".*Scalar function name not registered: native.default.array_least_frequent.*");
523+
assertQueryFails(session,
524+
"SELECT array_top_n(ARRAY[orderkey], 25, (x, y) -> if (x < y, cast(1 as bigint), if (x > y, cast(-1 as bigint), cast(0 as bigint)))) from orders",
525+
" Scalar function native\\.default\\.array_top_n not registered with arguments.*",
526+
true);
527+
528+
// Map functions
529+
assertQueryFails(session,
530+
"SELECT map_top_n_values(MAP(ARRAY[comment], ARRAY[nationkey]), 2, (x, y) -> if (x < y, cast(1 as bigint), if (x > y, cast(-1 as bigint), cast(0 as bigint)))) from nation",
531+
".*Scalar function native\\.default\\.map_top_n_values not registered with arguments.*",
532+
true);
533+
assertQueryFails(session,
534+
"SELECT map_top_n_keys(MAP(ARRAY[regionkey], ARRAY[nationkey]), 5, (x, y) -> if (x < y, cast(1 as bigint), if (x > y, cast(-1 as bigint), cast(0 as bigint)))) from nation",
535+
".*Scalar function native\\.default\\.map_top_n_keys not registered with arguments.*",
536+
true);
537+
538+
assertQueryFails(session,
539+
"select count(1) FROM lineitem l left JOIN orders o ON l.orderkey = o.orderkey JOIN customer c ON o.custkey = c.custkey",
540+
".*Scalar function name not registered: native.default.key_sampling_percent.*");
541+
>>>>>>> 5497c150c0 ([native] Introduce presto-native-sql-invoked-functions-plugin for sidecar enabled clusters)
542+
}
543+
426544
private String generateRandomTableName()
427545
{
428546
String tableName = "tmp_presto_" + UUID.randomUUID().toString().replace("-", "");

‎presto-native-sql-invoked-functions-plugin/pom.xml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
<parent>
5+
<groupId>com.facebook.presto</groupId>
6+
<artifactId>presto-root</artifactId>
7+
<version>0.295-SNAPSHOT</version>
8+
</parent>
9+
10+
<artifactId>presto-native-sql-invoked-functions-plugin</artifactId>
11+
<description>Presto Native - Sql invoked functions plugin</description>
12+
<packaging>presto-plugin</packaging>
13+
14+
<properties>
15+
<air.main.basedir>${project.parent.basedir}</air.main.basedir>
16+
</properties>
17+
18+
<dependencies>
19+
<dependency>
20+
<groupId>com.facebook.presto</groupId>
21+
<artifactId>presto-spi</artifactId>
22+
<scope>provided</scope>
23+
</dependency>
24+
<dependency>
25+
<groupId>com.google.guava</groupId>
26+
<artifactId>guava</artifactId>
27+
</dependency>
28+
</dependencies>
29+
</project>

0 commit comments

Comments
 (0)