Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add builders for table function arguments #12350

Merged
merged 1 commit into from
May 13, 2022
+248 −30
Merged

Add builders for table function arguments #12350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,10 @@ else if (argument.getValue() instanceof Expression) {
Type expectedArgumentType = ((ScalarArgumentSpecification) argumentSpecification).getType();
// currently, only constant arguments are supported
Object constantValue = ExpressionInterpreter.evaluateConstantExpression(expression, expectedArgumentType, plannerContext, session, accessControl, 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());
Expand All @@ -1687,7 +1690,10 @@ private Argument analyzeDefault(ArgumentSpecification argumentSpecification, Nod
throw 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());
Expand Down
2 changes: 1 addition & 1 deletion core/trino-spi/src/main/java/io/trino/spi/ptf/ArgumentSpecification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
33 changes: 25 additions & 8 deletions core/trino-spi/src/main/java/io/trino/spi/ptf/DescriptorArgument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,11 @@
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> 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> descriptor)
public DescriptorArgument(@JsonProperty("descriptor") Optional<Descriptor> descriptor)
{
this.descriptor = requireNonNull(descriptor, "descriptor is null");
}
Expand All @@ -50,4 +44,27 @@ public Optional<Descriptor> 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));
}
}
}
35 changes: 31 additions & 4 deletions core/trino-spi/src/main/java/io/trino/spi/ptf/DescriptorArgumentSpecification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
30 changes: 30 additions & 0 deletions core/trino-spi/src/main/java/io/trino/spi/ptf/ScalarArgument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like a typed value. I wouldn't foresee additional parameters here, and so the builder looks like an overkill

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. At some point, we will allow arbitrary expressions as scalar arguments.

}
}
}
51 changes: 43 additions & 8 deletions core/trino-spi/src/main/java/io/trino/spi/ptf/ScalarArgumentSpecification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
65 changes: 65 additions & 0 deletions core/trino-spi/src/main/java/io/trino/spi/ptf/TableArgument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,71 @@ public boolean isPassThroughColumns()
return passThroughColumns;
}

public static Builder builder()
{
return new Builder();
}

public static final class Builder
{
private Optional<QualifiedName> name;
private RowType rowType;
private List<String> partitionBy = List.of();
private List<SortItem> orderBy = List.of();
private boolean rowSemantics;
private boolean pruneWhenEmpty;
private boolean passThroughColumns;

private Builder() {}

public Builder name(Optional<QualifiedName> name)
{
this.name = name;
return this;
}

public Builder rowType(RowType rowType)
{
this.rowType = rowType;
return this;
}

public Builder partitionBy(List<String> partitionBy)
losipiuk marked this conversation as resolved.
Show resolved Hide resolved
{
this.partitionBy = partitionBy;
return this;
}

public Builder orderBy(List<SortItem> orderBy)
{
this.orderBy = orderBy;
return this;
}

public Builder rowSemantics(boolean rowSemantics)
{
this.rowSemantics = rowSemantics;
return this;
}

public Builder pruneWhenEmpty(boolean pruneWhenEmpty)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I prefer boolean methods to have a good default and then a no-arg method to change tha behavior, so I would make this:

public Builder pruneWhenEmpty()
{
    this.pruneWhenEmpty = true;
    return this;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was the initial approach. I changed it according to #12350 (comment)

{
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;
Expand Down
52 changes: 45 additions & 7 deletions core/trino-spi/src/main/java/io/trino/spi/ptf/TableArgumentSpecification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
Expand All @@ -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);
}
}
}