From 19df7a924b0a70bdab3d43202cf05a64233d2cac Mon Sep 17 00:00:00 2001 From: Elon Azoulay Date: Tue, 18 Apr 2023 14:42:29 -0700 Subject: [PATCH 1/2] Fix SQL formatting in TestDynamicTable --- .../pinot/query/DynamicTablePqlExtractor.java | 16 +- .../pinot/BasePinotConnectorSmokeTest.java | 4 +- .../trino/plugin/pinot/TestDynamicTable.java | 245 +++++++++++------- 3 files changed, 164 insertions(+), 101 deletions(-) diff --git a/plugin/trino-pinot/src/main/java/io/trino/plugin/pinot/query/DynamicTablePqlExtractor.java b/plugin/trino-pinot/src/main/java/io/trino/plugin/pinot/query/DynamicTablePqlExtractor.java index 40ff0c5dccaf..389730874ddb 100755 --- a/plugin/trino-pinot/src/main/java/io/trino/plugin/pinot/query/DynamicTablePqlExtractor.java +++ b/plugin/trino-pinot/src/main/java/io/trino/plugin/pinot/query/DynamicTablePqlExtractor.java @@ -33,7 +33,7 @@ private DynamicTablePqlExtractor() public static String extractPql(DynamicTable table, TupleDomain tupleDomain) { StringBuilder builder = new StringBuilder(); - builder.append("select "); + builder.append("SELECT "); if (!table.getProjections().isEmpty()) { builder.append(table.getProjections().stream() .map(DynamicTablePqlExtractor::formatExpression) @@ -49,34 +49,34 @@ public static String extractPql(DynamicTable table, TupleDomain tu .map(DynamicTablePqlExtractor::formatExpression) .collect(joining(", "))); } - builder.append(" from "); + builder.append(" FROM "); builder.append(table.getTableName()); builder.append(table.getSuffix().orElse("")); Optional filter = getFilter(table.getFilter(), tupleDomain, false); if (filter.isPresent()) { - builder.append(" where ") + builder.append(" WHERE ") .append(filter.get()); } if (!table.getGroupingColumns().isEmpty()) { - builder.append(" group by "); + builder.append(" GROUP BY "); builder.append(table.getGroupingColumns().stream() .map(PinotColumnHandle::getExpression) .collect(joining(", "))); } Optional havingClause = getFilter(table.getHavingExpression(), tupleDomain, true); if (havingClause.isPresent()) { - builder.append(" having ") + builder.append(" HAVING ") .append(havingClause.get()); } if (!table.getOrderBy().isEmpty()) { - builder.append(" order by ") + builder.append(" ORDER BY ") .append(table.getOrderBy().stream() .map(DynamicTablePqlExtractor::convertOrderByExpressionToPql) .collect(joining(", "))); } if (table.getLimit().isPresent()) { - builder.append(" limit "); + builder.append(" LIMIT "); if (table.getOffset().isPresent()) { builder.append(table.getOffset().getAsLong()) .append(", "); @@ -108,7 +108,7 @@ private static String convertOrderByExpressionToPql(OrderByExpression orderByExp StringBuilder builder = new StringBuilder() .append(orderByExpression.getExpression()); if (!orderByExpression.isAsc()) { - builder.append(" desc"); + builder.append(" DESC"); } return builder.toString(); } diff --git a/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java b/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java index e38170e6104b..7fe025643e9a 100644 --- a/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java +++ b/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java @@ -1303,7 +1303,7 @@ public void testMaxLimitForPassthroughQueries() assertQueryFails("SELECT string_col, updated_at_seconds" + " FROM \"SELECT updated_at_seconds, string_col FROM " + TOO_MANY_BROKER_ROWS_TABLE + " LIMIT " + (MAX_ROWS_PER_SPLIT_FOR_BROKER_QUERIES + 1) + "\"", - "Broker query returned '13' rows, maximum allowed is '12' rows. with query \"select \"updated_at_seconds\", \"string_col\" from too_many_broker_rows limit 13\""); + "Broker query returned '13' rows, maximum allowed is '12' rows. with query \"SELECT \"updated_at_seconds\", \"string_col\" FROM too_many_broker_rows LIMIT 13\""); // Pinot issue preventing Integer.MAX_VALUE from being a limit: https://github.com/apache/incubator-pinot/issues/7110 // This is now resolved in pinot 0.8.0 @@ -1312,7 +1312,7 @@ public void testMaxLimitForPassthroughQueries() // Pinot broker requests do not handle limits greater than Integer.MAX_VALUE // Note that -2147483648 is due to an integer overflow in Pinot: https://github.com/apache/pinot/issues/7242 assertQueryFails("SELECT * FROM \"SELECT string_col, long_col FROM " + ALL_TYPES_TABLE + " LIMIT " + ((long) Integer.MAX_VALUE + 1) + "\"", - "(?s)Query select \"string_col\", \"long_col\" from alltypes limit -2147483648 encountered exception .* with query \"select \"string_col\", \"long_col\" from alltypes limit -2147483648\""); + "(?s)Query SELECT \"string_col\", \"long_col\" FROM alltypes LIMIT -2147483648 encountered exception .* with query \"SELECT \"string_col\", \"long_col\" FROM alltypes LIMIT -2147483648\""); List tooManyBrokerRowsTableValues = new ArrayList<>(); for (int i = 0; i < MAX_ROWS_PER_SPLIT_FOR_BROKER_QUERIES; i++) { diff --git a/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/TestDynamicTable.java b/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/TestDynamicTable.java index 5724f20cb140..148783556999 100755 --- a/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/TestDynamicTable.java +++ b/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/TestDynamicTable.java @@ -34,7 +34,6 @@ import static io.trino.plugin.pinot.query.DynamicTableBuilder.buildFromPql; import static io.trino.plugin.pinot.query.DynamicTablePqlExtractor.extractPql; import static io.trino.spi.type.VarcharType.VARCHAR; -import static java.lang.String.format; import static java.lang.String.join; import static java.util.Locale.ENGLISH; import static java.util.stream.Collectors.joining; @@ -55,11 +54,11 @@ public void testSelectNoFilter() .map(columnName -> new OrderByExpression(quoteIdentifier(columnName), true)) .collect(toList()); long limit = 230; - String query = format("select %s from %s order by %s limit %s", + String query = "SELECT %s FROM %s ORDER BY %s DESC LIMIT %s".formatted( join(", ", columnNames), tableName, orderByColumns.stream() - .collect(joining(", ")) + " desc", + .collect(joining(", ")), limit); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); assertEquals(dynamicTable.getProjections().stream() @@ -76,7 +75,7 @@ public void testGroupBy() { String tableName = realtimeOnlyTable.getTableName(); long limit = 25; - String query = format("SELECT Origin, AirlineID, max(CarrierDelay), avg(CarrierDelay) FROM %s GROUP BY Origin, AirlineID LIMIT %s", tableName, limit); + String query = "SELECT Origin, AirlineID, max(CarrierDelay), avg(CarrierDelay) FROM %s GROUP BY Origin, AirlineID LIMIT %s".formatted(tableName, limit); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); assertEquals(dynamicTable.getGroupingColumns().stream() .map(PinotColumnHandle::getColumnName) @@ -92,16 +91,22 @@ public void testGroupBy() public void testFilter() { String tableName = realtimeOnlyTable.getTableName(); - String query = format("select FlightNum, AirlineID from %s where ((CancellationCode IN ('strike', 'weather', 'pilot_bac')) AND (Origin = 'jfk')) " + - "OR ((OriginCityName != 'catfish paradise') AND (OriginState != 'az') AND (AirTime between 1 and 5)) " + - "AND AirTime NOT IN (7,8,9) " + - "OR ((DepDelayMinutes < 10) AND (Distance >= 3) AND (ArrDelay > 4) AND (SecurityDelay < 5) AND (LateAircraftDelay <= 7)) limit 60", - tableName); + String query = """ + SELECT FlightNum, AirlineID + FROM %s + WHERE ((CancellationCode IN ('strike', 'weather', 'pilot_bac')) AND (Origin = 'jfk')) + OR ((OriginCityName != 'catfish paradise') AND (OriginState != 'az') AND (AirTime BETWEEN 1 AND 5)) + AND AirTime NOT IN (7,8,9) + OR ((DepDelayMinutes < 10) AND (Distance >= 3) AND (ArrDelay > 4) AND (SecurityDelay < 5) AND (LateAircraftDelay <= 7)) + LIMIT 60""".formatted(tableName); - String expected = format("select \"FlightNum\", \"AirlineID\" from %s where OR(AND(\"CancellationCode\" IN ('strike', 'weather', 'pilot_bac'), (\"Origin\") = 'jfk'), " + - "AND((\"OriginCityName\") != 'catfish paradise', (\"OriginState\") != 'az', (\"AirTime\") BETWEEN '1' AND '5', \"AirTime\" NOT IN ('7', '8', '9')), " + - "AND((\"DepDelayMinutes\") < '10', (\"Distance\") >= '3', (\"ArrDelay\") > '4', (\"SecurityDelay\") < '5', (\"LateAircraftDelay\") <= '7')) limit 60", - tableName); + String expected = """ + SELECT "FlightNum", "AirlineID"\ + FROM %s\ + WHERE OR(AND("CancellationCode" IN ('strike', 'weather', 'pilot_bac'), ("Origin") = 'jfk'),\ + AND(("OriginCityName") != 'catfish paradise', ("OriginState") != 'az', ("AirTime") BETWEEN '1' AND '5', "AirTime" NOT IN ('7', '8', '9')),\ + AND(("DepDelayMinutes") < '10', ("Distance") >= '3', ("ArrDelay") > '4', ("SecurityDelay") < '5', ("LateAircraftDelay") <= '7'))\ + LIMIT 60""".formatted(tableName); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expected); } @@ -114,13 +119,18 @@ public void testPrimitiveTypes() // ci will interpret as a string, i.e. "X''ABCD" // intellij will expand the X'abcd' to binary 0b10011100001111 // Pinot will interpret both forms as a string regardless, so we cannot use X'....' - String tableName = "primitive_types_table"; - String query = "SELECT string_col, long_col, int_col, bool_col, double_col, float_col, bytes_col" + - " FROM " + tableName + " WHERE string_col = 'string' AND long_col = 12345678901 AND int_col = 123456789" + - " AND double_col = 3.56 AND float_col = 3.56 AND bytes_col = 'abcd' LIMIT 60"; - String expected = "select \"string_col\", \"long_col\", \"int_col\", \"bool_col\", \"double_col\", \"float_col\", \"bytes_col\"" + - " from primitive_types_table where AND((\"string_col\") = 'string', (\"long_col\") = '12345678901'," + - " (\"int_col\") = '123456789', (\"double_col\") = '3.56', (\"float_col\") = '3.56', (\"bytes_col\") = 'abcd') limit 60"; + String query = """ + SELECT string_col, long_col, int_col, bool_col, double_col, float_col, bytes_col + FROM primitive_types_table + WHERE string_col = 'string' AND long_col = 12345678901 AND int_col = 123456789 + AND double_col = 3.56 AND float_col = 3.56 AND bytes_col = 'abcd' + LIMIT 60"""; + String expected = """ + SELECT "string_col", "long_col", "int_col", "bool_col", "double_col", "float_col", "bytes_col"\ + FROM primitive_types_table\ + WHERE AND(("string_col") = 'string', ("long_col") = '12345678901',\ + ("int_col") = '123456789', ("double_col") = '3.56', ("float_col") = '3.56', ("bytes_col") = 'abcd')\ + LIMIT 60"""; DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expected); } @@ -129,9 +139,9 @@ public void testPrimitiveTypes() public void testDoubleWithScientificNotation() { // Pinot recognizes double literals with scientific notation as of version 0.8.0 - String tableName = "primitive_types_table"; - String query = "SELECT string_col FROM " + tableName + " WHERE double_col = 3.5E5"; - String expected = "select \"string_col\" from primitive_types_table where (\"double_col\") = '350000.0' limit 10"; + String query = "SELECT string_col FROM primitive_types_table WHERE double_col = 3.5E5"; + String expected = """ + SELECT "string_col" FROM primitive_types_table WHERE ("double_col") = '350000.0' LIMIT 10"""; DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expected); } @@ -139,11 +149,15 @@ public void testDoubleWithScientificNotation() @Test public void testFilterWithCast() { - String tableName = "primitive_types_table"; - String query = "SELECT string_col, long_col" + - " FROM " + tableName + " WHERE string_col = CAST(123 AS STRING) AND long_col = CAST('123' AS LONG) LIMIT 60"; - String expected = "select \"string_col\", \"long_col\" from primitive_types_table " + - "where AND((\"string_col\") = '123', (\"long_col\") = '123') limit 60"; + String query = """ + SELECT string_col, long_col + FROM primitive_types_table + WHERE string_col = CAST(123 AS STRING) AND long_col = CAST('123' AS LONG) + LIMIT 60"""; + String expected = """ + SELECT "string_col", "long_col" FROM primitive_types_table\ + WHERE AND(("string_col") = '123', ("long_col") = '123')\ + LIMIT 60"""; DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expected); } @@ -152,15 +166,19 @@ public void testFilterWithCast() public void testFilterWithCaseStatements() { String tableName = realtimeOnlyTable.getTableName(); - String query = format("select FlightNum, AirlineID from %s " + - "where case when cancellationcode = 'strike' then 3 else 4 end != 5 " + - "AND case origincityname when 'nyc' then 'pizza' when 'la' then 'burrito' when 'boston' then 'clam chowder' " + - "else 'burger' end != 'salad'", tableName); - String expected = format("select \"FlightNum\", \"AirlineID\" from %s where AND((CASE WHEN equals(\"CancellationCode\", 'strike') " + - "THEN '3' ELSE '4' END) != '5', (CASE WHEN equals(\"OriginCityName\", 'nyc') " + - "THEN 'pizza' WHEN equals(\"OriginCityName\", 'la') THEN 'burrito' WHEN equals(\"OriginCityName\", 'boston') " + - "THEN 'clam chowder' ELSE 'burger' END) != 'salad') limit 10", - tableName); + String query = """ + SELECT FlightNum, AirlineID + FROM %s + WHERE CASE WHEN cancellationcode = 'strike' THEN 3 ELSE 4 END != 5 + AND CASE origincityname WHEN 'nyc' THEN 'pizza' WHEN 'la' THEN 'burrito' WHEN 'boston' THEN 'clam chowder' + ELSE 'burger' END != 'salad'""".formatted(tableName); + String expected = """ + SELECT "FlightNum", "AirlineID"\ + FROM %s\ + WHERE AND((CASE WHEN equals("CancellationCode", 'strike')\ + THEN '3' ELSE '4' END) != '5', (CASE WHEN equals("OriginCityName", 'nyc')\ + THEN 'pizza' WHEN equals("OriginCityName", 'la') THEN 'burrito' WHEN equals("OriginCityName", 'boston')\ + THEN 'clam chowder' ELSE 'burger' END) != 'salad') LIMIT 10""".formatted(tableName); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expected); } @@ -169,13 +187,17 @@ public void testFilterWithCaseStatements() public void testFilterWithPushdownConstraint() { String tableName = realtimeOnlyTable.getTableName(); - String query = format("select FlightNum from %s limit 60", tableName.toLowerCase(ENGLISH)); + String query = "SELECT FlightNum FROM %s LIMIT 60".formatted(tableName.toLowerCase(ENGLISH)); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); PinotColumnHandle columnHandle = new PinotColumnHandle("OriginCityName", VARCHAR); TupleDomain tupleDomain = TupleDomain.withColumnDomains(ImmutableMap.of( columnHandle, Domain.create(ValueSet.ofRanges(Range.equal(VARCHAR, Slices.utf8Slice("Catfish Paradise"))), false))); - String expectedPql = "select \"FlightNum\" from realtimeOnly where (\"OriginCityName\" = 'Catfish Paradise') limit 60"; + String expectedPql = """ + SELECT "FlightNum"\ + FROM realtimeOnly\ + WHERE ("OriginCityName" = 'Catfish Paradise')\ + LIMIT 60"""; assertEquals(extractPql(dynamicTable, tupleDomain), expectedPql); } @@ -185,9 +207,13 @@ public void testFilterWithUdf() String tableName = realtimeOnlyTable.getTableName(); // Note: before Pinot 0.12.1 the below query produced different results due to handling IEEE-754 approximate numerics // See https://github.com/apache/pinot/issues/10637 - String query = format("select FlightNum from %s where DivLongestGTimes = FLOOR(EXP(2 * LN(3)) + 0.1) AND 5 < EXP(CarrierDelay) limit 60", tableName.toLowerCase(ENGLISH)); + String query = "SELECT FlightNum FROM %s WHERE DivLongestGTimes = FLOOR(EXP(2 * LN(3)) + 0.1) AND 5 < EXP(CarrierDelay) LIMIT 60".formatted(tableName.toLowerCase(ENGLISH)); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = "select \"FlightNum\" from realtimeOnly where AND((\"DivLongestGTimes\") = '9.0', (exp(\"CarrierDelay\")) > '5') limit 60"; + String expectedPql = """ + SELECT "FlightNum"\ + FROM realtimeOnly\ + WHERE AND(("DivLongestGTimes") = '9.0', (exp("CarrierDelay")) > '5')\ + LIMIT 60"""; assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); } @@ -195,9 +221,9 @@ public void testFilterWithUdf() public void testSelectStarDynamicTable() { String tableName = realtimeOnlyTable.getTableName(); - String query = format("select * from %s limit 70", tableName.toLowerCase(ENGLISH)); + String query = "SELECT * FROM %s LIMIT 70".formatted(tableName.toLowerCase(ENGLISH)); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select %s from %s limit 70", getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableName); + String expectedPql = "SELECT %s FROM %s LIMIT 70".formatted(getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableName); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); } @@ -206,9 +232,9 @@ public void testOfflineDynamicTable() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + OFFLINE_SUFFIX; - String query = format("select * from %s limit 70", tableNameWithSuffix); + String query = "SELECT * FROM %s LIMIT 70".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select %s from %s limit 70", getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableNameWithSuffix); + String expectedPql = "SELECT %s FROM %s LIMIT 70".formatted(getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -218,9 +244,9 @@ public void testRealtimeOnlyDynamicTable() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select * from %s limit 70", tableNameWithSuffix); + String query = "SELECT * FROM %s LIMIT 70".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select %s from %s limit 70", getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableNameWithSuffix); + String expectedPql = "SELECT %s FROM %s LIMIT 70".formatted(getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -230,9 +256,9 @@ public void testLimitAndOffset() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select * from %s limit 70, 40", tableNameWithSuffix); + String query = "SELECT * FROM %s LIMIT 70, 40".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select %s from %s limit 70, 40", getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableNameWithSuffix); + String expectedPql = "SELECT %s FROM %s LIMIT 70, 40".formatted(getColumnNames(tableName).stream().map(TestDynamicTable::quoteIdentifier).collect(joining(", ")), tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -247,9 +273,10 @@ public void testRegexpLike() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select origincityname from %s where regexp_like(origincityname, '.*york.*') limit 70", tableNameWithSuffix); + String query = "SELECT origincityname FROM %s WHERE regexp_like(origincityname, '.*york.*') LIMIT 70".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select \"OriginCityName\" from %s where regexp_like(\"OriginCityName\", '.*york.*') limit 70", tableNameWithSuffix); + String expectedPql = """ + SELECT "OriginCityName" FROM %s WHERE regexp_like("OriginCityName", '.*york.*') LIMIT 70""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -259,9 +286,10 @@ public void testTextMatch() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select origincityname from %s where text_match(origincityname, 'new AND york') limit 70", tableNameWithSuffix); + String query = "SELECT origincityname FROM %s WHERE text_match(origincityname, 'new AND york') LIMIT 70".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select \"OriginCityName\" from %s where text_match(\"OriginCityName\", 'new and york') limit 70", tableNameWithSuffix); + String expectedPql = """ + SELECT "OriginCityName" FROM %s WHERE text_match("OriginCityName", 'new and york') LIMIT 70""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -271,9 +299,11 @@ public void testJsonMatch() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select origincityname from %s where json_match(origincityname, '\"$.name\"=''new york''') limit 70", tableNameWithSuffix); + String query = """ + SELECT origincityname FROM %s WHERE json_match(origincityname, '"$.name"=''new york''') LIMIT 70""".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select \"OriginCityName\" from %s where json_match(\"OriginCityName\", '\"$.name\"=''new york''') limit 70", tableNameWithSuffix); + String expectedPql = """ + SELECT "OriginCityName" FROM %s WHERE json_match("OriginCityName", '"$.name"=''new york''') LIMIT 70""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -283,16 +313,21 @@ public void testSelectExpressionsWithAliases() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select datetimeconvert(dayssinceEpoch, '1:seconds:epoch', '1:milliseconds:epoch', '15:minutes'), " + - "case origincityname when 'nyc' then 'pizza' when 'la' then 'burrito' when 'boston' then 'clam chowder'" + - " else 'burger' end != 'salad'," + - " timeconvert(dayssinceEpoch, 'seconds', 'minutes') as foo" + - " from %s limit 70", tableNameWithSuffix); + String query = """ + SELECT datetimeconvert(dayssinceEpoch, '1:seconds:epoch', '1:milliseconds:epoch', '15:minutes'), + CASE origincityname WHEN 'nyc' THEN 'pizza' WHEN 'la' THEN 'burrito' WHEN 'boston' THEN 'clam chowder' + ELSE 'burger' END != 'salad', + timeconvert(dayssinceEpoch, 'seconds', 'minutes') AS foo + FROM %s + LIMIT 70""".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select datetimeconvert(\"DaysSinceEpoch\", '1:SECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '15:MINUTES')," + - " not_equals(CASE WHEN equals(\"OriginCityName\", 'nyc') THEN 'pizza' WHEN equals(\"OriginCityName\", 'la') THEN 'burrito' WHEN equals(\"OriginCityName\", 'boston') THEN 'clam chowder' ELSE 'burger' END, 'salad')," + - " timeconvert(\"DaysSinceEpoch\", 'SECONDS', 'MINUTES') AS \"foo\" from %s limit 70", tableNameWithSuffix); + String expectedPql = """ + SELECT datetimeconvert("DaysSinceEpoch", '1:SECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '15:MINUTES'),\ + not_equals(CASE WHEN equals("OriginCityName", 'nyc') THEN 'pizza' WHEN equals("OriginCityName", 'la') THEN 'burrito' WHEN equals(\"OriginCityName\", 'boston') THEN 'clam chowder' ELSE 'burger' END, 'salad'),\ + timeconvert("DaysSinceEpoch", 'SECONDS', 'MINUTES') AS "foo"\ + FROM %s\ + LIMIT 70""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -302,26 +337,30 @@ public void testAggregateExpressionsWithAliases() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select datetimeconvert(dayssinceEpoch, '1:seconds:epoch', '1:milliseconds:epoch', '15:minutes'), " + - " count(*) as bar," + - " case origincityname when 'nyc' then 'pizza' when 'la' then 'burrito' when 'boston' then 'clam chowder'" + - " else 'burger' end != 'salad'," + - " timeconvert(dayssinceEpoch, 'seconds', 'minutes') as foo," + - " max(airtime) as baz" + - " from %s group by 1, 3, 4 limit 70", tableNameWithSuffix); + String query = """ + SELECT datetimeconvert(dayssinceEpoch, '1:seconds:epoch', '1:milliseconds:epoch', '15:minutes'), + count(*) AS bar, + CASE origincityname WHEN 'nyc' then 'pizza' WHEN 'la' THEN 'burrito' WHEN 'boston' THEN 'clam chowder' + ELSE 'burger' END != 'salad', + timeconvert(dayssinceEpoch, 'seconds', 'minutes') AS foo, + max(airtime) as baz + FROM %s + GROUP BY 1, 3, 4 + LIMIT 70""".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select datetimeconvert(\"DaysSinceEpoch\", '1:SECONDS:EPOCH'," + - " '1:MILLISECONDS:EPOCH', '15:MINUTES'), count(*) AS \"bar\"," + - " not_equals(CASE WHEN equals(\"OriginCityName\", 'nyc') THEN 'pizza' WHEN equals(\"OriginCityName\", 'la') THEN 'burrito'" + - " WHEN equals(\"OriginCityName\", 'boston') THEN 'clam chowder' ELSE 'burger' END, 'salad')," + - " timeconvert(\"DaysSinceEpoch\", 'SECONDS', 'MINUTES') AS \"foo\"," + - " max(\"AirTime\") AS \"baz\"" + - " from %s" + - " group by datetimeconvert(\"DaysSinceEpoch\", '1:SECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '15:MINUTES')," + - " not_equals(CASE WHEN equals(\"OriginCityName\", 'nyc') THEN 'pizza' WHEN equals(\"OriginCityName\", 'la') THEN 'burrito' WHEN equals(\"OriginCityName\", 'boston') THEN 'clam chowder' ELSE 'burger' END, 'salad')," + - " timeconvert(\"DaysSinceEpoch\", 'SECONDS', 'MINUTES')" + - " limit 70", tableNameWithSuffix); + String expectedPql = """ + SELECT datetimeconvert("DaysSinceEpoch", '1:SECONDS:EPOCH',\ + '1:MILLISECONDS:EPOCH', '15:MINUTES'), count(*) AS "bar",\ + not_equals(CASE WHEN equals("OriginCityName", 'nyc') THEN 'pizza' WHEN equals("OriginCityName", 'la') THEN 'burrito'\ + WHEN equals("OriginCityName", 'boston') THEN 'clam chowder' ELSE 'burger' END, 'salad'),\ + timeconvert("DaysSinceEpoch", 'SECONDS', 'MINUTES') AS "foo",\ + max("AirTime") AS "baz"\ + FROM %s\ + GROUP BY datetimeconvert("DaysSinceEpoch", '1:SECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '15:MINUTES'),\ + not_equals(CASE WHEN equals("OriginCityName", 'nyc') THEN 'pizza' WHEN equals("OriginCityName", 'la') THEN 'burrito' WHEN equals("OriginCityName", 'boston') THEN 'clam chowder' ELSE 'burger' END, 'salad'),\ + timeconvert("DaysSinceEpoch", 'SECONDS', 'MINUTES')\ + LIMIT 70""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -331,9 +370,13 @@ public void testOrderBy() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select ArrDelay + 34 - DaysSinceEpoch, FlightNum from %s order by ArrDelay asc, DaysSinceEpoch desc", tableNameWithSuffix); + String query = "SELECT ArrDelay + 34 - DaysSinceEpoch, FlightNum FROM %s ORDER BY ArrDelay ASC, DaysSinceEpoch DESC".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select plus(\"ArrDelay\", '34') - \"DaysSinceEpoch\", \"FlightNum\" from %s order by \"ArrDelay\", \"DaysSinceEpoch\" desc limit 10", tableNameWithSuffix); + String expectedPql = """ + SELECT plus("ArrDelay", '34') - "DaysSinceEpoch", "FlightNum"\ + FROM %s\ + ORDER BY "ArrDelay", "DaysSinceEpoch" DESC\ + LIMIT 10""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -343,9 +386,13 @@ public void testOrderByCountStar() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select count(*) from %s order by count(*)", tableNameWithSuffix); + String query = "SELECT count(*) FROM %s ORDER BY count(*)".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select count(*) from %s order by count(*) limit 10", tableNameWithSuffix); + String expectedPql = """ + SELECT count(*)\ + FROM %s\ + ORDER BY count(*)\ + LIMIT 10""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -355,9 +402,13 @@ public void testOrderByExpression() { String tableName = hybridTable.getTableName(); String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select ArrDelay + 34 - DaysSinceEpoch, FlightNum from %s order by ArrDelay + 34 - DaysSinceEpoch desc", tableNameWithSuffix); + String query = "SELECT ArrDelay + 34 - DaysSinceEpoch, FlightNum FROM %s ORDER BY ArrDelay + 34 - DaysSinceEpoch desc".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select plus(\"ArrDelay\", '34') - \"DaysSinceEpoch\", \"FlightNum\" from %s order by plus(\"ArrDelay\", '34') - \"DaysSinceEpoch\" desc limit 10", tableNameWithSuffix); + String expectedPql = """ + SELECT plus("ArrDelay", '34') - "DaysSinceEpoch", "FlightNum"\ + FROM %s\ + ORDER BY plus("ArrDelay", '34') - "DaysSinceEpoch" DESC\ + LIMIT 10""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -367,9 +418,15 @@ public void testQuotesInAlias() { String tableName = "quotes_in_column_names"; String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select non_quoted AS \"non\"\"quoted\" from %s limit 50", tableNameWithSuffix); + String query = """ + SELECT non_quoted AS "non""quoted" + FROM %s + LIMIT 50""".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select \"non_quoted\" AS \"non\"\"quoted\" from %s limit 50", tableNameWithSuffix); + String expectedPql = """ + SELECT "non_quoted" AS "non""quoted"\ + FROM %s\ + LIMIT 50""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } @@ -379,9 +436,15 @@ public void testQuotesInColumnName() { String tableName = "quotes_in_column_names"; String tableNameWithSuffix = tableName + REALTIME_SUFFIX; - String query = format("select \"qu\"\"ot\"\"ed\" from %s limit 50", tableNameWithSuffix); + String query = """ + SELECT "qu""ot""ed" + FROM %s + LIMIT 50""".formatted(tableNameWithSuffix); DynamicTable dynamicTable = buildFromPql(pinotMetadata, new SchemaTableName("default", query), mockClusterInfoFetcher, TESTING_TYPE_CONVERTER); - String expectedPql = format("select \"qu\"\"ot\"\"ed\" from %s limit 50", tableNameWithSuffix); + String expectedPql = """ + SELECT "qu""ot""ed"\ + FROM %s\ + LIMIT 50""".formatted(tableNameWithSuffix); assertEquals(extractPql(dynamicTable, TupleDomain.all()), expectedPql); assertEquals(dynamicTable.getTableName(), tableName); } From 5adf9154bb2fc4556b57a292d45c124330e9b6d4 Mon Sep 17 00:00:00 2001 From: Elon Azoulay Date: Thu, 20 Apr 2023 09:38:44 -0700 Subject: [PATCH 2/2] Remove unnecessary throws clause in testMaxLimitForPassthroughQueries --- .../java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java b/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java index 7fe025643e9a..ed85a8594ca8 100644 --- a/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java +++ b/plugin/trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotConnectorSmokeTest.java @@ -1298,7 +1298,6 @@ public void testBrokerQueryWithTooManyRowsForSegmentQuery() @Test public void testMaxLimitForPassthroughQueries() - throws InterruptedException { assertQueryFails("SELECT string_col, updated_at_seconds" + " FROM \"SELECT updated_at_seconds, string_col FROM " + TOO_MANY_BROKER_ROWS_TABLE +