Skip to content

Commit

Permalink
Quote field names in TypeSignature
Browse files Browse the repository at this point in the history 
TypeSignature is used for encoding the identity of a type, which
is then parsed (for now) using the SQL parser. Since some fields
names might collide with SQL reserved words, we need to quote them.
  • Loading branch information
@martint
martint committed Nov 8, 2019
1 parent 5bd872a commit 564ae8a
Show file tree
Hide file tree
Showing 19 changed files with 70 additions and 86 deletions.
19 changes: 3 additions & 16 deletions presto-client/src/main/java/io/prestosql/client/RowFieldName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,12 @@
public class RowFieldName
{
private final String name;
private final boolean delimited;

@JsonCreator
public RowFieldName(
@JsonProperty("name") String name,
@JsonProperty("delimited") boolean delimited)
@JsonProperty("name") String name)
{
this.name = requireNonNull(name, "name is null");
this.delimited = delimited;
}

@JsonProperty
Expand All @@ -40,12 +37,6 @@ public String getName()
return name;
}

@JsonProperty
public boolean isDelimited()
{
return delimited;
}

@Override
public boolean equals(Object o)
{
Expand All @@ -58,22 +49,18 @@ public boolean equals(Object o)

RowFieldName other = (RowFieldName) o;

return Objects.equals(this.name, other.name) &&
Objects.equals(this.delimited, other.delimited);
return Objects.equals(this.name, other.name);
}

@Override
public String toString()
{
if (!isDelimited()) {
return name;
}
return '"' + name.replace("\"", "\"\"") + '"';
}

@Override
public int hashCode()
{
return Objects.hash(name, delimited);
return Objects.hash(name);
}
}
8 changes: 4 additions & 4 deletions presto-client/src/test/java/io/prestosql/client/TestClientTypeSignature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public void testJsonRoundTrip()
assertJsonRoundTrip(new ClientTypeSignature(
"row",
ImmutableList.of(
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("foo", false)), bigint)),
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("bar", false)), bigint)))));
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("foo")), bigint)),
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("bar")), bigint)))));
}

@Test
Expand All @@ -64,8 +64,8 @@ public void testStringSerialization()
ClientTypeSignature row = new ClientTypeSignature(
StandardTypes.ROW,
ImmutableList.of(
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("foo", false)), bigint)),
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("bar", false)), bigint))));
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("foo")), bigint)),
ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(Optional.of(new RowFieldName("bar")), bigint))));
assertEquals(row.toString(), "row(foo bigint,bar bigint)");
}

Expand Down
2 changes: 1 addition & 1 deletion presto-client/src/test/java/io/prestosql/client/TestFixJsonDataUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private static ClientTypeSignatureParameter toClientTypeSignatureParameter(TypeS
case NAMED_TYPE:
return ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(
parameter.getNamedTypeSignature().getFieldName().map(value ->
new RowFieldName(value.getName(), value.isDelimited())),
new RowFieldName(value.getName())),
toClientTypeSignature(parameter.getNamedTypeSignature().getTypeSignature())));
case LONG:
return ClientTypeSignatureParameter.ofLong(parameter.getLongLiteral());
Expand Down
2 changes: 1 addition & 1 deletion presto-hive/src/main/java/io/prestosql/plugin/hive/HiveType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ private static TypeSignature getTypeSignature(TypeInfo typeInfo)
// Users can't work around this by casting in their queries because Presto parser always lower case types.
// TODO: This is a hack. Presto engine should be able to handle identifiers in a case insensitive way where necessary.
String rowFieldName = structFieldNames.get(i).toLowerCase(Locale.US);
typeSignatureBuilder.add(TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(Optional.of(new RowFieldName(rowFieldName, false)), typeSignature)));
typeSignatureBuilder.add(TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(Optional.of(new RowFieldName(rowFieldName)), typeSignature)));
}
return new TypeSignature(StandardTypes.ROW, typeSignatureBuilder.build());
}
Expand Down
8 changes: 4 additions & 4 deletions presto-hive/src/test/java/io/prestosql/plugin/hive/AbstractTestHive.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,9 @@ public abstract class AbstractTestHive
private static final Type ARRAY_TYPE = arrayType(createUnboundedVarcharType());
private static final Type MAP_TYPE = mapType(createUnboundedVarcharType(), BIGINT);
private static final Type ROW_TYPE = rowType(ImmutableList.of(
new NamedTypeSignature(Optional.of(new RowFieldName("f_string", false)), createUnboundedVarcharType().getTypeSignature()),
new NamedTypeSignature(Optional.of(new RowFieldName("f_bigint", false)), BIGINT.getTypeSignature()),
new NamedTypeSignature(Optional.of(new RowFieldName("f_boolean", false)), BOOLEAN.getTypeSignature())));
new NamedTypeSignature(Optional.of(new RowFieldName("f_string")), createUnboundedVarcharType().getTypeSignature()),
new NamedTypeSignature(Optional.of(new RowFieldName("f_bigint")), BIGINT.getTypeSignature()),
new NamedTypeSignature(Optional.of(new RowFieldName("f_boolean")), BOOLEAN.getTypeSignature())));

