Skip to content

Commit 9652426

Browse files
authored
fix: Add coordinator validation interface for Python UDF security (#26368)
Summary: This is the first diff in a series to move Python UDF script validation from workers to the coordinator. This OSS diff (presto-trunk) adds the infrastructure for coordinator-side validation: - Adds `validateFunctionCall()` method to `FunctionNamespaceManager` SPI with a default no-op implementation - Adds `validateFunctionCall()` to `FunctionAndTypeResolver` interface - Calls validation from `ExpressionAnalyzer` during function resolution This allows function namespace managers to perform custom validation logic (e.g., security checks for Python UDF scripts) during SQL analysis on the coordinator, before distributing work to workers. The default implementation does nothing, maintaining backward compatibility. The actual validation logic for Python UDFs will be implemented in subsequent diffs in presto-facebook-trunk. Follow-up diffs will: 1. Implement the actual Python UDF validation in the coordinator (presto-facebook-trunk) 2. Remove the validation from Java workers (presto-facebook-trunk) 3. Remove the validation from Velox/Prestissimo workers (velox/functions/facebook) This is part of D84739785's plan to consolidate validation at the coordinator level for: - Fail-fast error reporting - Better error messages with full query context - Single validation point instead of redundant checks - Resource efficiency (reject invalid queries before allocating worker resources) Differential Revision: D85081306 ``` == NO RELEASE NOTE == ```
1 parent a3e53c4 commit 9652426

File tree

4 files changed

+46
-0
lines changed

4 files changed

+46
-0
lines changed

presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/FunctionAndTypeResolver.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,13 @@ FunctionHandle resolveFunction(
5252
FunctionHandle lookupCast(String castType, Type fromType, Type toType);
5353

5454
QualifiedObjectName qualifyObjectName(QualifiedName name);
55+
56+
/**
57+
* Validate a function call during analysis phase on the coordinator.
58+
* Delegates to the FunctionNamespaceManager for custom validation logic.
59+
*
60+
* @param functionHandle The function handle being validated
61+
* @param arguments Raw argument expressions (not yet evaluated)
62+
*/
63+
void validateFunctionCall(FunctionHandle functionHandle, List<?> arguments);
5564
}

presto-main-base/src/main/java/com/facebook/presto/metadata/FunctionAndTypeManager.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,12 @@ public FunctionHandle lookupCast(String castType, Type fromType, Type toType)
319319
return FunctionAndTypeManager.this.lookupCast(CastType.valueOf(castType), fromType, toType);
320320
}
321321

322+
@Override
323+
public void validateFunctionCall(FunctionHandle functionHandle, List<?> arguments)
324+
{
325+
FunctionAndTypeManager.this.validateFunctionCall(functionHandle, arguments);
326+
}
327+
322328
public QualifiedObjectName qualifyObjectName(QualifiedName name)
323329
{
324330
if (name.getSuffix().startsWith("$internal")) {
@@ -719,6 +725,19 @@ public CompletableFuture<SqlFunctionResult> executeFunction(String source, Funct
719725
return functionNamespaceManager.get().executeFunction(source, functionHandle, inputPage, channels, this);
720726
}
721727

728+
public void validateFunctionCall(FunctionHandle functionHandle, List<?> arguments)
729+
{
730+
// Built-in functions don't need validation
731+
if (functionHandle instanceof BuiltInFunctionHandle) {
732+
return;
733+
}
734+
735+
Optional<FunctionNamespaceManager<?>> functionNamespaceManager = getServingFunctionNamespaceManager(functionHandle.getCatalogSchemaName());
736+
if (functionNamespaceManager.isPresent()) {
737+
functionNamespaceManager.get().validateFunctionCall(functionHandle, arguments);
738+
}
739+
}
740+
722741
public WindowFunctionSupplier getWindowFunctionImplementation(FunctionHandle functionHandle)
723742
{
724743
return builtInTypeAndFunctionNamespaceManager.getWindowFunctionImplementation(functionHandle);

presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/ExpressionAnalyzer.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,10 @@ else if (frame.getType() == GROUPS) {
11201120
FunctionHandle function = resolveFunction(sessionFunctions, transactionId, node, argumentTypes, functionAndTypeResolver);
11211121
FunctionMetadata functionMetadata = functionAndTypeResolver.getFunctionMetadata(function);
11221122

1123+
// Delegate function-specific validation to the FunctionNamespaceManager
1124+
// This allows function namespaces to perform custom validation
1125+
functionAndTypeResolver.validateFunctionCall(function, node.getArguments());
1126+
11231127
if (node.getOrderBy().isPresent()) {
11241128
for (SortItem sortItem : node.getOrderBy().get().getSortItems()) {
11251129
Type sortKeyType = process(sortItem.getSortKey(), context);

presto-spi/src/main/java/com/facebook/presto/spi/function/FunctionNamespaceManager.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,18 @@ default AggregationFunctionImplementation getAggregateFunctionImplementation(Fun
122122
{
123123
throw new UnsupportedOperationException("Does not support get aggregation function");
124124
}
125+
126+
/**
127+
* Validate a function call during analysis phase on the coordinator.
128+
* This allows function namespace managers to perform custom validation logic
129+
* such as security checks, argument validation, etc.
130+
*
131+
* @param functionHandle The function handle being validated
132+
* @param arguments Raw argument expressions (not yet evaluated) - type is Object to avoid SPI module dependencies
133+
* @throws RuntimeException if validation fails
134+
*/
135+
default void validateFunctionCall(FunctionHandle functionHandle, List<?> arguments)
136+
{
137+
// Default implementation: no validation
138+
}
125139
}

0 commit comments

Comments
 (0)