diff --git a/presto-common/src/main/java/com/facebook/presto/common/predicate/Range.java b/presto-common/src/main/java/com/facebook/presto/common/predicate/Range.java index 955f26e1ba31b..606f5f82686e7 100644 --- a/presto-common/src/main/java/com/facebook/presto/common/predicate/Range.java +++ b/presto-common/src/main/java/com/facebook/presto/common/predicate/Range.java @@ -240,17 +240,36 @@ public boolean equals(Object obj) Objects.equals(this.high, other.high); } + private void appendQuotedValue(StringBuilder buffer, Marker marker, SqlFunctionProperties properties) + { + buffer.append('"'); + buffer.append(marker.getPrintableValue(properties).toString().replace("\"", "\\\"")); + buffer.append('"'); + } + public String toString(SqlFunctionProperties properties) { StringBuilder buffer = new StringBuilder(); if (isSingleValue()) { - buffer.append('[').append(low.getPrintableValue(properties)).append(']'); + buffer.append('['); + appendQuotedValue(buffer, low, properties); + buffer.append(']'); } else { buffer.append((low.getBound() == Marker.Bound.EXACTLY) ? '[' : '('); - buffer.append(low.isLowerUnbounded() ? "" : low.getPrintableValue(properties)); + if (low.isLowerUnbounded()) { + buffer.append(""); + } + else { + appendQuotedValue(buffer, low, properties); + } buffer.append(", "); - buffer.append(high.isUpperUnbounded() ? "" : high.getPrintableValue(properties)); + if (high.isUpperUnbounded()) { + buffer.append(""); + } + else { + appendQuotedValue(buffer, high, properties); + } buffer.append((high.getBound() == Marker.Bound.EXACTLY) ? ']' : ')'); } return buffer.toString(); diff --git a/presto-common/src/test/java/com/facebook/presto/common/predicate/TestRange.java b/presto-common/src/test/java/com/facebook/presto/common/predicate/TestRange.java index 77f502a5a8921..ef6459b781d87 100644 --- a/presto-common/src/test/java/com/facebook/presto/common/predicate/TestRange.java +++ b/presto-common/src/test/java/com/facebook/presto/common/predicate/TestRange.java @@ -17,6 +17,7 @@ import com.facebook.presto.common.block.Block; import com.facebook.presto.common.block.TestingBlockEncodingSerde; import com.facebook.presto.common.block.TestingBlockJsonSerde; +import com.facebook.presto.common.function.SqlFunctionProperties; import com.facebook.presto.common.type.TestingTypeDeserializer; import com.facebook.presto.common.type.TestingTypeManager; import com.facebook.presto.common.type.Type; @@ -27,8 +28,10 @@ import static com.facebook.presto.common.type.BigintType.BIGINT; import static com.facebook.presto.common.type.BooleanType.BOOLEAN; import static com.facebook.presto.common.type.DoubleType.DOUBLE; +import static com.facebook.presto.common.type.TimeZoneKey.UTC_KEY; import static com.facebook.presto.common.type.VarcharType.VARCHAR; import static io.airlift.slice.Slices.utf8Slice; +import static java.util.Locale.ENGLISH; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertThrows; @@ -36,6 +39,14 @@ public class TestRange { + public static final SqlFunctionProperties PROPERTIES = SqlFunctionProperties.builder() + .setTimeZoneKey(UTC_KEY) + .setLegacyTimestamp(true) + .setSessionStartTime(0) + .setSessionLocale(ENGLISH) + .setSessionUser("user") + .build(); + @SuppressWarnings({"unchecked", "rawtypes"}) @Test(expectedExceptions = IllegalArgumentException.class) public void testMismatchedTypes() @@ -281,4 +292,41 @@ public void testJsonSerialization() range = Range.lessThanOrEqual(DOUBLE, Double.MAX_VALUE); assertEquals(range, mapper.readValue(mapper.writeValueAsString(range), Range.class)); } + + @Test + public void testToString() + { + Range range = Range.all(VARCHAR); + assertEquals(range.toString(PROPERTIES), "(, )"); + + range = Range.equal(VARCHAR, utf8Slice("some string")); + assertEquals(range.toString(PROPERTIES), "[\"some string\"]"); + + range = Range.equal(VARCHAR, utf8Slice("here's a quote: \"")); + assertEquals(range.toString(PROPERTIES), "[\"here's a quote: \\\"\"]"); + + range = Range.equal(VARCHAR, utf8Slice("")); + assertEquals(range.toString(PROPERTIES), "[\"\"]"); + + range = Range.greaterThanOrEqual(VARCHAR, utf8Slice("string with \"quotes\" inside")).intersect(Range.lessThanOrEqual(VARCHAR, utf8Slice("this string's quote is here -> \""))); + assertEquals(range.toString(PROPERTIES), "[\"string with \\\"quotes\\\" inside\", \"this string's quote is here -> \\\"\"]"); + + range = Range.greaterThan(VARCHAR, utf8Slice("string with \"quotes\" inside")).intersect(Range.lessThan(VARCHAR, utf8Slice("this string's quote is here -> \""))); + assertEquals(range.toString(PROPERTIES), "(\"string with \\\"quotes\\\" inside\", \"this string's quote is here -> \\\"\")"); + + range = Range.equal(VARCHAR, utf8Slice("")); + assertEquals(range.toString(PROPERTIES), "[\"\"]"); + + range = Range.equal(VARCHAR, utf8Slice("")); + assertEquals(range.toString(PROPERTIES), "[\"\"]"); + + range = Range.equal(VARCHAR, utf8Slice(", ")); + assertEquals(range.toString(PROPERTIES), "[\", \"]"); + + range = Range.greaterThanOrEqual(VARCHAR, utf8Slice("a")).intersect(Range.lessThanOrEqual(VARCHAR, utf8Slice("b"))); + assertEquals(range.toString(PROPERTIES), "[\"a\", \"b\"]"); + + range = Range.equal(VARCHAR, utf8Slice("a, b")); + assertEquals(range.toString(PROPERTIES), "[\"a, b\"]"); + } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java index 693ff98ee2314..e1df9bcd54ba9 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java @@ -1207,33 +1207,7 @@ private String formatDomain(Domain domain) domain.getValues().getValuesProcessor().consume( ranges -> { for (Range range : ranges.getOrderedRanges()) { - StringBuilder builder = new StringBuilder(); - if (range.isSingleValue()) { - String value = castToVarchar(type, range.getSingleValue(), functionAndTypeManager, session); - builder.append('[').append(value).append(']'); - } - else { - builder.append(range.isLowInclusive() ? '[' : '('); - - if (range.isLowUnbounded()) { - builder.append(""); - } - else { - builder.append(castToVarchar(type, range.getLowBoundedValue(), functionAndTypeManager, session)); - } - - builder.append(", "); - - if (range.isHighUnbounded()) { - builder.append(""); - } - else { - builder.append(castToVarchar(type, range.getHighBoundedValue(), functionAndTypeManager, session)); - } - - builder.append(range.isHighInclusive() ? ']' : ')'); - } - parts.add(builder.toString()); + parts.add(range.toString(session.getSqlFunctionProperties())); } }, discreteValues -> discreteValues.getValues().stream() @@ -1300,7 +1274,7 @@ private static String castToVarchar(Type type, Object value, FunctionAndTypeMana try { FunctionHandle cast = functionAndTypeManager.lookupCast(CAST, type.getTypeSignature(), VARCHAR.getTypeSignature()); Slice coerced = (Slice) new InterpretedFunctionInvoker(functionAndTypeManager).invoke(cast, session.getSqlFunctionProperties(), value); - return coerced.toStringUtf8(); + return "\"" + coerced.toStringUtf8().replace("\"", "\\\"") + "\""; } catch (OperatorNotFoundException e) { return ""; diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/planPrinter/TestPlanPrinter.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/planPrinter/TestPlanPrinter.java new file mode 100644 index 0000000000000..25ebf6af46d80 --- /dev/null +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/planPrinter/TestPlanPrinter.java @@ -0,0 +1,189 @@ +/* + * 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.sql.planner.planPrinter; + +import com.facebook.presto.common.predicate.Domain; +import com.facebook.presto.common.predicate.Range; +import com.facebook.presto.common.predicate.TupleDomain; +import com.facebook.presto.common.predicate.ValueSet; +import com.facebook.presto.cost.StatsAndCosts; +import com.facebook.presto.metadata.FunctionAndTypeManager; +import com.facebook.presto.operator.StageExecutionDescriptor; +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.ConnectorId; +import com.facebook.presto.spi.TableHandle; +import com.facebook.presto.spi.plan.PlanNodeIdAllocator; +import com.facebook.presto.spi.plan.TableScanNode; +import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.planner.Partitioning; +import com.facebook.presto.sql.planner.PartitioningScheme; +import com.facebook.presto.sql.planner.PlanFragment; +import com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder; +import com.facebook.presto.sql.planner.plan.PlanFragmentId; +import com.facebook.presto.testing.TestingHandle; +import com.facebook.presto.testing.TestingMetadata; +import com.facebook.presto.testing.TestingTransactionHandle; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.testng.annotations.Test; + +import java.util.Optional; + +import static com.facebook.presto.SessionTestUtils.TEST_SESSION; +import static com.facebook.presto.common.predicate.TupleDomain.withColumnDomains; +import static com.facebook.presto.common.type.VarcharType.VARCHAR; +import static com.facebook.presto.metadata.AbstractMockMetadata.dummyMetadata; +import static com.facebook.presto.metadata.FunctionAndTypeManager.createTestFunctionAndTypeManager; +import static com.facebook.presto.sql.planner.SystemPartitioningHandle.SOURCE_DISTRIBUTION; +import static io.airlift.slice.Slices.utf8Slice; +import static org.testng.Assert.assertTrue; + +public class TestPlanPrinter +{ + private static final PlanBuilder PLAN_BUILDER = new PlanBuilder(TEST_SESSION, new PlanNodeIdAllocator(), dummyMetadata()); + private static final FunctionAndTypeManager FUNCTION_AND_TYPE_MANAGER = createTestFunctionAndTypeManager(); + private static final VariableReferenceExpression COLUMN_VARIABLE = new VariableReferenceExpression("column", VARCHAR); + private static final ColumnHandle COLUMN_HANDLE = new TestingMetadata.TestingColumnHandle("column"); + private static final TableHandle TABLE_HANDLE_WITH_LAYOUT = new TableHandle( + new ConnectorId("testConnector"), + new TestingMetadata.TestingTableHandle(), + TestingTransactionHandle.create(), + Optional.of(TestingHandle.INSTANCE)); + + // Creates a trivial TableScan with the given domain on some column + private String domainToPrintedScan(VariableReferenceExpression variable, ColumnHandle colHandle, Domain domain) + { + TupleDomain tupleDomain = withColumnDomains(ImmutableMap.builder().put(colHandle, domain).build()); + + TableScanNode scanNode = PLAN_BUILDER.tableScan( + TABLE_HANDLE_WITH_LAYOUT, + ImmutableList.of(variable), + ImmutableMap.of(variable, colHandle), + tupleDomain, + tupleDomain); + + PlanFragment testFragment = new PlanFragment( + new PlanFragmentId(0), + scanNode, + ImmutableSet.of(variable), + SOURCE_DISTRIBUTION, + ImmutableList.of(scanNode.getId()), + new PartitioningScheme(Partitioning.create(SOURCE_DISTRIBUTION, ImmutableList.of()), ImmutableList.of(variable)), + StageExecutionDescriptor.ungroupedExecution(), + false, + StatsAndCosts.empty(), + Optional.empty()); + + return PlanPrinter.textPlanFragment(testFragment, FUNCTION_AND_TYPE_MANAGER, TEST_SESSION, false); + } + + // Asserts that the PlanPrinter output for this domain is what was expected + private void assertDomainFormat(VariableReferenceExpression variable, ColumnHandle colHandle, Domain domain, String expected) + { + String printed = domainToPrintedScan(variable, colHandle, domain); + assertTrue(printed.contains(":: " + expected)); + } + + private void assertDomainFormat(Domain domain, String expected) + { + assertDomainFormat(COLUMN_VARIABLE, COLUMN_HANDLE, domain, expected); + } + + @Test + public void testDomainTextFormatting() + { + assertDomainFormat( + Domain.create( + ValueSet.all(VARCHAR), + false), + "[(, )]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice("some string")), + false), + "[[\"some string\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice("here's a quote: \"")), + false), + "[[\"here's a quote: \\\"\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice("")), + false), + "[[\"\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.ofRanges( + Range.greaterThanOrEqual(VARCHAR, utf8Slice("string with \"quotes\" inside")) + .intersect(Range.lessThanOrEqual(VARCHAR, utf8Slice("this string's quote is here -> \"")))), + false), + "[[\"string with \\\"quotes\\\" inside\", \"this string's quote is here -> \\\"\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.ofRanges( + Range.greaterThan(VARCHAR, utf8Slice("string with \"quotes\" inside")) + .intersect(Range.lessThan(VARCHAR, utf8Slice("this string's quote is here -> \"")))), + false), + "[(\"string with \\\"quotes\\\" inside\", \"this string's quote is here -> \\\"\")]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice("")), + false), + "[[\"\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice("")), + false), + "[[\"\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice(", ")), + false), + "[[\", \"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.ofRanges( + Range.greaterThanOrEqual(VARCHAR, utf8Slice("a")) + .intersect(Range.lessThanOrEqual(VARCHAR, utf8Slice("b")))), + false), + "[[\"a\", \"b\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice("a, b")), + false), + "[[\"a, b\"]]"); + + assertDomainFormat( + Domain.create( + ValueSet.of(VARCHAR, utf8Slice("xyz")), + true), + "[NULL, [\"xyz\"]]"); + + assertDomainFormat( + Domain.onlyNull(VARCHAR), + "[NULL]"); + } +}