From fd81ad1c524e31845592d5f693032e0d74c43e92 Mon Sep 17 00:00:00 2001 From: mohsaka <135669458+mohsaka@users.noreply.github.com> Date: Fri, 7 Mar 2025 14:18:27 -0800 Subject: [PATCH 1/5] Add builders for table function arguments Changes adapted from trino/PR#12350 Original commit: 998315075343beecef962657b8cbf440d53cc13b Author: kasiafi Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com> --- .../sql/analyzer/StatementAnalyzer.java | 10 ++- .../function/table/ArgumentSpecification.java | 2 +- .../function/table/DescriptorArgument.java | 31 +++++++-- .../DescriptorArgumentSpecification.java | 35 ++++++++-- .../spi/function/table/ScalarArgument.java | 30 +++++++++ .../table/ScalarArgumentSpecification.java | 51 +++++++++++--- .../spi/function/table/TableArgument.java | 66 +++++++++++++++++++ .../table/TableArgumentSpecification.java | 52 +++++++++++++-- 8 files changed, 248 insertions(+), 29 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java index 5d9d50fba03e4..b578a795644f7 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java @@ -1477,7 +1477,10 @@ else if (argument.getValue() instanceof Expression) { Type expectedArgumentType = ((ScalarArgumentSpecification) argumentSpecification).getType(); // currently, only constant arguments are supported Object constantValue = ExpressionInterpreter.evaluateConstantExpression(expression, expectedArgumentType, metadata, session, analysis.getParameters()); - return new ScalarArgument(expectedArgumentType, constantValue); // TODO test coercion, test parameter + return ScalarArgument.builder() + .type(expectedArgumentType) + .value(constantValue) + .build(); // TODO test coercion, test parameter } throw new IllegalStateException("Unexpected argument specification: " + argumentSpecification.getClass().getSimpleName()); @@ -1495,7 +1498,10 @@ private Argument analyzeDefault(ArgumentSpecification argumentSpecification, Nod throw new SemanticException(NOT_SUPPORTED, errorLocation, "Descriptor arguments are not yet supported for table functions"); } if (argumentSpecification instanceof ScalarArgumentSpecification) { - return new ScalarArgument(((ScalarArgumentSpecification) argumentSpecification).getType(), argumentSpecification.getDefaultValue()); + return ScalarArgument.builder() + .type(((ScalarArgumentSpecification) argumentSpecification).getType()) + .value(argumentSpecification.getDefaultValue()) + .build(); } throw new IllegalStateException("Unexpected argument specification: " + argumentSpecification.getClass().getSimpleName()); diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java index f9572cc2fe045..ade58aa14fe35 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java @@ -36,7 +36,7 @@ public abstract class ArgumentSpecification // native representation private final Object defaultValue; - public ArgumentSpecification(String name, boolean required, @Nullable Object defaultValue) + ArgumentSpecification(String name, boolean required, @Nullable Object defaultValue) { this.name = checkNotNullOrEmpty(name, "name"); checkArgument(!required || defaultValue == null, "non-null default value for a required argument"); diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgument.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgument.java index 97960b11b318c..b1f0b32b659c0 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgument.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgument.java @@ -29,15 +29,9 @@ public class DescriptorArgument extends Argument { - public static final DescriptorArgument NULL_DESCRIPTOR = new DescriptorArgument(Optional.empty()); + public static final DescriptorArgument NULL_DESCRIPTOR = builder().build(); private final Optional descriptor; - public static DescriptorArgument descriptorArgument(Descriptor descriptor) - { - requireNonNull(descriptor, "descriptor is null"); - return new DescriptorArgument(Optional.of(descriptor)); - } - @JsonCreator private DescriptorArgument(@JsonProperty("descriptor") Optional descriptor) { @@ -49,4 +43,27 @@ public Optional getDescriptor() { return descriptor; } + + public static Builder builder() + { + return new Builder(); + } + + public static final class Builder + { + private Descriptor descriptor; + + private Builder() {} + + public Builder descriptor(Descriptor descriptor) + { + this.descriptor = descriptor; + return this; + } + + public DescriptorArgument build() + { + return new DescriptorArgument(Optional.ofNullable(descriptor)); + } + } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgumentSpecification.java index 1586ea3d01cb1..637d25fac73be 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorArgumentSpecification.java @@ -16,13 +16,40 @@ public class DescriptorArgumentSpecification extends ArgumentSpecification { - public DescriptorArgumentSpecification(String name) + private DescriptorArgumentSpecification(String name, boolean required, Descriptor defaultValue) { - super(name, true, null); + super(name, required, defaultValue); } - public DescriptorArgumentSpecification(String name, Descriptor defaultValue) + public static Builder builder() { - super(name, false, defaultValue); + return new Builder(); + } + + public static final class Builder + { + private String name; + private boolean required = true; + private Descriptor defaultValue; + + private Builder() {} + + public Builder name(String name) + { + this.name = name; + return this; + } + + public Builder defaultValue(Descriptor defaultValue) + { + this.required = false; + this.defaultValue = defaultValue; + return this; + } + + public DescriptorArgumentSpecification build() + { + return new DescriptorArgumentSpecification(name, required, defaultValue); + } } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgument.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgument.java index ae15f9d2fea45..c9250d1fd9fe6 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgument.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgument.java @@ -54,4 +54,34 @@ public Object getValue() { return value; } + + public static Builder builder() + { + return new Builder(); + } + + public static final class Builder + { + private Type type; + private Object value; + + private Builder() {} + + public Builder type(Type type) + { + this.type = type; + return this; + } + + public Builder value(Object value) + { + this.value = value; + return this; + } + + public ScalarArgument build() + { + return new ScalarArgument(type, value); + } + } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java index a1f2b423c974a..b79f10c3a8f13 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java @@ -24,21 +24,56 @@ public class ScalarArgumentSpecification { private final Type type; - public ScalarArgumentSpecification(String name, Type type) + private ScalarArgumentSpecification(String name, Type type, boolean required, Object defaultValue) { - super(name, true, null); + super(name, required, defaultValue); this.type = requireNonNull(type, "type is null"); + if (defaultValue != null) { + checkArgument(type.getJavaType().equals(defaultValue.getClass()), format("default value %s does not match the declared type: %s", defaultValue, type)); + } } - public ScalarArgumentSpecification(String name, Type type, Object defaultValue) + public Type getType() { - super(name, false, defaultValue); - this.type = requireNonNull(type, "type is null"); - checkArgument(type.getJavaType().equals(defaultValue.getClass()), format("default value %s does not match the declared type: %s", defaultValue, type)); + return type; } - public Type getType() + public static Builder builder() { - return type; + return new Builder(); + } + + public static final class Builder + { + private String name; + private Type type; + private boolean required = true; + private Object defaultValue; + + private Builder() {} + + public Builder name(String name) + { + this.name = name; + return this; + } + + public Builder type(Type type) + { + this.type = type; + return this; + } + + public Builder defaultValue(Object defaultValue) + { + this.required = false; + this.defaultValue = defaultValue; + return this; + } + + public ScalarArgumentSpecification build() + { + return new ScalarArgumentSpecification(name, type, required, defaultValue); + } } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java index 36d61fa1996a3..ee56e13cb2a7b 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java @@ -17,6 +17,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.Collections; import java.util.List; import java.util.Optional; @@ -101,6 +102,71 @@ public boolean isPassThroughColumns() return passThroughColumns; } + public static Builder builder() + { + return new Builder(); + } + + public static final class Builder + { + private Optional name; + private RowType rowType; + private List partitionBy = Collections.emptyList(); + private List orderBy = Collections.emptyList(); + private boolean rowSemantics; + private boolean pruneWhenEmpty; + private boolean passThroughColumns; + + private Builder() {} + + public Builder name(Optional name) + { + this.name = name; + return this; + } + + public Builder rowType(RowType rowType) + { + this.rowType = rowType; + return this; + } + + public Builder partitionBy(List partitionBy) + { + this.partitionBy = partitionBy; + return this; + } + + public Builder orderBy(List orderBy) + { + this.orderBy = orderBy; + return this; + } + + public Builder rowSemantics(boolean rowSemantics) + { + this.rowSemantics = rowSemantics; + return this; + } + + public Builder pruneWhenEmpty(boolean pruneWhenEmpty) + { + this.pruneWhenEmpty = pruneWhenEmpty; + return this; + } + + public Builder passThroughColumns(boolean passThroughColumns) + { + this.passThroughColumns = passThroughColumns; + return this; + } + + public TableArgument build() + { + return new TableArgument(name, rowType, partitionBy, orderBy, rowSemantics, pruneWhenEmpty, passThroughColumns); + } + } + public static class QualifiedName { private final String catalogName; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java index 86e584d5bdc95..37de0c489d39b 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java @@ -20,7 +20,7 @@ public class TableArgumentSpecification private final boolean pruneWhenEmpty; private final boolean passThroughColumns; - public TableArgumentSpecification(String name, boolean rowSemantics, boolean pruneWhenEmpty, boolean passThroughColumns) + private TableArgumentSpecification(String name, boolean rowSemantics, boolean pruneWhenEmpty, boolean passThroughColumns) { super(name, true, null); @@ -29,12 +29,6 @@ public TableArgumentSpecification(String name, boolean rowSemantics, boolean pru this.passThroughColumns = passThroughColumns; } - public TableArgumentSpecification(String name) - { - // defaults - this(name, false, false, false); - } - public boolean isRowSemantics() { return rowSemantics; @@ -49,4 +43,48 @@ public boolean isPassThroughColumns() { return passThroughColumns; } + + public static Builder builder(String name) + { + return new Builder(); + } + + public static final class Builder + { + private String name; + private boolean rowSemantics; + private boolean pruneWhenEmpty; + private boolean passThroughColumns; + + private Builder() {} + + public Builder name(String name) + { + this.name = name; + return this; + } + + public Builder rowSemantics(boolean rowSemantics) + { + this.rowSemantics = rowSemantics; + return this; + } + + public Builder pruneWhenEmpty(boolean pruneWhenEmpty) + { + this.pruneWhenEmpty = pruneWhenEmpty; + return this; + } + + public Builder passThroughColumns(boolean passThroughColumns) + { + this.passThroughColumns = passThroughColumns; + return this; + } + + public TableArgumentSpecification build() + { + return new TableArgumentSpecification(name, rowSemantics, pruneWhenEmpty, passThroughColumns); + } + } } From 1f6e8415398227ce8b17c4057023751977ee8b4b Mon Sep 17 00:00:00 2001 From: mohsaka <135669458+mohsaka@users.noreply.github.com> Date: Fri, 7 Mar 2025 14:57:58 -0800 Subject: [PATCH 2/5] Refactor table function analysis Changes adapted from trino/PR#12407 Original commit: 712b8e98ff8a726f95295ac539159fc532628273 Original commit: 131dc44af97b31a2fa8115028d98d06671641bfa Author: kasiafi Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com> --- .../sql/analyzer/StatementAnalyzer.java | 5 +- .../table/ConnectorTableFunction.java | 53 +-------- .../function/table/TableFunctionAnalysis.java | 105 ++++++++++++++++++ 3 files changed, 109 insertions(+), 54 deletions(-) create mode 100644 presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java index b578a795644f7..3a7be8e90a05c 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java @@ -62,6 +62,7 @@ import com.facebook.presto.spi.function.table.ScalarArgument; import com.facebook.presto.spi.function.table.ScalarArgumentSpecification; import com.facebook.presto.spi.function.table.TableArgumentSpecification; +import com.facebook.presto.spi.function.table.TableFunctionAnalysis; import com.facebook.presto.spi.relation.DomainTranslator; import com.facebook.presto.spi.relation.RowExpression; import com.facebook.presto.spi.security.AccessControl; @@ -1296,7 +1297,7 @@ protected Scope visitTableFunctionInvocation(TableFunctionInvocation node, Optio ConnectorTransactionHandle transactionHandle = transactionManager.getConnectorTransaction( session.getRequiredTransactionId(), connectorId); - ConnectorTableFunction.Analysis functionAnalysis = function.analyze(session.toConnectorSession(connectorId), transactionHandle, passedArguments); + TableFunctionAnalysis functionAnalysis = function.analyze(session.toConnectorSession(connectorId), transactionHandle, passedArguments); analysis.setTableFunctionAnalysis(node, new Analysis.TableFunctionInvocationAnalysis(connectorId, functionName.toString(), passedArguments, functionAnalysis.getHandle(), transactionHandle)); // TODO handle the DescriptorMapping descriptorsToTables mapping from the TableFunction.Analysis: @@ -1309,7 +1310,7 @@ protected Scope visitTableFunctionInvocation(TableFunctionInvocation node, Optio // 4. at this point, the Identifier should be recorded as a column reference to the appropriate table // 5. record the mapping NameAndPosition -> Identifier // ... later translate Identifier to Symbol in Planner, and eventually translate it to channel before execution - if (!functionAnalysis.getDescriptorsToTables().isEmpty()) { + if (!functionAnalysis.getDescriptorMapping().isEmpty()) { throw new SemanticException(NOT_SUPPORTED, node, "Table arguments are not yet supported for table functions"); } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java index a8f0a609ce5cd..d50322f41a3c1 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java @@ -19,7 +19,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import static java.util.Collections.unmodifiableList; @@ -89,57 +88,7 @@ public ReturnTypeSpecification getReturnTypeSpecification() * * @param arguments actual invocation arguments, mapped by argument names */ - public Analysis analyze(ConnectorSession session, ConnectorTransactionHandle transaction, Map arguments) - { - throw new UnsupportedOperationException("analyze method not implemented for Table Function " + name); - } - - /** - * The `analyze()` method should produce an object of this class, containing all the analysis results: - *

