Skip to content

Fix BigQuery predicate pushdown for varchars with backslashes or newlines #14254

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

Merged
findepi merged 6 commits into from
Sep 23, 2022
Merged

Fix BigQuery predicate pushdown for varchars with backslashes or newlines #14254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
68b4df8
Distinguish different smoke test runs in test report
findepi Sep 22, 2022
ba405b1
Fix big query predicate pushdown for backslashes
findepi Sep 22, 2022
9adfc02
Fix big query predicate pushdown for new lines
findepi Sep 22, 2022
b71a7c7
Smoke test varchar value with backslash at the end
findepi Sep 22, 2022
af45cb4
Remove redundant method call for getting long value
findepi Sep 22, 2022
dc064ee
empty
findepi Sep 23, 2022
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
15 changes: 6 additions & 9 deletions plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ private static OptionalToStringConverter unsupportedToStringConverter()
@VisibleForTesting
public static String dateToStringConverter(Object value)
{
LocalDate date = LocalDate.ofEpochDay(((Long) value).longValue());
return quote(date.toString());
LocalDate date = LocalDate.ofEpochDay((long) value);
return "'" + date + "'";
}

private static String datetimeToStringConverter(Object value)
Expand Down Expand Up @@ -223,8 +223,10 @@ private static ZonedDateTime toZonedDateTime(long epochSeconds, long nanoAdjustm
static String stringToStringConverter(Object value)
{
Slice slice = (Slice) value;
// TODO (https://github.com/trinodb/trino/issues/7900) Add support for all String and Bytes literals
return quote(slice.toStringUtf8().replace("'", "\\'"));
return "'%s'".formatted(slice.toStringUtf8()
.replace("\\", "\\\\")
.replace("\n", "\\n")
.replace("'", "\\'"));
}

static String numericToStringConverter(Object value)
Expand Down Expand Up @@ -314,11 +316,6 @@ private static StandardSQLTypeName toStandardSqlTypeName(Type type)
throw new TrinoException(NOT_SUPPORTED, "Unsupported column type: " + type.getDisplayName());
}

private static String quote(String value)
{
return "'" + value + "'";
}

public Optional<String> convertToString(Type type, Object value)
{
if (type instanceof ArrayType) {
Expand Down
9 changes: 6 additions & 3 deletions testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4156,7 +4156,9 @@ private List<DataMappingTestSetup> testDataMappingSmokeTestData()
.add(new DataMappingTestSetup("char(3)", "'ab'", "'zzz'"))
.add(new DataMappingTestSetup("varchar(3)", "'de'", "'zzz'"))
.add(new DataMappingTestSetup("varchar", "'łąka for the win'", "'ŻŻŻŻŻŻŻŻŻŻ'"))
.add(new DataMappingTestSetup("varchar", "'a \\backslash'", "'a a'")) // `a` sorts after `\`
.add(new DataMappingTestSetup("varchar", "'a \\backslash'", "'a a'")) // `a` sorts after `\`; \b may be interpreted as an escape sequence
.add(new DataMappingTestSetup("varchar", "'end backslash \\'", "'end backslash a'")) // `a` sorts after `\`; final \ before end quote may confuse a parser
.add(new DataMappingTestSetup("varchar", "U&'a \\000a newline'", "'a a'")) // `a` sorts after `\n`; newlines can require special handling in a remote system's language
.add(new DataMappingTestSetup("varbinary", "X'12ab3f'", "X'ffffffffffffffffffff'"))
.build();
}
Expand Down Expand Up @@ -4877,8 +4879,9 @@ public DataMappingTestSetup asUnsupported()
@Override
public String toString()
{
// toString is brief because it's used for test case labels in IDE
return trinoTypeName + (unsupportedType ? "!" : "");
// Used for test case labels in IDE
return trinoTypeName + (unsupportedType ? "!" : "") +
":" + sampleValueLiteral.replaceAll("[^a-zA-Z0-9_-]", "");
}
}
}