Skip to content

Commit 0458101

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 0458101

File tree

17 files changed

+392
-17
lines changed

17 files changed

+392
-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: 121 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;
@@ -45,9 +47,12 @@
4547
import java.util.stream.Collectors;
4648

4749
import static com.facebook.airlift.units.DataSize.Unit.MEGABYTE;
50+
import static com.facebook.presto.SystemSessionProperties.INLINE_SQL_FUNCTIONS;
51+
import static com.facebook.presto.SystemSessionProperties.KEY_BASED_SAMPLING_ENABLED;
4852
import static com.facebook.presto.SystemSessionProperties.REMOVE_MAP_CAST;
4953
import static com.facebook.presto.common.Utils.checkArgument;
5054
import static com.facebook.presto.common.type.BigintType.BIGINT;
55+
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createCustomer;
5156
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createLineitem;
5257
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createNation;
5358
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createOrders;
@@ -65,6 +70,7 @@ public class TestNativeSidecarPlugin
6570
private static final String REGEX_FUNCTION_NAMESPACE = "native.default.*";
6671
private static final String REGEX_SESSION_NAMESPACE = "Native Execution only.*";
6772
private static final long SIDECAR_HTTP_CLIENT_MAX_CONTENT_SIZE_MB = 128;
73+
private static final int INLINED_SQL_FUNCTIONS_COUNT = 7;
6874

6975
@Override
7076
protected void createTables()
@@ -75,6 +81,7 @@ protected void createTables()
7581
createOrders(queryRunner);
7682
createOrdersEx(queryRunner);
7783
createRegion(queryRunner);
84+
createCustomer(queryRunner);
7885
}
7986

8087
@Override
@@ -93,9 +100,11 @@ protected QueryRunner createQueryRunner()
93100
protected QueryRunner createExpectedQueryRunner()
94101
throws Exception
95102
{
96-
return PrestoNativeQueryRunnerUtils.javaHiveQueryRunnerBuilder()
103+
QueryRunner queryRunner = PrestoNativeQueryRunnerUtils.javaHiveQueryRunnerBuilder()
97104
.setAddStorageFormatToPath(true)
98105
.build();
106+
queryRunner.installPlugin(new SqlInvokedFunctionsPlugin());
107+
return queryRunner;
99108
}
100109

101110
public static void setupNativeSidecarPlugin(QueryRunner queryRunner)
@@ -113,6 +122,7 @@ public static void setupNativeSidecarPlugin(QueryRunner queryRunner)
113122
"sidecar.http-client.max-content-length", SIDECAR_HTTP_CLIENT_MAX_CONTENT_SIZE_MB + "MB"));
114123
queryRunner.loadTypeManager(NativeTypeManagerFactory.NAME);
115124
queryRunner.loadPlanCheckerProviderManager("native", ImmutableMap.of());
125+
queryRunner.installPlugin(new NativeSqlInvokedFunctionsPlugin());
116126
}
117127

118128
@Test
@@ -163,6 +173,7 @@ public void testSetNativeWorkerSessionProperty()
163173
@Test
164174
public void testShowFunctions()
165175
{
176+
int inlinedSQLFunctionsCount = 0;
166177
@Language("SQL") String sql = "SHOW FUNCTIONS";
167178
MaterializedResult actualResult = computeActual(sql);
168179
List<MaterializedRow> actualRows = actualResult.getMaterializedRows();
@@ -176,11 +187,17 @@ public void testShowFunctions()
176187

177188
// function namespace should be present.
178189
String fullFunctionName = row.get(5).toString();
179-
if (Pattern.matches(REGEX_FUNCTION_NAMESPACE, fullFunctionName)) {
180-
continue;
190+
if (!Pattern.matches(REGEX_FUNCTION_NAMESPACE, fullFunctionName)) {
191+
// If no namespace match found, check if it's an inlined SQL Invoked function.
192+
String language = row.get(9).toString();
193+
if (language.equalsIgnoreCase("SQL")) {
194+
inlinedSQLFunctionsCount++;
195+
continue;
196+
}
197+
fail(format("No namespace match found for row: %s", row));
181198
}
182-
fail(format("No namespace match found for row: %s", row));
183199
}
200+
assertEquals(inlinedSQLFunctionsCount, INLINED_SQL_FUNCTIONS_COUNT);
184201
}
185202

186203
@Test
@@ -321,7 +338,7 @@ public void testApproxPercentile()
321338
public void testInformationSchemaTables()
322339
{
323340
assertQuery("select lower(table_name) from information_schema.tables "
324-
+ "where table_name = 'lineitem' or table_name = 'LINEITEM' ");
341+
+ "where table_name = 'lineitem' or table_name = 'LINEITEM' ");
325342
}
326343

327344
@Test
@@ -423,6 +440,105 @@ public void testRemoveMapCast()
423440
"values 0.5, 0.1");
424441
}
425442

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