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

Jump to bottom

[native] Disable IPAddress type on presto native #23436

Merged
merged 1 commit into from
Aug 23, 2024
Merged

[native] Disable IPAddress type on presto native #23436

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
14 changes: 14 additions & 0 deletions presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ public class FeaturesConfig
private boolean isOptimizeMultipleApproxPercentileOnSameFieldEnabled = true;
private boolean nativeExecutionEnabled;
private boolean disableTimeStampWithTimeZoneForNative = true;
private boolean disableIPAddressForNative = true;
private String nativeExecutionExecutablePath = "./presto_server";
private String nativeExecutionProgramArguments = "";
private boolean nativeExecutionProcessReuseEnabled = true;
Expand Down Expand Up @@ -2655,6 +2656,19 @@ public boolean isDisableTimeStampWithTimeZoneForNative()
return this.disableTimeStampWithTimeZoneForNative;
}

@Config("disable-ipaddress-for-native-execution")
@ConfigDescription("Disable ipaddress type on native engine")
public FeaturesConfig setDisableIPAddressForNative(boolean disableIPAddressForNative)
{
this.disableIPAddressForNative = disableIPAddressForNative;
return this;
}

public boolean isDisableIPAddressForNative()
{
return this.disableIPAddressForNative;
}

@Config("native-execution-executable-path")
@ConfigDescription("Native execution executable file path")
public FeaturesConfig setNativeExecutionExecutablePath(String nativeExecutionExecutablePath)
Expand Down
70 changes: 44 additions & 26 deletions ...ity/CheckNoTimestampWithTimezoneType.java → ...ity/CheckUnsupportedPrestissimoTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,41 +33,54 @@
import com.facebook.presto.spi.relation.RowExpression;
import com.facebook.presto.spi.relation.SpecialFormExpression;
import com.facebook.presto.spi.relation.VariableReferenceExpression;
import com.facebook.presto.sql.analyzer.FeaturesConfig;
import com.facebook.presto.sql.parser.SqlParser;
import com.facebook.presto.sql.planner.SimplePlanVisitor;
import com.facebook.presto.sql.planner.TypeProvider;
import com.facebook.presto.sql.planner.plan.WindowNode;

import java.util.List;
import java.util.Objects;
import java.util.Optional;

import static com.facebook.presto.common.type.TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE;
import static com.facebook.presto.type.IpAddressType.IPADDRESS;
import static com.google.common.base.Preconditions.checkState;
import static java.util.Objects.requireNonNull;