private static final List<ColumnMetadata> CREATE_TABLE_COLUMNS = ImmutableList.<ColumnMetadata>builder()
.add(new ColumnMetadata("id", BIGINT))
Expand Down Expand Up @@ -356,7 +356,7 @@ public abstract class AbstractTestHive
private static RowType toRowType(List<ColumnMetadata> columns)
{
return rowType(columns.stream()
.map(col -> new NamedTypeSignature(Optional.of(new RowFieldName(format("f_%s", col.getName()), false)), col.getType().getTypeSignature()))
.map(col -> new NamedTypeSignature(Optional.of(new RowFieldName(format("f_%s", col.getName()))), col.getType().getTypeSignature()))
.collect(toImmutableList()));
}

Expand Down
6 changes: 3 additions & 3 deletions presto-main/src/main/java/io/prestosql/operator/TypeSignatureParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ else if (c == ')') {
verify(tokenStart >= 0, "Expect tokenStart to be non-negative");
verify(delimitedColumnName != null, "Expect delimitedColumnName to be non-null");
fields.add(TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(
Optional.of(new RowFieldName(delimitedColumnName, true)),
Optional.of(new RowFieldName(delimitedColumnName)),
parseTypeSignature(signature.substring(tokenStart, i).trim(), literalParameters))));
delimitedColumnName = null;
tokenStart = -1;
Expand All @@ -204,7 +204,7 @@ else if (c == ',' && bracketLevel == 1) {
verify(tokenStart >= 0, "Expect tokenStart to be non-negative");
verify(delimitedColumnName != null, "Expect delimitedColumnName to be non-null");
fields.add(TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(
Optional.of(new RowFieldName(delimitedColumnName, true)),
Optional.of(new RowFieldName(delimitedColumnName)),
parseTypeSignature(signature.substring(tokenStart, i).trim(), literalParameters))));
delimitedColumnName = null;
tokenStart = -1;
Expand Down Expand Up @@ -238,7 +238,7 @@ private static TypeSignatureParameter parseTypeOrNamedType(String typeOrNamedTyp
String firstPart = typeOrNamedType.substring(0, split);
if (IDENTIFIER_PATTERN.matcher(firstPart).matches()) {
return TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(
Optional.of(new RowFieldName(firstPart, false)),
Optional.of(new RowFieldName(firstPart)),
parseTypeSignature(typeOrNamedType.substring(split + 1).trim(), literalParameters)));
}

Expand Down
2 changes: 1 addition & 1 deletion presto-main/src/main/java/io/prestosql/server/protocol/Query.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ private static ClientTypeSignatureParameter toClientTypeSignatureParameter(TypeS
case NAMED_TYPE:
return ClientTypeSignatureParameter.ofNamedType(new NamedClientTypeSignature(
parameter.getNamedTypeSignature().getFieldName().map(value ->
new RowFieldName(value.getName(), value.isDelimited())),
new RowFieldName(value.getName())),
toClientTypeSignature(parameter.getNamedTypeSignature().getTypeSignature())));
case LONG:
return ClientTypeSignatureParameter.ofLong(parameter.getLongLiteral());
Expand Down
2 changes: 1 addition & 1 deletion presto-main/src/main/java/io/prestosql/sql/analyzer/TypeSignatureTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private static TypeSignature toTypeSignature(RowDataType type)
.map(field -> namedTypeParameter(new NamedTypeSignature(
field.getName()
.map(TypeSignatureTranslator::canonicalize)
.map(value -> new RowFieldName(value, false)),
.map(value -> new RowFieldName(value)),
toTypeSignature(field.getType()))))
.collect(toImmutableList());

