From 90c788e22ab7a64ddf1b6ea7662c234c487a3ab7 Mon Sep 17 00:00:00 2001 From: satish Date: Tue, 3 Oct 2023 18:53:09 +0530 Subject: [PATCH 01/12] ENG-35650: add support for map type in Trino handler --- .../DefaultColumnRequestConverter.java | 41 +++++++++---- .../QueryRequestToTrinoSQLConverterTest.java | 60 ++++--------------- 2 files changed, 40 insertions(+), 61 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java index d89c09b1..3eadb860 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java @@ -31,7 +31,7 @@ /** Converts {@link QueryRequest} to Trino SQL query */ class DefaultColumnRequestConverter implements ColumnRequestConverter { - + private static final String MAP_KEY_FUNCTION = "element_at"; private static final String QUESTION_MARK = "?"; private static final String REGEX_OPERATOR = "~*"; private static final int MAP_KEY_INDEX = 0; @@ -111,26 +111,35 @@ private String convertFilterToString( convertExpressionToString(filter.getRhs(), paramsBuilder, trinoExecutionContext)); break; case CONTAINS_KEY: + builder.append(MAP_KEY_FUNCTION); + builder.append("("); builder.append(lhs); - builder.append("->>"); + builder.append(", "); builder.append( convertLiteralToString( - convertMapKeyExpressionToLiterals(filter.getRhs()), paramsBuilder)); + convertMapKeyExpressionToLiteral(filter.getRhs()), paramsBuilder)); + builder.append(")"); builder.append(" IS NOT NULL"); break; case NOT_CONTAINS_KEY: + builder.append(MAP_KEY_FUNCTION); + builder.append("("); builder.append(lhs); - builder.append("->>"); + builder.append(", "); builder.append( convertLiteralToString( - convertMapKeyExpressionToLiterals(filter.getRhs()), paramsBuilder)); + convertMapKeyExpressionToLiteral(filter.getRhs()), paramsBuilder)); + builder.append(")"); builder.append(" IS NULL"); break; case CONTAINS_KEYVALUE: List kvp = convertMapKeyValueExpressionToLiterals(filter.getRhs()); + builder.append(MAP_KEY_FUNCTION); + builder.append("("); builder.append(lhs); - builder.append("->>"); + builder.append(", "); builder.append(convertLiteralToString(kvp.get(MAP_KEY_INDEX), paramsBuilder)); + builder.append(")"); builder.append(" = "); builder.append(convertLiteralToString(kvp.get(MAP_VALUE_INDEX), paramsBuilder)); break; @@ -349,7 +358,11 @@ private String convertExpressionToString( .setValue(Value.newBuilder().setString(pathExpression).build()) .build(); - return columnName + "->>" + convertLiteralToString(pathExpressionLiteral, paramsBuilder); + return String.format( + "%s(%s, %s)", + MAP_KEY_FUNCTION, + columnName, + convertLiteralToString(pathExpressionLiteral, paramsBuilder)); } case COLUMNIDENTIFIER: String logicalColumnName = @@ -380,17 +393,14 @@ private String convertExpressionToString( return ""; } - private LiteralConstant convertMapKeyExpressionToLiterals(Expression expression) { - List literals = new ArrayList<>(1); + private LiteralConstant convertMapKeyExpressionToLiteral(Expression expression) { if (expression.getValueCase() == LITERAL) { LiteralConstant value = expression.getLiteral(); if (value.getValue().getValueType() == ValueType.STRING) { - literals.add(value.getValue().getString()); - } else { - throw new IllegalArgumentException("Unsupported arguments for CONTAINS_KEY operator"); + return getLiteralConstant(value.getValue().getString()); } } - return getLiteralConstants(literals).get(0); + throw new IllegalArgumentException("Unsupported map key expression"); } private List convertMapKeyValueExpressionToLiterals(Expression expression) { @@ -421,6 +431,11 @@ private LiteralConstant convertMapLikeExpressionToLiterals(Expression expression return getLiteralConstants(literals).get(0); } + @NotNull + private LiteralConstant getLiteralConstant(String literal) { + return LiteralConstant.newBuilder().setValue(Value.newBuilder().setString(literal)).build(); + } + @NotNull private List getLiteralConstants(List literals) { return literals.stream() diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java index b3947e37..3c70daaa 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java @@ -13,7 +13,6 @@ import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNotEqualsFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNotInFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNullNumberLiteralValueExpression; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNullStringFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNullStringLiteralValueExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createOrderByExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringArrayLiteralValueExpression; @@ -541,7 +540,7 @@ void testQueryWithLikeOperator() { executionContext); } - // @Test + @Test void testQueryWithContainsKeyOperator() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.tags")); @@ -552,18 +551,18 @@ void testQueryWithContainsKeyOperator() { assertSQLQuery( builder.build(), - "SELECT cast(tags as text) FROM public.\"span-event-view\" " + "SELECT tags FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND tags->>'flags' IS NOT NULL", + + "AND element_at(tags, 'flags') IS NOT NULL", tableDefinition, executionContext); } - // @Test + @Test void testQueryWithNotContainsKeyOperator() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.tags")); @@ -574,18 +573,18 @@ void testQueryWithNotContainsKeyOperator() { assertSQLQuery( builder.build(), - "SELECT cast(tags as text) FROM public.\"span-event-view\" " + "SELECT tags FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND tags->>'flags' IS NULL", + + "AND element_at(tags, 'flags') IS NULL", tableDefinition, executionContext); } - // @Test + @Test void testQueryWithContainsKeyValueOperator() { Builder builder = QueryRequest.newBuilder(); Expression spanTag = createColumnExpression("Span.tags").build(); @@ -605,13 +604,13 @@ void testQueryWithContainsKeyValueOperator() { assertSQLQuery( builder.build(), - "SELECT cast(tags as text) FROM public.\"span-event-view\" " + "SELECT tags FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND tags->>'flags' = '0'", + + "AND element_at(tags, 'flags') = '0'", tableDefinition, executionContext); } @@ -647,7 +646,7 @@ void testQueryWithContainsKeyLikeOperator() { executionContext); } - // @Test + @Test void testQueryWithComplexKeyValueOperator() { Builder builder = QueryRequest.newBuilder(); Expression spanTag = createColumnExpression("Span.tags").build(); @@ -667,13 +666,13 @@ void testQueryWithComplexKeyValueOperator() { assertSQLQuery( builder.build(), - "SELECT cast(tags as text) FROM public.\"span-event-view\" " + "SELECT tags FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND tags->>'flags' = '0'", + + "AND element_at(tags, 'flags') = '0'", tableDefinition, executionContext); } @@ -1273,41 +1272,6 @@ private QueryRequest buildSimpleQueryWithFilter(Filter filter) { return builder.build(); } - private QueryRequest buildAvgRateQueryForOrderBy() { - Builder builder = QueryRequest.newBuilder(); - - Expression serviceId = createColumnExpression("SERVICE.id").build(); - Expression serviceName = createColumnExpression("SERVICE.name").build(); - Expression serviceErrorCount = createColumnExpression("SERVICE.errorCount").build(); - - Expression countFunction = createFunctionExpression("COUNT", serviceId); - Expression avgrateFunction = createFunctionExpression("AVGRATE", serviceErrorCount); - - Filter nullCheckFilter = createNullStringFilter("SERVICE.id", Operator.NEQ); - Filter startTimeFilter = createTimeFilter("SERVICE.startTime", Operator.GE, 1637297304041L); - Filter endTimeFilter = createTimeFilter("SERVICE.startTime", Operator.LT, 1637300904041L); - Filter andFilter = - Filter.newBuilder() - .setOperator(Operator.AND) - .addChildFilter(startTimeFilter) - .addChildFilter(endTimeFilter) - .addChildFilter(nullCheckFilter) - .build(); - builder.setFilter(andFilter); - - builder.addSelection(serviceId); - builder.addSelection(serviceName); - builder.addSelection(countFunction); - - builder.addGroupBy(serviceId); - builder.addGroupBy(serviceName); - - builder.addOrderBy(createOrderByExpression(avgrateFunction.toBuilder(), SortOrder.ASC)); - - builder.setLimit(10000); - return builder.build(); - } - private QueryRequest buildOrderByQuery() { Builder builder = QueryRequest.newBuilder(); Expression startTimeColumn = createColumnExpression("Span.start_time_millis").build(); From dae66fbe0aa276d280bd4904e52c4295a8f58281 Mon Sep 17 00:00:00 2001 From: satish Date: Wed, 4 Oct 2023 12:20:46 +0530 Subject: [PATCH 02/12] pinot-servicemanager use 0.6.6 --- .../core/query/service/htqueries/HTPinotQueriesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java index c257044c..300a5a7f 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java @@ -95,7 +95,7 @@ public static void setup() throws Exception { kafkaZk.start(); pinotServiceManager = - new GenericContainer<>(DockerImageName.parse("hypertrace/pinot-servicemanager:main")) + new GenericContainer<>(DockerImageName.parse("hypertrace/pinot-servicemanager:0.6.6")) .withNetwork(network) .withNetworkAliases("pinot-controller", "pinot-server", "pinot-broker") .withExposedPorts(8099, 9000) From e54a792ba155838b60dec49a6563f519db02bdfe Mon Sep 17 00:00:00 2001 From: satish Date: Wed, 4 Oct 2023 12:36:48 +0530 Subject: [PATCH 03/12] use hypertrace-view-generator:0.9.20 for integrationTest --- .../core/query/service/htqueries/HTPinotQueriesTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java index 300a5a7f..40dcd5ef 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java @@ -95,7 +95,7 @@ public static void setup() throws Exception { kafkaZk.start(); pinotServiceManager = - new GenericContainer<>(DockerImageName.parse("hypertrace/pinot-servicemanager:0.6.6")) + new GenericContainer<>(DockerImageName.parse("hypertrace/pinot-servicemanager:main")) .withNetwork(network) .withNetworkAliases("pinot-controller", "pinot-server", "pinot-broker") .withExposedPorts(8099, 9000) @@ -218,7 +218,7 @@ private static boolean bootstrapConfig() throws Exception { private static boolean generateData() throws Exception { // start view-gen service GenericContainer viewGen = - new GenericContainer(DockerImageName.parse("hypertrace/hypertrace-view-generator:main")) + new GenericContainer(DockerImageName.parse("hypertrace/hypertrace-view-generator:0.9.20")) .withNetwork(network) .dependsOn(kafkaZk) .withEnv("KAFKA_BOOTSTRAP_SERVERS", "kafka:9092") From d7fb9c17d0090b539377049b1ba463a2e2a4e82b Mon Sep 17 00:00:00 2001 From: satish Date: Wed, 4 Oct 2023 12:47:37 +0530 Subject: [PATCH 04/12] use hypertrace-view-generator:0.9.21 for integrationTest --- .../core/query/service/htqueries/HTPinotQueriesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java index 40dcd5ef..1b4100b4 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java @@ -218,7 +218,7 @@ private static boolean bootstrapConfig() throws Exception { private static boolean generateData() throws Exception { // start view-gen service GenericContainer viewGen = - new GenericContainer(DockerImageName.parse("hypertrace/hypertrace-view-generator:0.9.20")) + new GenericContainer(DockerImageName.parse("hypertrace/hypertrace-view-generator:0.9.21")) .withNetwork(network) .dependsOn(kafkaZk) .withEnv("KAFKA_BOOTSTRAP_SERVERS", "kafka:9092") From 8f35ae11825f85cfdd104244dcf1f029b1eff257 Mon Sep 17 00:00:00 2001 From: satish Date: Wed, 4 Oct 2023 14:30:18 +0530 Subject: [PATCH 05/12] fix typo --- helm/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/values.yaml b/helm/values.yaml index 50483b79..3d776dab 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -81,7 +81,7 @@ queryServiceConfig: postgresqlPasswordSecretKey: password postgresMaxConnectionAttempts: 200 postgresConnectionRetryBackoff: 5s - trinogresConnectionString: jdbc:trino://localhost:8080/iceberg/iceberg_gcs + trinoConnectionString: jdbc:trino://localhost:8080/iceberg/iceberg_gcs trinoUser: trino trinoPasswordEnvVariable: TRINO_PASSWORD trinoPasswordSecretName: trino From ef33caf55cce0395eb71e87e0915805d8a7b5224 Mon Sep 17 00:00:00 2001 From: satish Date: Wed, 4 Oct 2023 17:56:13 +0530 Subject: [PATCH 06/12] use hypertrace/hypertrace-view-generator:main in integrationTest --- .../core/query/service/htqueries/HTPinotQueriesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java index 1b4100b4..c257044c 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java @@ -218,7 +218,7 @@ private static boolean bootstrapConfig() throws Exception { private static boolean generateData() throws Exception { // start view-gen service GenericContainer viewGen = - new GenericContainer(DockerImageName.parse("hypertrace/hypertrace-view-generator:0.9.21")) + new GenericContainer(DockerImageName.parse("hypertrace/hypertrace-view-generator:main")) .withNetwork(network) .dependsOn(kafkaZk) .withEnv("KAFKA_BOOTSTRAP_SERVERS", "kafka:9092") From 6374ab7045436fc79b9e1919df5ea643f3a258cc Mon Sep 17 00:00:00 2001 From: satish Date: Wed, 4 Oct 2023 18:35:34 +0530 Subject: [PATCH 07/12] fix vulnerabilities --- owasp-suppressions.xml | 6 +++--- query-service-impl/build.gradle.kts | 8 ++++---- query-service/build.gradle.kts | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/owasp-suppressions.xml b/owasp-suppressions.xml index 0c07ab6d..c4e0a07f 100644 --- a/owasp-suppressions.xml +++ b/owasp-suppressions.xml @@ -18,7 +18,7 @@ CVE-2020-13956 - + @@ -27,7 +27,7 @@ CVE-2018-8012 CVE-2019-0201 - + ^pkg:maven/com\.fasterxml\.jackson\.core/jackson\-databind@.*$ CVE-2023-35116 - + diff --git a/query-service-impl/build.gradle.kts b/query-service-impl/build.gradle.kts index b7025cf0..316ae9e0 100644 --- a/query-service-impl/build.gradle.kts +++ b/query-service-impl/build.gradle.kts @@ -25,8 +25,8 @@ dependencies { implementation("org.apache.calcite:calcite-babel:1.34.0") { because("CVE-2022-39135") } - implementation("org.apache.avro:avro:1.11.1") { - because("CVE-2021-43045") + implementation("org.apache.avro:avro:1.11.3") { + because("CVE-2023-39410") } implementation("org.apache.helix:helix-core:1.3.0") { because("CVE-2022-47500") @@ -37,8 +37,8 @@ dependencies { implementation("net.minidev:json-smart:2.4.11") { because("CVE-2023-1370") } - implementation("org.xerial.snappy:snappy-java:1.1.10.1") { - because("CVE-2023-34453, CVE-2023-34454, CVE-2023-34455") + implementation("org.xerial.snappy:snappy-java:1.1.10.5") { + because("CVE-2023-43642") } implementation("org.yaml:snakeyaml:2.0") { because("CVE-2022-1471") diff --git a/query-service/build.gradle.kts b/query-service/build.gradle.kts index a54c7e16..c8d77f96 100644 --- a/query-service/build.gradle.kts +++ b/query-service/build.gradle.kts @@ -27,7 +27,7 @@ dependencies { integrationTestImplementation("org.apache.kafka:kafka-clients:7.2.1-ccs") integrationTestImplementation("org.apache.kafka:kafka-streams:7.2.1-ccs") - integrationTestImplementation("org.apache.avro:avro:1.11.1") + integrationTestImplementation("org.apache.avro:avro:1.11.3") integrationTestImplementation("com.google.guava:guava:32.1.2-jre") integrationTestImplementation("org.hypertrace.core.datamodel:data-model:0.1.12") integrationTestImplementation("org.hypertrace.core.kafkastreams.framework:kafka-streams-serdes:0.3.2") From 8224d1b92a86c5abf621f0e5167efe458584374d Mon Sep 17 00:00:00 2001 From: satish Date: Wed, 4 Oct 2023 18:51:56 +0530 Subject: [PATCH 08/12] fix vulnerability --- query-service-impl/build.gradle.kts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/query-service-impl/build.gradle.kts b/query-service-impl/build.gradle.kts index 316ae9e0..7b1d0e2b 100644 --- a/query-service-impl/build.gradle.kts +++ b/query-service-impl/build.gradle.kts @@ -28,6 +28,9 @@ dependencies { implementation("org.apache.avro:avro:1.11.3") { because("CVE-2023-39410") } + implementation("org.apache.commons:commons-compress:1.24.0") { + because("CVE-2023-42503") + } implementation("org.apache.helix:helix-core:1.3.0") { because("CVE-2022-47500") } From 1904836f2699b140737a2463a1e24a30739f690f Mon Sep 17 00:00:00 2001 From: satish Date: Thu, 5 Oct 2023 12:07:23 +0530 Subject: [PATCH 09/12] rename variable --- .../converters/DefaultColumnRequestConverter.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java index 3eadb860..120fedb7 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java @@ -31,7 +31,7 @@ /** Converts {@link QueryRequest} to Trino SQL query */ class DefaultColumnRequestConverter implements ColumnRequestConverter { - private static final String MAP_KEY_FUNCTION = "element_at"; + private static final String MAP_KEY_OPERATOR = "element_at"; private static final String QUESTION_MARK = "?"; private static final String REGEX_OPERATOR = "~*"; private static final int MAP_KEY_INDEX = 0; @@ -111,7 +111,7 @@ private String convertFilterToString( convertExpressionToString(filter.getRhs(), paramsBuilder, trinoExecutionContext)); break; case CONTAINS_KEY: - builder.append(MAP_KEY_FUNCTION); + builder.append(operator); builder.append("("); builder.append(lhs); builder.append(", "); @@ -122,7 +122,7 @@ private String convertFilterToString( builder.append(" IS NOT NULL"); break; case NOT_CONTAINS_KEY: - builder.append(MAP_KEY_FUNCTION); + builder.append(operator); builder.append("("); builder.append(lhs); builder.append(", "); @@ -134,7 +134,7 @@ private String convertFilterToString( break; case CONTAINS_KEYVALUE: List kvp = convertMapKeyValueExpressionToLiterals(filter.getRhs()); - builder.append(MAP_KEY_FUNCTION); + builder.append(operator); builder.append("("); builder.append(lhs); builder.append(", "); @@ -335,7 +335,7 @@ private String convertOperatorToString(Operator operator) { case CONTAINS_KEY: case NOT_CONTAINS_KEY: case CONTAINS_KEYVALUE: - return ""; + return MAP_KEY_OPERATOR; case RANGE: throw new UnsupportedOperationException("RANGE NOT supported use >= and <="); case UNRECOGNIZED: @@ -360,7 +360,7 @@ private String convertExpressionToString( return String.format( "%s(%s, %s)", - MAP_KEY_FUNCTION, + MAP_KEY_OPERATOR, columnName, convertLiteralToString(pathExpressionLiteral, paramsBuilder)); } From 7dcb200098dc85f1f147fb3ea2fb1ac465882aa6 Mon Sep 17 00:00:00 2001 From: satish Date: Thu, 5 Oct 2023 13:41:44 +0530 Subject: [PATCH 10/12] add support for like and contains_key_like operator --- .../DefaultColumnRequestConverter.java | 48 +++++++------------ .../QueryRequestToTrinoSQLConverterTest.java | 14 +++--- 2 files changed, 25 insertions(+), 37 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java index 120fedb7..024217a4 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java @@ -33,7 +33,7 @@ class DefaultColumnRequestConverter implements ColumnRequestConverter { private static final String MAP_KEY_OPERATOR = "element_at"; private static final String QUESTION_MARK = "?"; - private static final String REGEX_OPERATOR = "~*"; + private static final String REGEX_OPERATOR = "regexp_like"; private static final int MAP_KEY_INDEX = 0; private static final int MAP_VALUE_INDEX = 1; @@ -103,12 +103,13 @@ private String convertFilterToString( String lhs = convertExpressionToString(filter.getLhs(), paramsBuilder, trinoExecutionContext); switch (filter.getOperator()) { case LIKE: - builder.append(lhs); - builder.append(" "); builder.append(operator); - builder.append(" "); + builder.append("("); + builder.append(lhs); + builder.append(", "); builder.append( convertExpressionToString(filter.getRhs(), paramsBuilder, trinoExecutionContext)); + builder.append(")"); break; case CONTAINS_KEY: builder.append(operator); @@ -144,14 +145,19 @@ private String convertFilterToString( builder.append(convertLiteralToString(kvp.get(MAP_VALUE_INDEX), paramsBuilder)); break; case CONTAINS_KEY_LIKE: + // any_match(map_keys(tags), k -> regexp_like(k, pattern)) + builder.append("any_match"); + builder.append("("); + builder.append("map_keys"); + builder.append("("); builder.append(lhs); - builder.append("::jsonb::text"); - builder.append(" "); - builder.append(operator); - builder.append(" "); + builder.append(")"); + builder.append(", k -> regexp_like(k, "); builder.append( convertLiteralToString( - convertMapLikeExpressionToLiterals(filter.getRhs()), paramsBuilder)); + convertMapKeyExpressionToLiteral(filter.getRhs()), paramsBuilder)); + builder.append(")"); + builder.append(")"); break; default: if (isFilterForBytesColumnType(filter, trinoExecutionContext)) { @@ -330,8 +336,9 @@ private String convertOperatorToString(Operator operator) { case LE: return "<="; case LIKE: - case CONTAINS_KEY_LIKE: return REGEX_OPERATOR; + case CONTAINS_KEY_LIKE: + return ""; case CONTAINS_KEY: case NOT_CONTAINS_KEY: case CONTAINS_KEYVALUE: @@ -418,19 +425,6 @@ private List convertMapKeyValueExpressionToLiterals(Expression return getLiteralConstants(literals); } - private LiteralConstant convertMapLikeExpressionToLiterals(Expression expression) { - List literals = new ArrayList<>(1); - if (expression.getValueCase() == LITERAL) { - LiteralConstant value = expression.getLiteral(); - if (value.getValue().getValueType() == ValueType.STRING) { - literals.add(".*\"" + value.getValue().getString() + "\":.*"); - } else { - throw new IllegalArgumentException("Unsupported arguments for CONTAINS_KEY_LIKE operator"); - } - } - return getLiteralConstants(literals).get(0); - } - @NotNull private LiteralConstant getLiteralConstant(String literal) { return LiteralConstant.newBuilder().setValue(Value.newBuilder().setString(literal)).build(); @@ -438,13 +432,7 @@ private LiteralConstant getLiteralConstant(String literal) { @NotNull private List getLiteralConstants(List literals) { - return literals.stream() - .map( - literal -> - LiteralConstant.newBuilder() - .setValue(Value.newBuilder().setString(literal)) - .build()) - .collect(Collectors.toUnmodifiableList()); + return literals.stream().map(this::getLiteralConstant).collect(Collectors.toUnmodifiableList()); } /** TODO:Handle all types */ diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java index 3c70daaa..c286aaa2 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java @@ -510,7 +510,7 @@ void testSQLiWithStringArrayFilter() { executionContext); } - // @Test + @Test void testQueryWithLikeOperator() { Builder builder = QueryRequest.newBuilder(); Expression spanId = createColumnExpression("Span.displaySpanName").build(); @@ -520,7 +520,7 @@ void testQueryWithLikeOperator() { Filter.newBuilder() .setOperator(Operator.LIKE) .setLhs(spanId) - .setRhs(createStringLiteralValueExpression("%test%")) + .setRhs(createStringLiteralValueExpression("order")) .build(); builder.setFilter(likeFilter); @@ -529,13 +529,13 @@ void testQueryWithLikeOperator() { assertSQLQuery( builder.build(), - "SELECT span_name FROM public.\"span-event-view\" " + "SELECT span_name FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND span_name ~* '%test%'", + + "AND regexp_like(span_name, 'order')", tableDefinition, executionContext); } @@ -615,7 +615,7 @@ void testQueryWithContainsKeyValueOperator() { executionContext); } - // @Test + @Test void testQueryWithContainsKeyLikeOperator() { Builder builder = QueryRequest.newBuilder(); Expression spanTag = createColumnExpression("Span.tags").build(); @@ -635,13 +635,13 @@ void testQueryWithContainsKeyLikeOperator() { assertSQLQuery( builder.build(), - "SELECT cast(tags as text) FROM public.\"span-event-view\" " + "SELECT tags FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND tags::jsonb::text ~* '.*\"my_tag_name.*\":.*'", + + "AND any_match(map_keys(tags), k -> regexp_like(k, 'my_tag_name.*'))", tableDefinition, executionContext); } From f2b6f9ad02e56dad601fdfb610e33c13d69a2288 Mon Sep 17 00:00:00 2001 From: satish Date: Tue, 10 Oct 2023 14:33:48 +0530 Subject: [PATCH 11/12] handle array columns in Trino handler --- .../DefaultColumnRequestConverter.java | 69 ++++++++++------ .../QueryRequestToTrinoSQLConverterTest.java | 82 +++++++++---------- .../test/resources/trino_request_handler.conf | 4 +- 3 files changed, 89 insertions(+), 66 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java index 561a79c3..b30ef3a5 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/trino/converters/DefaultColumnRequestConverter.java @@ -243,37 +243,60 @@ private void handleConversionForArrayColumnExpression( if (handleConversionForNullOrEmptyArrayLiteral(lhs, operator, builder, value)) return; - // support only equals and IN operator - // both of them are handled as contains check to align with existing implementation - if (operator.equals("=") - || operator.equals("IN") - || operator.equals("!=") - || operator.equals("NOT IN")) { - // add NOT operator to negate the match condition - if (operator.equals("!=") || operator.equals("NOT IN")) { + // Support only equals and IN operator + if (operator.equals("=") || operator.equals("!=")) { + // Equals (=): CONTAINS(ip_types, 'Bot') + // Not equals (!=): NOT CONTAINS(ip_types, 'Bot') + if (operator.equals("!=")) { builder.append("NOT "); } + builder.append("CONTAINS("); builder.append(lhs); - // overlap operator for array - builder.append(" && "); + builder.append(", "); + if (value.getValueType() == ValueType.STRING) { + builder.append(QUESTION_MARK); + paramsBuilder.addStringParam(value.getString()); + } else { + throw new IllegalArgumentException( + String.format("Unsupported value {%s} for array column", value)); + } + builder.append(")"); + } else if (operator.equals("IN") || operator.equals("NOT IN")) { + // IN: CARDINALITY(ARRAY_INTERSECT(ip_types, ARRAY['Public Proxy', 'Bot'])) > 0 + // NOT IN: CARDINALITY(ARRAY_INTERSECT(ip_types, ARRAY['Public Proxy', 'Bot'])) = 0 + builder.append("CARDINALITY("); + builder.append("ARRAY_INTERSECT("); + builder.append(lhs); + builder.append(", ARRAY["); + switch (value.getValueType()) { + case STRING: + builder.append(QUESTION_MARK); + paramsBuilder.addStringParam(value.getString()); + break; + case STRING_ARRAY: + String delim = ""; + for (String item : value.getStringArrayList()) { + builder.append(delim); + builder.append(QUESTION_MARK); + paramsBuilder.addStringParam(item); + delim = ", "; + } + break; + default: + throw new IllegalArgumentException( + String.format("Unsupported value {%s} for array column", value)); + } + builder.append("]))"); + if (operator.equals("IN")) { + builder.append(" > 0"); + } else { + builder.append(" = 0"); + } } else { throw new IllegalArgumentException( String.format( "Unsupported operator {%s} for array column with non-empty value", operator)); } - builder.append("?"); - switch (value.getValueType()) { - case STRING: - paramsBuilder.addStringParam("{" + value.getString() + "}"); - break; - case STRING_ARRAY: - paramsBuilder.addStringParam("{" + String.join(", ", value.getStringArrayList()) + "}"); - break; - default: - throw new IllegalArgumentException( - String.format("Unsupported value {%s} for array column", value)); - } - builder.append(QUESTION_MARK); } private boolean handleConversionForNullOrEmptyArrayLiteral( diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java index c286aaa2..2b530a08 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/trino/QueryRequestToTrinoSQLConverterTest.java @@ -175,7 +175,7 @@ void testQueryWithStringFilter() { executionContext); } - // @Test + @Test void testSQLiWithStringValueFilter() { QueryRequest queryRequest = buildSimpleQueryWithFilter( @@ -185,7 +185,7 @@ void testSQLiWithStringValueFilter() { assertSQLQuery( queryRequest, - "Select encode(span_id, 'hex') FROM public.\"span-event-view\" WHERE " + "Select lower(to_hex(span_id)) FROM span-event-view WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID @@ -217,7 +217,7 @@ void testQueryWithBooleanFilter() { executionContext); } - // @Test + @Test void testQueryWithDoubleFilter() { QueryRequest queryRequest = buildSimpleQueryWithFilter(createEqualsFilter("Span.metrics.duration_millis", 1.2)); @@ -226,7 +226,7 @@ void testQueryWithDoubleFilter() { assertSQLQuery( queryRequest, - "Select encode(span_id, 'hex') FROM public.\"span-event-view\" WHERE " + "Select lower(to_hex(span_id)) FROM span-event-view WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID @@ -294,14 +294,14 @@ void testQueryWithTimestampFilter() { executionContext); } - // @Test + @Test void testQueryWithOrderBy() { TableDefinition tableDefinition = getDefaultTableDefinition(); defaultMockingForExecutionContext(); assertSQLQuery( buildOrderByQuery(), - "Select encode(span_id, 'hex'), start_time_millis, end_time_millis FROM public.\"span-event-view\" WHERE " + "Select lower(to_hex(span_id)), start_time_millis, end_time_millis FROM span-event-view WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID @@ -311,7 +311,7 @@ void testQueryWithOrderBy() { executionContext); } - // @Test + @Test void testQueryWithOrderByWithPagination() { QueryRequest orderByQueryRequest = buildOrderByQuery(); Builder builder = QueryRequest.newBuilder(orderByQueryRequest); @@ -321,7 +321,7 @@ void testQueryWithOrderByWithPagination() { assertSQLQuery( builder.build(), - "Select encode(span_id, 'hex'), start_time_millis, end_time_millis FROM public.\"span-event-view\" WHERE " + "Select lower(to_hex(span_id)), start_time_millis, end_time_millis FROM span-event-view WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID @@ -331,7 +331,7 @@ void testQueryWithOrderByWithPagination() { executionContext); } - // @Test + @Test void testQueryWithGroupByWithMultipleAggregates() { QueryRequest groupByQueryRequest = buildMultipleGroupByMultipleAggQuery(); Builder builder = QueryRequest.newBuilder(groupByQueryRequest); @@ -341,7 +341,7 @@ void testQueryWithGroupByWithMultipleAggregates() { assertSQLQuery( builder.build(), - "select count(*), avg(duration_millis) FROM public.\"span-event-view\"" + "select count(*), avg(duration_millis) FROM span-event-view" + " where " + tableDefinition.getTenantIdColumn() + " = '" @@ -353,7 +353,7 @@ void testQueryWithGroupByWithMultipleAggregates() { executionContext); } - // @Test + @Test void testQueryWithGroupByWithMultipleAggregatesAndOrderBy() { QueryRequest orderByQueryRequest = buildMultipleGroupByMultipleAggAndOrderByQuery(); Builder builder = QueryRequest.newBuilder(orderByQueryRequest); @@ -363,7 +363,7 @@ void testQueryWithGroupByWithMultipleAggregatesAndOrderBy() { assertSQLQuery( builder.build(), - "select count(*), avg(duration_millis) FROM public.\"span-event-view\"" + "select count(*), avg(duration_millis) FROM span-event-view" + " where " + tableDefinition.getTenantIdColumn() + " = '" @@ -375,7 +375,7 @@ void testQueryWithGroupByWithMultipleAggregatesAndOrderBy() { executionContext); } - // @Test + @Test void testQueryWithDistinctCountAggregation() { Filter startTimeFilter = createTimeFilter("Span.start_time_millis", Operator.GT, 1570658506605L); @@ -399,7 +399,7 @@ void testQueryWithDistinctCountAggregation() { assertSQLQuery( queryRequest, - "select count(distinct encode(span_id, 'hex')) FROM public.\"span-event-view\"" + "select count(distinct lower(to_hex(span_id))) FROM span-event-view" + " where " + tableDefinition.getTenantIdColumn() + " = '" @@ -411,7 +411,7 @@ void testQueryWithDistinctCountAggregation() { executionContext); } - // @Test + @Test void testQueryWithDistinctCountAggregationAndGroupBy() { Filter startTimeFilter = createTimeFilter("Span.start_time_millis", Operator.GT, 1570658506605L); @@ -442,14 +442,14 @@ void testQueryWithDistinctCountAggregationAndGroupBy() { assertSQLQuery( queryRequest, - "select encode(span_id, 'hex'), count(distinct encode(span_id, 'hex')) FROM public.\"span-event-view\"" + "select lower(to_hex(span_id)), count(distinct lower(to_hex(span_id))) FROM span-event-view" + " where " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " + "and ( start_time_millis > 1570658506605 and end_time_millis < 1570744906673 )" - + " group by span_id order by count(distinct span_id) limit 15", + + " group by 1 order by 2 limit 15", tableDefinition, executionContext); } @@ -485,7 +485,7 @@ void testQueryWithStringArray() { executionContext); } - // @Test + @Test void testSQLiWithStringArrayFilter() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.displaySpanName")); @@ -500,7 +500,7 @@ void testSQLiWithStringArrayFilter() { assertSQLQuery( builder.build(), - "SELECT span_name FROM public.\"span-event-view\" WHERE " + "SELECT span_name FROM span-event-view WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID @@ -805,12 +805,12 @@ void testQueryWithBytesColumnInFilter() { executionContext); } - // @Test + @Test void testQueryWithArrayColumnWithValidValue() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.id").build()); - Filter filter = createEqualsFilter("Span.labels", "label1"); + Filter filter = createEqualsFilter("Span.ip_types", "Bot"); builder.setFilter(filter); builder.setLimit(5); @@ -820,13 +820,13 @@ void testQueryWithArrayColumnWithValidValue() { assertSQLQuery( request, - "SELECT encode(span_id, 'hex') FROM public.\"span-event-view\" " + "SELECT lower(to_hex(span_id)) FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND labels && '{label1}' limit 5", + + "AND CONTAINS(ip_types, 'Bot') limit 5", tableDefinition, executionContext); } @@ -883,12 +883,12 @@ void testQueryWithArrayColumnWithEmptyValue() { executionContext); } - // @Test + @Test void testQueryWithArrayColumnNotEqualsFilter() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.id").build()); - Filter filter = createNotEqualsFilter("Span.labels", "label1"); + Filter filter = createNotEqualsFilter("Span.ip_types", "Bot"); builder.setFilter(filter); builder.setLimit(5); @@ -898,23 +898,23 @@ void testQueryWithArrayColumnNotEqualsFilter() { assertSQLQuery( request, - "SELECT encode(span_id, 'hex') FROM public.\"span-event-view\" " + "SELECT lower(to_hex(span_id)) FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND NOT labels && '{label1}' limit 5", + + "AND NOT CONTAINS(ip_types, 'Bot') limit 5", tableDefinition, executionContext); } - // @Test + @Test void testQueryWithArrayColumnInFilter() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.id").build()); - Filter filter = createInFilter("Span.labels", List.of("label1", "label2")); + Filter filter = createInFilter("Span.ip_types", List.of("Public Proxy", "Bot")); builder.setFilter(filter); builder.setLimit(5); @@ -924,23 +924,23 @@ void testQueryWithArrayColumnInFilter() { assertSQLQuery( request, - "SELECT encode(span_id, 'hex') FROM public.\"span-event-view\" " + "SELECT lower(to_hex(span_id)) FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND labels && '{label1, label2}' limit 5", + + "AND CARDINALITY(ARRAY_INTERSECT(ip_types, ARRAY['Public Proxy', 'Bot'])) > 0 limit 5", tableDefinition, executionContext); } - // @Test + @Test void testQueryWithArrayColumnNotInFilter() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.id").build()); - Filter filter = createNotInFilter("Span.labels", List.of("label1", "label2")); + Filter filter = createNotInFilter("Span.ip_types", List.of("Public Proxy", "Bot")); builder.setFilter(filter); builder.setLimit(5); @@ -950,18 +950,18 @@ void testQueryWithArrayColumnNotInFilter() { assertSQLQuery( request, - "SELECT encode(span_id, 'hex') FROM public.\"span-event-view\" " + "SELECT lower(to_hex(span_id)) FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND NOT labels && '{label1, label2}' limit 5", + + "AND CARDINALITY(ARRAY_INTERSECT(ip_types, ARRAY['Public Proxy', 'Bot'])) = 0 limit 5", tableDefinition, executionContext); } - // @Test + @Test void testQueryWithStringColumnWithNullString() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.id")); @@ -978,18 +978,18 @@ void testQueryWithStringColumnWithNullString() { assertSQLQuery( request, - "SELECT encode(span_id, 'hex') FROM public.\"span-event-view\" " + "SELECT lower(to_hex(span_id)) FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND ( span_id != '' ) limit 5", + + "AND ( lower(to_hex(span_id)) != '' ) limit 5", tableDefinition, executionContext); } - // @Test + @Test void testQueryWithLongColumn() { Builder builder = QueryRequest.newBuilder(); builder.addSelection(createColumnExpression("Span.id")); @@ -1010,7 +1010,7 @@ void testQueryWithLongColumn() { assertSQLQuery( request, - "SELECT encode(span_id, 'hex') FROM public.\"span-event-view\" " + "SELECT lower(to_hex(span_id)) FROM span-event-view " + "WHERE " + tableDefinition.getTenantIdColumn() + " = '" @@ -1342,7 +1342,7 @@ private QueryRequest buildMultipleGroupByMultipleAggAndOrderByQuery() { SortOrder.DESC)); builder.addOrderBy( createOrderByExpression( - createAliasedFunctionExpression("COUNT", "Span.id", "count_encode(span_id, 'hex')"), + createAliasedFunctionExpression("COUNT", "Span.id", "count_lower(to_hex(span_id))"), SortOrder.DESC)); return builder.build(); } diff --git a/query-service-impl/src/test/resources/trino_request_handler.conf b/query-service-impl/src/test/resources/trino_request_handler.conf index 146edf65..111b1c9f 100644 --- a/query-service-impl/src/test/resources/trino_request_handler.conf +++ b/query-service-impl/src/test/resources/trino_request_handler.conf @@ -7,7 +7,7 @@ tableName = span-event-view mapFields = ["tags", "request_headers"] bytesFields = ["parent_span_id", "span_id"] - arrayFields = ["labels"] + arrayFields = ["ip_types"] tdigestFields = ["response_time_millis_tdigest"] fieldMap = { "Span.tags": "tags", @@ -28,7 +28,7 @@ "Span.metrics.duration_millis": "duration_millis", "Span.serviceName": "service_name", "Span.attributes.parent_span_id": "parent_span_id", - "Span.labels": "labels", + "Span.ip_types": "ip_types", "Span.user_latitude": "user_latitude", } } From 38cba44a42d21240839194b11d6074f46e2cf84b Mon Sep 17 00:00:00 2001 From: satish Date: Tue, 17 Oct 2023 16:43:25 +0530 Subject: [PATCH 12/12] fix vuln --- query-service-impl/build.gradle.kts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/query-service-impl/build.gradle.kts b/query-service-impl/build.gradle.kts index 7b1d0e2b..d227935a 100644 --- a/query-service-impl/build.gradle.kts +++ b/query-service-impl/build.gradle.kts @@ -34,6 +34,9 @@ dependencies { implementation("org.apache.helix:helix-core:1.3.0") { because("CVE-2022-47500") } + implementation("org.apache.zookeeper:zookeeper:3.7.2") { + because("CVE-2023-44981") + } implementation("org.webjars:swagger-ui:5.1.0") { because("CVE-2019-16728,CVE-2020-26870") }