public class CheckNoTimestampWithTimezoneType
public class CheckUnsupportedPrestissimoTypes
implements PlanChecker.Checker
{
private static final String errorMessage = "Timestamp with Timezone type is not supported in Prestissimo";
private static final String timestampWithTimeszoneErrorMessage = "Timestamp with Timezone type is not supported in Prestissimo";
private static final String ipAddressErrorMessage = "IPAddress type is not supported in Prestissimo";
private FeaturesConfig config;

public CheckUnsupportedPrestissimoTypes(FeaturesConfig config)
{
this.config = requireNonNull(config);
}

@Override
public void validate(PlanNode planNode, Session session, Metadata metadata, SqlParser sqlParser, TypeProvider types, WarningCollector warningCollector)
{
planNode.accept(new Visitor(), null);
}

private static class Visitor
private class Visitor
extends SimplePlanVisitor<Void>
{
private final NoTimeStampWithTimeZoneTypeChecker noTimeStampWithTimeZoneTypeChecker;
private final UnsupportedTypeChecker unsupportedTypeChecker;

public Visitor()
{
this.noTimeStampWithTimeZoneTypeChecker = new NoTimeStampWithTimeZoneTypeChecker();
this.unsupportedTypeChecker = new UnsupportedTypeChecker();
}

@Override
public Void visitPlan(PlanNode node, Void context)
{
checkState(node.getOutputVariables().stream().noneMatch(x -> hasTimestampWithTimezoneType(x.getType())), errorMessage);
Optional<String> res = node.getOutputVariables().stream().map(x -> getUnsupportedTypeErrorMessage(x.getType())).filter(Objects::nonNull).findFirst().orElse(Optional.empty());
res.ifPresent(str -> checkState(false, str));
return super.visitPlan(node, context);
}

Expand All @@ -76,9 +89,9 @@ public Void visitAggregation(AggregationNode node, Void context)
{
visitPlan(node, context);
node.getAggregations().forEach((variable, aggregation) -> {
aggregation.getCall().accept(noTimeStampWithTimeZoneTypeChecker, null);
aggregation.getCall().accept(unsupportedTypeChecker, null);
if (aggregation.getFilter().isPresent()) {
aggregation.getFilter().get().accept(noTimeStampWithTimeZoneTypeChecker, null);
aggregation.getFilter().get().accept(unsupportedTypeChecker, null);
}
});

Expand All @@ -90,7 +103,7 @@ public Void visitWindow(WindowNode node, Void context)
{
visitPlan(node, context);
node.getWindowFunctions().forEach((variable, function) -> {
function.getFunctionCall().accept(noTimeStampWithTimeZoneTypeChecker, null);
function.getFunctionCall().accept(unsupportedTypeChecker, null);
});

return null;
Expand All @@ -101,7 +114,7 @@ public Void visitProject(ProjectNode node, Void context)
{
visitPlan(node, context);
node.getAssignments().getMap().forEach((variable, expression) -> {
expression.accept(noTimeStampWithTimeZoneTypeChecker, null);
expression.accept(unsupportedTypeChecker, null);
});

return null;
Expand All @@ -112,7 +125,7 @@ public Void visitValues(ValuesNode node, Void context)
{
visitPlan(node, context);
for (List<RowExpression> row : node.getRows()) {
row.forEach(x -> x.accept(noTimeStampWithTimeZoneTypeChecker, null));
row.forEach(x -> x.accept(unsupportedTypeChecker, null));
}
return null;
}
Expand All @@ -121,71 +134,76 @@ public Void visitValues(ValuesNode node, Void context)
public Void visitFilter(FilterNode node, Void context)
{
visitPlan(node, context);
node.getPredicate().accept(noTimeStampWithTimeZoneTypeChecker, null);
node.getPredicate().accept(unsupportedTypeChecker, null);
return null;
}
}

private static class NoTimeStampWithTimeZoneTypeChecker
private class UnsupportedTypeChecker
extends DefaultRowExpressionTraversalVisitor<Void>
{
@Override
public Void visitConstant(ConstantExpression literal, Void context)
{
checkState(!hasTimestampWithTimezoneType(literal.getType()), errorMessage);
Optional<String> errorMessage = getUnsupportedTypeErrorMessage(literal.getType());
checkState(!errorMessage.isPresent(), errorMessage);
return null;
}

@Override
public Void visitVariableReference(VariableReferenceExpression reference, Void context)
{
checkState(!hasTimestampWithTimezoneType(reference.getType()), errorMessage);
getUnsupportedTypeErrorMessage(reference.getType()).ifPresent(str -> checkState(false, str));
return null;
}

@Override
public Void visitInputReference(InputReferenceExpression input, Void context)
{
checkState(!hasTimestampWithTimezoneType(input.getType()), errorMessage);
getUnsupportedTypeErrorMessage(input.getType()).ifPresent(str -> checkState(false, str));
return null;
}

@Override
public Void visitCall(CallExpression call, Void context)
{
checkState(!hasTimestampWithTimezoneType(call.getType()), errorMessage);
getUnsupportedTypeErrorMessage(call.getType()).ifPresent(str -> checkState(false, str));
return super.visitCall(call, context);
}

@Override
public Void visitSpecialForm(SpecialFormExpression specialForm, Void context)
{
checkState(!hasTimestampWithTimezoneType(specialForm.getType()), errorMessage);
getUnsupportedTypeErrorMessage(specialForm.getType()).ifPresent(str -> checkState(false, str));
return super.visitSpecialForm(specialForm, context);
}

@Override
public Void visitIntermediateFormExpression(IntermediateFormExpression expression, Void context)
{
checkState(!hasTimestampWithTimezoneType(expression.getType()), errorMessage);
getUnsupportedTypeErrorMessage(expression.getType()).ifPresent(str -> checkState(false, str));
return super.visitIntermediateFormExpression(expression, context);
}
}

private static boolean hasTimestampWithTimezoneType(Type type)
private Optional<String> getUnsupportedTypeErrorMessage(Type type)
{
if (type.equals(TIMESTAMP_WITH_TIME_ZONE)) {
return true;
if (type.equals(TIMESTAMP_WITH_TIME_ZONE) && config.isDisableTimeStampWithTimeZoneForNative()) {
return Optional.of(timestampWithTimeszoneErrorMessage);
}
if (type.equals(IPADDRESS) && config.isDisableIPAddressForNative()) {
return Optional.of(ipAddressErrorMessage);
}
if (type instanceof ArrayType) {
return hasTimestampWithTimezoneType(((ArrayType) type).getElementType());
return getUnsupportedTypeErrorMessage(((ArrayType) type).getElementType());
}
else if (type instanceof MapType) {
return hasTimestampWithTimezoneType(((MapType) type).getKeyType()) || hasTimestampWithTimezoneType(((MapType) type).getValueType());
Optional<String> key = getUnsupportedTypeErrorMessage(((MapType) type).getKeyType());
return key.isPresent() ? key : getUnsupportedTypeErrorMessage(((MapType) type).getValueType());
}
else if (type instanceof RowType) {
return ((RowType) type).getTypeParameters().stream().anyMatch(CheckNoTimestampWithTimezoneType::hasTimestampWithTimezoneType);
return ((RowType) type).getTypeParameters().stream().map(this::getUnsupportedTypeErrorMessage).filter(opt -> opt.isPresent()).findFirst().orElse(Optional.empty());
}
return false;
return Optional.empty();
}
}
5 changes: 3 additions & 2 deletions presto-main/src/main/java/com/facebook/presto/sql/planner/sanity/PlanChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ public PlanChecker(FeaturesConfig featuresConfig, boolean forceSingleNode)
new VerifyProjectionLocality(),
new DynamicFiltersChecker(),
new WarnOnScanWithoutPartitionPredicate(featuresConfig));
if (featuresConfig.isNativeExecutionEnabled() && featuresConfig.isDisableTimeStampWithTimeZoneForNative()) {
builder.put(Stage.INTERMEDIATE, new CheckNoTimestampWithTimezoneType());
if (featuresConfig.isNativeExecutionEnabled() && (featuresConfig.isDisableTimeStampWithTimeZoneForNative() ||
featuresConfig.isDisableIPAddressForNative())) {
builder.put(Stage.INTERMEDIATE, new CheckUnsupportedPrestissimoTypes(featuresConfig));
}
checkers = builder.build();
}
Expand Down
3 changes: 3 additions & 0 deletions presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ public void testDefaults()
.setOptimizeMultipleApproxPercentileOnSameFieldEnabled(true)
.setNativeExecutionEnabled(false)
.setDisableTimeStampWithTimeZoneForNative(true)
.setDisableIPAddressForNative(true)
.setNativeExecutionExecutablePath("./presto_server")
.setNativeExecutionProgramArguments("")
.setNativeExecutionProcessReuseEnabled(true)
Expand Down Expand Up @@ -454,6 +455,7 @@ public void testExplicitPropertyMappings()
.put("optimizer.optimize-multiple-approx-percentile-on-same-field", "false")
.put("native-execution-enabled", "true")
.put("disable-timestamp-with-timezone-for-native-execution", "false")
.put("disable-ipaddress-for-native-execution", "false")
.put("native-execution-executable-path", "/bin/echo")
.put("native-execution-program-arguments", "--v 1")
.put("native-execution-process-reuse-enabled", "false")
Expand Down Expand Up @@ -675,6 +677,7 @@ public void testExplicitPropertyMappings()
.setOptimizeMultipleApproxPercentileOnSameFieldEnabled(false)
.setNativeExecutionEnabled(true)
.setDisableTimeStampWithTimeZoneForNative(false)
.setDisableIPAddressForNative(false)
.setNativeExecutionExecutablePath("/bin/echo")
.setNativeExecutionProgramArguments("--v 1")
.setNativeExecutionProcessReuseEnabled(false)
Expand Down
53 changes: 48 additions & 5 deletions ...TestCheckNoTimestampWithTimezoneType.java → ...TestCheckUnsupportedPrestissimoTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.facebook.presto.spi.plan.PlanNode;
import com.facebook.presto.spi.plan.PlanNodeIdAllocator;
import com.facebook.presto.spi.relation.VariableReferenceExpression;
import com.facebook.presto.sql.analyzer.FeaturesConfig;
import com.facebook.presto.sql.parser.SqlParser;
import com.facebook.presto.sql.planner.TypeProvider;
import com.facebook.presto.sql.planner.assertions.BasePlanTest;
Expand All @@ -35,14 +36,16 @@
import static com.facebook.presto.common.type.VarcharType.VARCHAR;
import static com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder.assignment;
import static com.facebook.presto.testing.TestingSession.testSessionBuilder;
import static com.facebook.presto.type.IpAddressType.IPADDRESS;

public class TestCheckNoTimestampWithTimezoneType
public class TestCheckUnsupportedPrestissimoTypes
extends BasePlanTest
{
private Session testSession;
private Metadata metadata;
private SqlParser sqlParser;
private PlanNodeIdAllocator idAllocator = new PlanNodeIdAllocator();
private FeaturesConfig featuresConfig = new FeaturesConfig();

@BeforeClass
public void setup()
Expand All @@ -65,7 +68,7 @@ public void tearDown()
}

@Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Timestamp with Timezone type is not supported in Prestissimo")
public void testValidateProjectFail()
public void testValidateTimestampTZProjectFail()
{
validatePlan(
p -> {
Expand All @@ -78,7 +81,7 @@ public void testValidateProjectFail()
}

@Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Timestamp with Timezone type is not supported in Prestissimo")
public void testValidateProjectAssignmentFail()
public void testValidateTimestampTZProjectAssignmentFail()
{
validatePlan(
p -> {
Expand All @@ -92,7 +95,7 @@ public void testValidateProjectAssignmentFail()
}

@Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Timestamp with Timezone type is not supported in Prestissimo")
public void testValidateValueFail()
public void testValidateTimestampTZValueFail()
{
validatePlan(
p -> {
Expand All @@ -104,14 +107,54 @@ public void testValidateValueFail()
});
}

@Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "IPAddress type is not supported in Prestissimo")
public void testValidateIPAddressProjectFail()
{
validatePlan(
p -> {
VariableReferenceExpression col = p.variable("col", VARCHAR);
VariableReferenceExpression col2 = p.variable("col2", IPADDRESS);
return p.project(
assignment(col2, p.rowExpression("cast(col as ipaddress)")),
p.values(col));
});
}

@Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "IPAddress type is not supported in Prestissimo")
public void testValidateIPAddressProjectAssignmentFail()
{
validatePlan(
p -> {
VariableReferenceExpression col = p.variable("col", VARCHAR);
VariableReferenceExpression col1 = p.variable("col1", VARCHAR);
VariableReferenceExpression col2 = p.variable("col2", BOOLEAN);
return p.project(
assignment(col2, p.rowExpression("cast(col as ipaddress) > cast(col1 as ipaddress)")),
p.values(col, col1));
});
}

@Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "IPAddress type is not supported in Prestissimo")
public void testValidateIPAddressValueFail()
{
validatePlan(
p -> {
VariableReferenceExpression col = p.variable("col", IPADDRESS);
VariableReferenceExpression col2 = p.variable("col2", VARCHAR);
return p.project(
assignment(col2, p.rowExpression("cast(col as varchar)")),
p.values(col));
});
}

private void validatePlan(Function<PlanBuilder, PlanNode> planProvider)
{
PlanBuilder builder = new PlanBuilder(TEST_SESSION, idAllocator, metadata);
PlanNode planNode = planProvider.apply(builder);
TypeProvider types = builder.getTypes();
getQueryRunner().inTransaction(testSession, session -> {
session.getCatalog().ifPresent(catalog -> metadata.getCatalogHandle(session, catalog));
new CheckNoTimestampWithTimezoneType().validate(planNode, session, metadata, sqlParser, types, WarningCollector.NOOP);
new CheckUnsupportedPrestissimoTypes(featuresConfig).validate(planNode, session, metadata, sqlParser, types, WarningCollector.NOOP);
return null;
});
}
Expand Down
1 change: 1 addition & 0 deletions ...cution/src/test/java/com/facebook/presto/nativeworker/AbstractTestNativeAggregations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ public void testChecksum()
assertQuery("SELECT checksum(quantity_by_linenumber) FROM orders_ex");
assertQuery("SELECT shipmode, checksum(extendedprice) FROM lineitem GROUP BY shipmode");
assertQueryFails("SELECT checksum(from_unixtime(orderkey, '+01:00')) FROM lineitem WHERE orderkey < 20", ".*Timestamp with Timezone type is not supported in Prestissimo.*");
assertQueryFails("SELECT checksum(cast(v as ipaddress)) FROM (VALUES '192.168.1.1', NULL ) as t (v)", ".*IPAddress type is not supported in Prestissimo.*");

// test DECIMAL data
assertQuery("SELECT checksum(a), checksum(b) FROM (VALUES (DECIMAL '1.234', DECIMAL '611180549424.4633133')) AS t(a, b)");
Expand Down
Loading
Loading