Expand Down
59 changes: 29 additions & 30 deletions presto-main/src/test/java/io/prestosql/operator/TestTypeSignature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,32 +64,32 @@ public void parseRowSignature()
// row signature with named fields
assertRowSignature(
"row(a bigint,b varchar)",
rowSignature(namedParameter("a", false, signature("bigint")), namedParameter("b", false, varchar())));
rowSignature(namedParameter("a", signature("bigint")), namedParameter("b", varchar())));
assertRowSignature(
"row(__a__ bigint,_b@_: _varchar)",
rowSignature(namedParameter("__a__", false, signature("bigint")), namedParameter("_b@_:", false, signature("_varchar"))));
rowSignature(namedParameter("__a__", signature("bigint")), namedParameter("_b@_:", signature("_varchar"))));
assertRowSignature(
"row(a bigint,b array(bigint),c row(a bigint))",
rowSignature(
namedParameter("a", false, signature("bigint")),
namedParameter("b", false, array(signature("bigint"))),
namedParameter("c", false, rowSignature(namedParameter("a", false, signature("bigint"))))));
namedParameter("a", signature("bigint")),
namedParameter("b", array(signature("bigint"))),
namedParameter("c", rowSignature(namedParameter("a", signature("bigint"))))));
assertRowSignature(
"row(a varchar(10),b row(a bigint))",
rowSignature(
namedParameter("a", false, varchar(10)),
namedParameter("b", false, rowSignature(namedParameter("a", false, signature("bigint"))))));
namedParameter("a", varchar(10)),
namedParameter("b", rowSignature(namedParameter("a", signature("bigint"))))));
assertRowSignature(
"array(row(col0 bigint,col1 double))",
array(rowSignature(namedParameter("col0", false, signature("bigint")), namedParameter("col1", false, signature("double")))));
array(rowSignature(namedParameter("col0", signature("bigint")), namedParameter("col1", signature("double")))));
assertRowSignature(
"row(col0 array(row(col0 bigint,col1 double)))",
rowSignature(namedParameter("col0", false, array(
rowSignature(namedParameter("col0", false, signature("bigint")), namedParameter("col1", false, signature("double")))))));
rowSignature(namedParameter("col0", array(
rowSignature(namedParameter("col0", signature("bigint")), namedParameter("col1", signature("double")))))));
assertRowSignature(
"row(a decimal(p1,s1),b decimal(p2,s2))",
ImmutableSet.of("p1", "s1", "p2", "s2"),
rowSignature(namedParameter("a", false, decimal("p1", "s1")), namedParameter("b", false, decimal("p2", "s2"))));
rowSignature(namedParameter("a", decimal("p1", "s1")), namedParameter("b", decimal("p2", "s2"))));

// row with mixed fields
assertRowSignature(
Expand All @@ -100,40 +100,40 @@ public void parseRowSignature()
rowSignature(
unnamedParameter(signature("bigint")),
unnamedParameter(array(signature("bigint"))),
unnamedParameter(rowSignature(namedParameter("a", false, signature("bigint"))))));
unnamedParameter(rowSignature(namedParameter("a", signature("bigint"))))));
assertRowSignature(
"row(varchar(10),b row(bigint))",
rowSignature(
unnamedParameter(varchar(10)),
namedParameter("b", false, rowSignature(unnamedParameter(signature("bigint"))))));
namedParameter("b", rowSignature(unnamedParameter(signature("bigint"))))));
assertRowSignature(
"array(row(col0 bigint,double))",
array(rowSignature(namedParameter("col0", false, signature("bigint")), unnamedParameter(signature("double")))));
array(rowSignature(namedParameter("col0", signature("bigint")), unnamedParameter(signature("double")))));
assertRowSignature(
"row(col0 array(row(bigint,double)))",
rowSignature(namedParameter("col0", false, array(
rowSignature(namedParameter("col0", array(
rowSignature(unnamedParameter(signature("bigint")), unnamedParameter(signature("double")))))));
assertRowSignature(
"row(a decimal(p1,s1),decimal(p2,s2))",
ImmutableSet.of("p1", "s1", "p2", "s2"),
rowSignature(namedParameter("a", false, decimal("p1", "s1")), unnamedParameter(decimal("p2", "s2"))));
rowSignature(namedParameter("a", decimal("p1", "s1")), unnamedParameter(decimal("p2", "s2"))));

// named fields of types with spaces
assertRowSignature(
"row(time time with time zone)",
rowSignature(namedParameter("time", false, signature("time with time zone"))));
rowSignature(namedParameter("time", signature("time with time zone"))));
assertRowSignature(
"row(time timestamp with time zone)",
rowSignature(namedParameter("time", false, signature("timestamp with time zone"))));
rowSignature(namedParameter("time", signature("timestamp with time zone"))));
assertRowSignature(
"row(interval interval day to second)",
rowSignature(namedParameter("interval", false, signature("interval day to second"))));
rowSignature(namedParameter("interval", signature("interval day to second"))));
assertRowSignature(
"row(interval interval year to month)",
rowSignature(namedParameter("interval", false, signature("interval year to month"))));
rowSignature(namedParameter("interval", signature("interval year to month"))));
assertRowSignature(
"row(double double precision)",
rowSignature(namedParameter("double", false, signature("double precision"))));
rowSignature(namedParameter("double", signature("double precision"))));