- * The `returnedType` field is used to inform the Analyzer of the proper columns returned by the Table - * Function, that is, the columns produced by the function, as opposed to the columns passed from the - * input tables. The `returnedType` should only be set if the declared returned type is GENERIC_TABLE. - *

- * The `descriptorsToTables` field is used to inform the Analyzer of the semantics of descriptor arguments. - * Some descriptor arguments (or some of their fields) might be references to columns of the input tables. - * In such case, the Analyzer must be informed of those dependencies. It allows to pass the right values - * (input channels) to the Table Function during execution. It also allows to prune unused input columns - * during the optimization phase. - *

- * The `handle` field can be used to carry all information necessary to execute the table function, - * gathered at analysis time. Typically, these are the values of the constant arguments, and results - * of pre-processing arguments. - */ - public static class Analysis - { - private final Optional returnedType; - private final DescriptorMapping descriptorsToTables; - private final ConnectorTableFunctionHandle handle; - - public Analysis(Optional returnedType, DescriptorMapping descriptorsToTables, ConnectorTableFunctionHandle handle) - { - this.returnedType = requireNonNull(returnedType, "returnedType is null"); - returnedType.ifPresent(descriptor -> checkArgument(descriptor.isTyped(), "field types not specified")); - this.descriptorsToTables = requireNonNull(descriptorsToTables, "descriptorsToTables is null"); - this.handle = requireNonNull(handle, "handle is null"); - } - - public Optional getReturnedType() - { - return returnedType; - } - - public DescriptorMapping getDescriptorsToTables() - { - return descriptorsToTables; - } - - public ConnectorTableFunctionHandle getHandle() - { - return handle; - } - } + public abstract TableFunctionAnalysis analyze(ConnectorSession session, ConnectorTransactionHandle transaction, Map arguments); static String checkNotNullOrEmpty(String value, String name) { diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java new file mode 100644 index 0000000000000..bad1507d46220 --- /dev/null +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java @@ -0,0 +1,105 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.spi.function.table; + +import java.util.Optional; + +import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkArgument; +import static com.facebook.presto.spi.function.table.DescriptorMapping.EMPTY_MAPPING; +import static java.util.Objects.requireNonNull; + +/** + * An object of this class is produced by the `analyze()` method of a `ConnectorTableFunction` + * implementation. It contains all the analysis results: + *

+ * The `returnedType` field is used to inform the Analyzer of the proper columns returned by the Table + * Function, that is, the columns produced by the function, as opposed to the columns passed from the + * input tables. The `returnedType` should only be set if the declared returned type is GENERIC_TABLE. + *

+ * The `descriptorMapping` field is used to inform the Analyzer of the semantics of descriptor arguments. + * Some descriptor arguments (or some of their fields) might be references to columns of the input tables. + * In such case, the Analyzer must be informed of those dependencies. It allows to pass the right values + * (input channels) to the Table Function during execution. It also allows to prune unused input columns + * during the optimization phase. + *

+ * The `handle` field can be used to carry all information necessary to execute the table function, + * gathered at analysis time. Typically, these are the values of the constant arguments, and results + * of pre-processing arguments. + */ +public final class TableFunctionAnalysis +{ + private final Optional returnedType; + private final DescriptorMapping descriptorMapping; + private final ConnectorTableFunctionHandle handle; + + private TableFunctionAnalysis(Optional returnedType, DescriptorMapping descriptorMapping, ConnectorTableFunctionHandle handle) + { + this.returnedType = requireNonNull(returnedType, "returnedType is null"); + returnedType.ifPresent(descriptor -> checkArgument(descriptor.isTyped(), "field types not specified")); + this.descriptorMapping = requireNonNull(descriptorMapping, "descriptorMapping is null"); + this.handle = requireNonNull(handle, "handle is null"); + } + + public Optional getReturnedType() + { + return returnedType; + } + + public DescriptorMapping getDescriptorMapping() + { + return descriptorMapping; + } + + public ConnectorTableFunctionHandle getHandle() + { + return handle; + } + + public static Builder builder() + { + return new Builder(); + } + + public static final class Builder + { + private Descriptor returnedType; + private DescriptorMapping descriptorMapping = EMPTY_MAPPING; + private ConnectorTableFunctionHandle handle = new ConnectorTableFunctionHandle() {}; + + private Builder() {} + + public Builder returnedType(Descriptor returnedType) + { + this.returnedType = returnedType; + return this; + } + + public Builder descriptorMapping(DescriptorMapping descriptorMapping) + { + this.descriptorMapping = descriptorMapping; + return this; + } + + public Builder handle(ConnectorTableFunctionHandle handle) + { + this.handle = handle; + return this; + } + + public TableFunctionAnalysis build() + { + return new TableFunctionAnalysis(Optional.ofNullable(returnedType), descriptorMapping, handle); + } + } +} From ae3fde1aa93ef91da6611a5ec9017e3c1732da05 Mon Sep 17 00:00:00 2001 From: mohsaka <135669458+mohsaka@users.noreply.github.com> Date: Fri, 7 Mar 2025 15:07:42 -0800 Subject: [PATCH 3/5] Add argument-less builder methods for boolean values Changes adapted from trino/PR#12476 Original commit: 0da095e14b0855f89af3c4f254a5a60280fc7170 Author: kasiafi Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com> --- .../function/table/TableArgumentSpecification.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java index 37de0c489d39b..ce109aea93e0e 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java @@ -64,21 +64,21 @@ public Builder name(String name) return this; } - public Builder rowSemantics(boolean rowSemantics) + public Builder rowSemantics() { - this.rowSemantics = rowSemantics; + this.rowSemantics = true; return this; } - public Builder pruneWhenEmpty(boolean pruneWhenEmpty) + public Builder pruneWhenEmpty() { - this.pruneWhenEmpty = pruneWhenEmpty; + this.pruneWhenEmpty = true; return this; } - public Builder passThroughColumns(boolean passThroughColumns) + public Builder passThroughColumns() { - this.passThroughColumns = passThroughColumns; + this.passThroughColumns = true; return this; } From 3a6dada4986e6a2df074ffd4920dceac61914329 Mon Sep 17 00:00:00 2001 From: mohsaka <135669458+mohsaka@users.noreply.github.com> Date: Fri, 7 Mar 2025 15:40:49 -0800 Subject: [PATCH 4/5] Refactor ConnectorTableFunction into interface Changes adapted from trino/PR#12531 Original commit: 18bb60262cb0850cf839c2b20b434344921f5122 Original commit: 4a7d72afb64f93a9748a4c6b4defc2d42bbae000 Author: kasiafi Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com> --- .../metadata/TableFunctionRegistry.java | 33 +++++++++ .../table/AbstractConnectorTableFunction.java | 68 ++++++++++++++++++ .../function/table/ArgumentSpecification.java | 4 +- .../table/ConnectorTableFunction.java | 71 ++----------------- .../presto/spi/function/table/Descriptor.java | 4 +- .../spi/function/table/DescriptorMapping.java | 4 +- .../spi/function/table/NameAndPosition.java | 7 +- .../spi/function/table/Preconditions.java | 35 +++++++++ .../table/ReturnTypeSpecification.java | 2 +- .../table/ScalarArgumentSpecification.java | 2 +- .../spi/function/table/TableArgument.java | 2 +- .../function/table/TableFunctionAnalysis.java | 2 +- 12 files changed, 158 insertions(+), 76 deletions(-) create mode 100644 presto-spi/src/main/java/com/facebook/presto/spi/function/table/AbstractConnectorTableFunction.java create mode 100644 presto-spi/src/main/java/com/facebook/presto/spi/function/table/Preconditions.java diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/TableFunctionRegistry.java b/presto-main/src/main/java/com/facebook/presto/metadata/TableFunctionRegistry.java index fe29fbbc3d4f8..5b9730f31edb7 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/TableFunctionRegistry.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/TableFunctionRegistry.java @@ -19,7 +19,9 @@ import com.facebook.presto.spi.StandardErrorCode; import com.facebook.presto.spi.function.CatalogSchemaFunctionName; import com.facebook.presto.spi.function.SchemaFunctionName; +import com.facebook.presto.spi.function.table.ArgumentSpecification; import com.facebook.presto.spi.function.table.ConnectorTableFunction; +import com.facebook.presto.spi.function.table.TableArgumentSpecification; import com.facebook.presto.sql.analyzer.SemanticException; import com.facebook.presto.sql.tree.QualifiedName; import com.google.common.collect.ImmutableList; @@ -28,11 +30,14 @@ import javax.annotation.concurrent.ThreadSafe; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import static com.facebook.presto.spi.StandardErrorCode.MISSING_CATALOG_NAME; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.CATALOG_NOT_SPECIFIED; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.SCHEMA_NOT_SPECIFIED; import static com.google.common.base.Preconditions.checkState; @@ -50,6 +55,9 @@ public void addTableFunctions(ConnectorId catalogName, Collection builder = ImmutableMap.builder(); for (ConnectorTableFunction function : functions) { builder.put( @@ -115,4 +123,29 @@ public TableFunctionMetadata resolve(Session session, QualifiedName qualifiedNam return null; } + + private static void validateTableFunction(ConnectorTableFunction tableFunction) + { + requireNonNull(tableFunction, "tableFunction is null"); + requireNonNull(tableFunction.getName(), "table function name is null"); + requireNonNull(tableFunction.getSchema(), "table function schema name is null"); + requireNonNull(tableFunction.getArguments(), "table function arguments is null"); + requireNonNull(tableFunction.getReturnTypeSpecification(), "table function returnTypeSpecification is null"); + + checkArgument(!tableFunction.getName().isEmpty(), "table function name is empty"); + checkArgument(!tableFunction.getSchema().isEmpty(), "table function schema name is empty"); + + Set argumentNames = new HashSet<>(); + for (ArgumentSpecification specification : tableFunction.getArguments()) { + if (!argumentNames.add(specification.getName())) { + throw new IllegalArgumentException("duplicate argument name: " + specification.getName()); + } + } + long tableArgumentsWithRowSemantics = tableFunction.getArguments().stream() + .filter(specification -> specification instanceof TableArgumentSpecification) + .map(TableArgumentSpecification.class::cast) + .filter(TableArgumentSpecification::isRowSemantics) + .count(); + checkArgument(tableArgumentsWithRowSemantics <= 1, "more than one table argument with row semantics"); + } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/AbstractConnectorTableFunction.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/AbstractConnectorTableFunction.java new file mode 100644 index 0000000000000..f4190a6d93af5 --- /dev/null +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/AbstractConnectorTableFunction.java @@ -0,0 +1,68 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.spi.function.table; + +import com.facebook.presto.spi.ConnectorSession; +import com.facebook.presto.spi.connector.ConnectorTransactionHandle; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import static java.util.Objects.requireNonNull; + +public abstract class AbstractConnectorTableFunction + implements ConnectorTableFunction +{ + private final String schema; + private final String name; + private final List arguments; + private final ReturnTypeSpecification returnTypeSpecification; + + public AbstractConnectorTableFunction(String schema, String name, List arguments, ReturnTypeSpecification returnTypeSpecification) + { + this.schema = requireNonNull(schema, "schema is null"); + this.name = requireNonNull(name, "name is null"); + this.arguments = Collections.unmodifiableList(new ArrayList<>(requireNonNull(arguments, "arguments is null"))); + this.returnTypeSpecification = requireNonNull(returnTypeSpecification, "returnTypeSpecification is null"); + } + + @Override + public String getSchema() + { + return schema; + } + + @Override + public String getName() + { + return name; + } + + @Override + public List getArguments() + { + return arguments; + } + + @Override + public ReturnTypeSpecification getReturnTypeSpecification() + { + return returnTypeSpecification; + } + + @Override + public abstract TableFunctionAnalysis analyze(ConnectorSession session, ConnectorTransactionHandle transaction, Map arguments); +} diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java index ade58aa14fe35..73e92d07d0323 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ArgumentSpecification.java @@ -15,8 +15,8 @@ import javax.annotation.Nullable; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkArgument; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkNotNullOrEmpty; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; +import static com.facebook.presto.spi.function.table.Preconditions.checkNotNullOrEmpty; /** * Abstract class to capture the three supported argument types for a table function: diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java index d50322f41a3c1..d15546dc8fc7b 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ConnectorTableFunction.java @@ -16,61 +16,18 @@ import com.facebook.presto.spi.ConnectorSession; import com.facebook.presto.spi.connector.ConnectorTransactionHandle; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; -import static java.util.Collections.unmodifiableList; -import static java.util.Objects.requireNonNull; - -public abstract class ConnectorTableFunction +public interface ConnectorTableFunction { - private final String schema; - private final String name; - private final List arguments; - private final ReturnTypeSpecification returnTypeSpecification; - - public ConnectorTableFunction(String schema, String name, List arguments, ReturnTypeSpecification returnTypeSpecification) - { - this.schema = checkNotNullOrEmpty(schema, "schema"); - this.name = checkNotNullOrEmpty(name, "name"); - requireNonNull(arguments, "arguments is null"); - Set argumentNames = new HashSet<>(); - for (ArgumentSpecification specification : arguments) { - if (!argumentNames.add(specification.getName())) { - throw new IllegalArgumentException("duplicate argument name: " + specification.getName()); - } - } - long tableArgumentsWithRowSemantics = arguments.stream() - .filter(specification -> specification instanceof TableArgumentSpecification) - .map(TableArgumentSpecification.class::cast) - .filter(TableArgumentSpecification::isRowSemantics) - .count(); - checkArgument(tableArgumentsWithRowSemantics <= 1, "more than one table argument with row semantics"); - this.arguments = unmodifiableList(arguments); - this.returnTypeSpecification = requireNonNull(returnTypeSpecification, "returnTypeSpecification is null"); - } - - public String getSchema() - { - return schema; - } + String getSchema(); - public String getName() - { - return name; - } + String getName(); - public List getArguments() - { - return arguments; - } + List getArguments(); - public ReturnTypeSpecification getReturnTypeSpecification() - { - return returnTypeSpecification; - } + ReturnTypeSpecification getReturnTypeSpecification(); /** * This method is called by the Analyzer. Its main purposes are to: @@ -79,7 +36,7 @@ public ReturnTypeSpecification getReturnTypeSpecification() * 3. Perform function-specific validation and pre-processing of the input arguments. * As part of function-specific validation, the Table Function's author might want to: * - check if the descriptors which reference input tables contain a correct number of column references - * - check if the referenced input columns have appropriate types to fit the function's logic // TODO return request for coercions to the Analyzer in the Analysis object + * - check if the referenced input columns have appropriate types to fit the function's logic // TODO return request for coercions to the Analyzer in the TableFunctionAnalysis object * - if there is a descriptor which describes the function's output, check if it matches the shape of the actual function's output * - for table arguments, check the number and types of ordering columns *

@@ -88,19 +45,5 @@ public ReturnTypeSpecification getReturnTypeSpecification() * * @param arguments actual invocation arguments, mapped by argument names */ - public abstract TableFunctionAnalysis analyze(ConnectorSession session, ConnectorTransactionHandle transaction, Map arguments); - - static String checkNotNullOrEmpty(String value, String name) - { - requireNonNull(value, name + " is null"); - checkArgument(!value.isEmpty(), name + " is empty"); - return value; - } - - static void checkArgument(boolean assertion, String message) - { - if (!assertion) { - throw new IllegalArgumentException(message); - } - } + TableFunctionAnalysis analyze(ConnectorSession session, ConnectorTransactionHandle transaction, Map arguments); } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/Descriptor.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/Descriptor.java index 811afc5122fae..ce334ddb498ce 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/Descriptor.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/Descriptor.java @@ -23,8 +23,8 @@ import java.util.Optional; import java.util.stream.Collectors; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkArgument; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkNotNullOrEmpty; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; +import static com.facebook.presto.spi.function.table.Preconditions.checkNotNullOrEmpty; import static java.util.Collections.unmodifiableList; import static java.util.Objects.requireNonNull; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorMapping.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorMapping.java index bbc190b8500fd..bb232267ba301 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorMapping.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/DescriptorMapping.java @@ -18,8 +18,8 @@ import java.util.Map; import java.util.Set; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkArgument; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkNotNullOrEmpty; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; +import static com.facebook.presto.spi.function.table.Preconditions.checkNotNullOrEmpty; import static java.lang.String.format; import static java.util.Collections.unmodifiableMap; import static java.util.Objects.requireNonNull; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/NameAndPosition.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/NameAndPosition.java index bf5789a8df760..93216c3acc975 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/NameAndPosition.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/NameAndPosition.java @@ -15,6 +15,9 @@ import java.util.Objects; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; +import static com.facebook.presto.spi.function.table.Preconditions.checkNotNullOrEmpty; + /** * This class represents a descriptor field reference. * `name` is the descriptor argument name, `position` is the zero-based field index. @@ -31,8 +34,8 @@ public class NameAndPosition public NameAndPosition(String name, int position) { - this.name = ConnectorTableFunction.checkNotNullOrEmpty(name, "name"); - ConnectorTableFunction.checkArgument(position >= 0, "position in descriptor must not be negative"); + this.name = checkNotNullOrEmpty(name, "name"); + checkArgument(position >= 0, "position in descriptor must not be negative"); this.position = position; } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/Preconditions.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/Preconditions.java new file mode 100644 index 0000000000000..83edc78526ade --- /dev/null +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/Preconditions.java @@ -0,0 +1,35 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.spi.function.table; + +import static java.util.Objects.requireNonNull; + +public final class Preconditions +{ + private Preconditions() {} + + public static String checkNotNullOrEmpty(String value, String name) + { + requireNonNull(value, name + " is null"); + checkArgument(!value.isEmpty(), name + " is empty"); + return value; + } + + public static void checkArgument(boolean assertion, String message) + { + if (!assertion) { + throw new IllegalArgumentException(message); + } + } +} diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ReturnTypeSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ReturnTypeSpecification.java index f3c1c38d7de2b..45bf370a45714 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ReturnTypeSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ReturnTypeSpecification.java @@ -13,7 +13,7 @@ */ package com.facebook.presto.spi.function.table; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkArgument; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; /** diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java index b79f10c3a8f13..2dc78034734e3 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/ScalarArgumentSpecification.java @@ -15,7 +15,7 @@ import com.facebook.presto.common.type.Type; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkArgument; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; import static java.lang.String.format; import static java.util.Objects.requireNonNull; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java index ee56e13cb2a7b..958f1f5549b4a 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgument.java @@ -21,7 +21,7 @@ import java.util.List; import java.util.Optional; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkNotNullOrEmpty; +import static com.facebook.presto.spi.function.table.Preconditions.checkNotNullOrEmpty; import static java.util.Objects.requireNonNull; /** diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java index bad1507d46220..8baeda40329dd 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableFunctionAnalysis.java @@ -15,8 +15,8 @@ import java.util.Optional; -import static com.facebook.presto.spi.function.table.ConnectorTableFunction.checkArgument; import static com.facebook.presto.spi.function.table.DescriptorMapping.EMPTY_MAPPING; +import static com.facebook.presto.spi.function.table.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; /** From 8342f7068672c5ee1e2a6e4be59fce5e8230708f Mon Sep 17 00:00:00 2001 From: mohsaka <135669458+mohsaka@users.noreply.github.com> Date: Fri, 7 Mar 2025 15:44:07 -0800 Subject: [PATCH 5/5] Remove unused argument from TableArgumentSpecification.java builder. Changes adapted from trino/PR#12813 Original commit: 5310671f80291394b12ba2ea746e4e60051aaff4 Author: kasiafi Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com> --- .../presto/spi/function/table/TableArgumentSpecification.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java index ce109aea93e0e..f845022d6b349 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/function/table/TableArgumentSpecification.java @@ -44,7 +44,7 @@ public boolean isPassThroughColumns() return passThroughColumns; } - public static Builder builder(String name) + public static Builder builder() { return new Builder(); }