// unnamed fields of types with spaces
assertRowSignature(
Expand Down Expand Up @@ -162,25 +162,25 @@ public void parseRowSignature()
assertRowSignature(
"row(\"time with time zone\" time with time zone,\"double\" double)",
rowSignature(
namedParameter("time with time zone", true, signature("time with time zone")),
namedParameter("double", true, signature("double"))));
namedParameter("time with time zone", signature("time with time zone")),
namedParameter("double", signature("double"))));

// allow spaces
assertSignature(
"row( time time with time zone, array( interval day to seconds ) )",
"row",
ImmutableList.of("time time with time zone", "array(interval day to seconds)"),
"row(time time with time zone,array(interval day to seconds))");
ImmutableList.of("\"time\" time with time zone", "array(interval day to seconds)"),
"row(\"time\" time with time zone,array(interval day to seconds))");

// preserve base name case
assertRowSignature(
"RoW(a bigint,b varchar)",
rowSignature(namedParameter("a", false, signature("bigint")), namedParameter("b", false, varchar())));
rowSignature(namedParameter("a", signature("bigint")), namedParameter("b", varchar())));

// signature with invalid type
assertRowSignature(
"row(\"time\" with time zone)",
rowSignature(namedParameter("time", true, signature("with time zone"))));
rowSignature(namedParameter("time", signature("with time zone"))));
}

private TypeSignature varchar()
Expand All @@ -204,9 +204,9 @@ private static TypeSignature rowSignature(NamedTypeSignature... columns)
return new TypeSignature("row", transform(asList(columns), TypeSignatureParameter::namedTypeParameter));
}

private static NamedTypeSignature namedParameter(String name, boolean delimited, TypeSignature value)
private static NamedTypeSignature namedParameter(String name, TypeSignature value)
{
return new NamedTypeSignature(Optional.of(new RowFieldName(name, delimited)), value);
return new NamedTypeSignature(Optional.of(new RowFieldName(name)), value);
}

private static NamedTypeSignature unnamedParameter(TypeSignature value)
Expand Down Expand Up @@ -302,7 +302,6 @@ private static void assertRowSignature(
{
TypeSignature signature = parseTypeSignature(typeName, literalParameters);
assertEquals(signature, expectedSignature);
assertEquals(signature.toString(), typeName);
}

private static void assertRowSignature(
Expand Down
4 changes: 2 additions & 2 deletions presto-main/src/test/java/io/prestosql/type/TestRowParametricType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public void testTypeSignatureRoundTrip()
TypeManager typeManager = new InternalTypeManager(createTestMetadataManager());
TypeSignature typeSignature = new TypeSignature(
ROW,
TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(Optional.of(new RowFieldName("col1", false)), BIGINT.getTypeSignature())),
TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(Optional.of(new RowFieldName("col2", true)), DOUBLE.getTypeSignature())));
TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(Optional.of(new RowFieldName("col1")), BIGINT.getTypeSignature())),
TypeSignatureParameter.namedTypeParameter(new NamedTypeSignature(Optional.of(new RowFieldName("col2")), DOUBLE.getTypeSignature())));
List<TypeParameter> parameters = typeSignature.getParameters().stream()
.map(parameter -> TypeParameter.of(parameter, typeManager))
.collect(Collectors.toList());
Expand Down
Loading

0 comments on commit 564ae8a

Please sign in to comment.