diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java index 5cb3ba73..04506568 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java @@ -1,11 +1,16 @@ package org.hypertrace.core.query.service; import static org.hypertrace.core.query.service.api.Expression.ValueCase.ATTRIBUTE_EXPRESSION; +import static org.hypertrace.core.query.service.api.Expression.ValueCase.COLUMNIDENTIFIER; +import java.util.List; +import org.hypertrace.core.query.service.api.AttributeExpression; import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Expression.ValueCase; +import org.hypertrace.core.query.service.api.Filter; import org.hypertrace.core.query.service.api.LiteralConstant; +import org.hypertrace.core.query.service.api.Operator; import org.hypertrace.core.query.service.api.Value; import org.hypertrace.core.query.service.api.ValueType; @@ -72,29 +77,65 @@ public static Expression createNullNumberLiteralExpression() { .build(); } - public static boolean isDateTimeFunction(Expression expression) { - return expression.getValueCase() == ValueCase.FUNCTION - && expression.getFunction().getFunctionName().equals("dateTimeConvert"); + public static Filter createContainsKeyFilter(AttributeExpression complexAttributeExpression) { + return createContainsKeyFilter( + complexAttributeExpression.getAttributeId(), complexAttributeExpression.getSubpath()); } - public static boolean isComplexAttribute(Expression expression) { - return expression.getValueCase().equals(ATTRIBUTE_EXPRESSION) - && expression.getAttributeExpression().hasSubpath(); + public static Filter createContainsKeyFilter(String column, String value) { + return Filter.newBuilder() + .setOperator(Operator.CONTAINS_KEY) + .setLhs(createColumnExpression(column)) + .setRhs(createStringArrayLiteralValueExpression(List.of(value))) + .build(); } - public static boolean isSimpleColumnExpression(Expression expression) { - return expression.getValueCase() == ValueCase.COLUMNIDENTIFIER - || (expression.getValueCase() == ATTRIBUTE_EXPRESSION && !isComplexAttribute(expression)); + public static Expression createStringArrayLiteralValueExpression(List values) { + return Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder() + .addAllStringArray(values) + .setValueType(ValueType.STRING_ARRAY))) + .build(); } - public static String getLogicalColumnNameForSimpleColumnExpression(Expression expression) { - if (!isSimpleColumnExpression(expression)) { - throw new RuntimeException("Expecting expression of type COLUMN or ATTRIBUTE"); + // attribute expression with sub path is always a map attribute + public static boolean isAttributeExpressionWithSubpath(Expression expression) { + return expression.getValueCase() == ATTRIBUTE_EXPRESSION + && expression.getAttributeExpression().hasSubpath(); + } + + public static boolean isSimpleAttributeExpression(Expression expression) { + switch (expression.getValueCase()) { + case COLUMNIDENTIFIER: + return true; + case ATTRIBUTE_EXPRESSION: + return !expression.getAttributeExpression().hasSubpath(); + default: + return false; } - if (expression.getValueCase() == ValueCase.COLUMNIDENTIFIER) { - return expression.getColumnIdentifier().getColumnName(); - } else { - return expression.getAttributeExpression().getAttributeId(); + } + + public static boolean isDateTimeFunction(Expression expression) { + return expression.getValueCase() == ValueCase.FUNCTION + && expression.getFunction().getFunctionName().equals("dateTimeConvert"); + } + + public static String getLogicalColumnName(Expression expression) { + switch (expression.getValueCase()) { + case COLUMNIDENTIFIER: + return expression.getColumnIdentifier().getColumnName(); + case ATTRIBUTE_EXPRESSION: + return expression.getAttributeExpression().getAttributeId(); + default: + throw new IllegalArgumentException( + "Supports " + + ATTRIBUTE_EXPRESSION + + " and " + + COLUMNIDENTIFIER + + " expression type only"); } } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java index 52b3237b..278ed173 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java @@ -34,6 +34,7 @@ import org.hypertrace.core.query.service.api.Filter; import org.hypertrace.core.query.service.api.LiteralConstant; import org.hypertrace.core.query.service.api.Operator; +import org.hypertrace.core.query.service.api.OrderByExpression; import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.Row; import org.hypertrace.core.query.service.api.Row.Builder; @@ -621,5 +622,46 @@ private void validateQueryRequest(ExecutionContext executionContext, QueryReques noGroupBy && noAggregations, "If distinct selections are requested, there should be no groupBys or aggregations."); } + + // Validate attribute expressions + + validateAttributeExpressionFilter(request.getFilter()); + + for (Expression expression : executionContext.getAllSelections()) { + if (isInvalidExpression(expression)) { + throw new IllegalArgumentException("Invalid Query"); + } + } + + for (Expression expression : request.getGroupByList()) { + if (isInvalidExpression(expression)) { + throw new IllegalArgumentException("Invalid Query"); + } + } + + for (OrderByExpression orderByExpression : request.getOrderByList()) { + if (isInvalidExpression(orderByExpression.getExpression())) { + throw new IllegalArgumentException("Invalid Query"); + } + } + } + + private void validateAttributeExpressionFilter(Filter filter) { + if (filter.getChildFilterCount() > 0) { + for (Filter childFilter : filter.getChildFilterList()) { + validateAttributeExpressionFilter(childFilter); + } + } else { + if (isInvalidExpression(filter.getLhs())) { + throw new IllegalArgumentException("Invalid Query"); + } + } + } + + private boolean isInvalidExpression(Expression expression) { + return expression.getValueCase() == ValueCase.ATTRIBUTE_EXPRESSION + && expression.getAttributeExpression().hasSubpath() + && viewDefinition.getColumnType(expression.getAttributeExpression().getAttributeId()) + != ValueType.STRING_MAP; } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java index 1180f6d0..8c0adb80 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java @@ -1,7 +1,8 @@ package org.hypertrace.core.query.service.pinot; -import static org.hypertrace.core.query.service.QueryRequestUtil.isComplexAttribute; -import static org.hypertrace.core.query.service.api.Expression.ValueCase.ATTRIBUTE_EXPRESSION; +import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnName; +import static org.hypertrace.core.query.service.QueryRequestUtil.isAttributeExpressionWithSubpath; +import static org.hypertrace.core.query.service.QueryRequestUtil.isSimpleAttributeExpression; import static org.hypertrace.core.query.service.api.Expression.ValueCase.COLUMNIDENTIFIER; import static org.hypertrace.core.query.service.api.Expression.ValueCase.LITERAL; @@ -66,7 +67,7 @@ Entry toSQL( // how it is created. for (Expression expr : allSelections) { pqlBuilder.append(delim); - pqlBuilder.append(convertExpression2String(expr, paramsBuilder, executionContext)); + pqlBuilder.append(convertExpressionToString(expr, paramsBuilder, executionContext)); delim = ", "; } @@ -79,7 +80,7 @@ Entry toSQL( if (request.hasFilter()) { pqlBuilder.append(" AND "); String filterClause = - convertFilter2String(request.getFilter(), paramsBuilder, executionContext); + convertFilterToString(request.getFilter(), paramsBuilder, executionContext); pqlBuilder.append(filterClause); } @@ -89,7 +90,7 @@ Entry toSQL( for (Expression groupByExpression : request.getGroupByList()) { pqlBuilder.append(delim); pqlBuilder.append( - convertExpression2String(groupByExpression, paramsBuilder, executionContext)); + convertExpressionToString(groupByExpression, paramsBuilder, executionContext)); delim = ", "; } } @@ -98,10 +99,9 @@ Entry toSQL( delim = ""; for (OrderByExpression orderByExpression : request.getOrderByList()) { pqlBuilder.append(delim); - String orderBy = - convertExpression2String( - orderByExpression.getExpression(), paramsBuilder, executionContext); - pqlBuilder.append(orderBy); + pqlBuilder.append( + convertExpressionToString( + orderByExpression.getExpression(), paramsBuilder, executionContext)); if (SortOrder.DESC.equals(orderByExpression.getOrder())) { pqlBuilder.append(" desc "); } @@ -126,16 +126,16 @@ Entry toSQL( return new SimpleEntry<>(pqlBuilder.toString(), paramsBuilder.build()); } - private String convertFilter2String( + private String convertFilterToString( Filter filter, Builder paramsBuilder, ExecutionContext executionContext) { StringBuilder builder = new StringBuilder(); - String operator = convertOperator2String(filter.getOperator()); + String operator = convertOperatorToString(filter.getOperator()); if (filter.getChildFilterCount() > 0) { String delim = ""; builder.append("( "); for (Filter childFilter : filter.getChildFilterList()) { builder.append(delim); - builder.append(convertFilter2String(childFilter, paramsBuilder, executionContext)); + builder.append(convertFilterToString(childFilter, paramsBuilder, executionContext)); builder.append(" "); delim = operator + " "; } @@ -149,9 +149,9 @@ private String convertFilter2String( builder.append(operator); builder.append("("); builder.append( - convertExpression2String(filter.getLhs(), paramsBuilder, executionContext)); + convertExpressionToString(filter.getLhs(), paramsBuilder, executionContext)); builder.append(","); - builder.append(convertExpression2String(rhs, paramsBuilder, executionContext)); + builder.append(convertExpressionToString(rhs, paramsBuilder, executionContext)); builder.append(")"); break; case CONTAINS_KEY: @@ -185,11 +185,11 @@ private String convertFilter2String( default: rhs = handleValueConversionForLiteralExpression(filter.getLhs(), filter.getRhs()); builder.append( - convertExpression2String(filter.getLhs(), paramsBuilder, executionContext)); + convertExpressionToString(filter.getLhs(), paramsBuilder, executionContext)); builder.append(" "); builder.append(operator); builder.append(" "); - builder.append(convertExpression2String(rhs, paramsBuilder, executionContext)); + builder.append(convertExpressionToString(rhs, paramsBuilder, executionContext)); } } return builder.toString(); @@ -203,9 +203,7 @@ private String convertFilter2String( * @return newly created literal {@link Expression} of rhs if converted else the same one. */ private Expression handleValueConversionForLiteralExpression(Expression lhs, Expression rhs) { - if (!((lhs.getValueCase().equals(COLUMNIDENTIFIER) - || (lhs.getValueCase().equals(ATTRIBUTE_EXPRESSION) && !isComplexAttribute(lhs))) - && rhs.getValueCase().equals(LITERAL))) { + if (!(isSimpleAttributeExpression(lhs) && rhs.getValueCase().equals(LITERAL))) { return rhs; } @@ -226,7 +224,7 @@ private Expression handleValueConversionForLiteralExpression(Expression lhs, Exp } } - private String convertOperator2String(Operator operator) { + private String convertOperatorToString(Operator operator) { switch (operator) { case AND: return "AND"; @@ -263,7 +261,7 @@ private String convertOperator2String(Operator operator) { } } - private String convertExpression2String( + private String convertExpressionToString( Expression expression, Builder paramsBuilder, ExecutionContext executionContext) { switch (expression.getValueCase()) { case COLUMNIDENTIFIER: @@ -272,8 +270,21 @@ private String convertExpression2String( viewDefinition.getPhysicalColumnNames(getLogicalColumnName(expression)); return joiner.join(columnNames); case ATTRIBUTE_EXPRESSION: - if (isComplexAttribute(expression)) { - /** TODO:Handle complex attribute */ + if (isAttributeExpressionWithSubpath(expression)) { + String keyCol = convertExpressionToMapKeyColumn(expression); + String valCol = convertExpressionToMapValueColumn(expression); + String pathExpression = expression.getAttributeExpression().getSubpath(); + LiteralConstant pathExpressionLiteral = + LiteralConstant.newBuilder() + .setValue(Value.newBuilder().setString(pathExpression).build()) + .build(); + + return String.format( + "%s(%s,%s,%s)", + MAP_VALUE, + keyCol, + convertLiteralToString(pathExpressionLiteral, paramsBuilder), + valCol); } else { // this takes care of the Map Type where it's split into 2 columns columnNames = viewDefinition.getPhysicalColumnNames(getLogicalColumnName(expression)); @@ -286,10 +297,10 @@ private String convertExpression2String( executionContext, expression.getFunction(), argExpression -> - convertExpression2String(argExpression, paramsBuilder, executionContext)); + convertExpressionToString(argExpression, paramsBuilder, executionContext)); case ORDERBY: OrderByExpression orderBy = expression.getOrderBy(); - return convertExpression2String(orderBy.getExpression(), paramsBuilder, executionContext); + return convertExpressionToString(orderBy.getExpression(), paramsBuilder, executionContext); case VALUE_NOT_SET: break; } @@ -297,43 +308,19 @@ private String convertExpression2String( } private String convertExpressionToMapKeyColumn(Expression expression) { - if ((expression.getValueCase() == COLUMNIDENTIFIER) - || (expression.getValueCase() == ATTRIBUTE_EXPRESSION && !isComplexAttribute(expression))) { - String col = viewDefinition.getKeyColumnNameForMap(getLogicalColumnName(expression)); - if (col != null && col.length() > 0) { - return col; - } + String col = viewDefinition.getKeyColumnNameForMap(getLogicalColumnName(expression)); + if (col != null && col.length() > 0) { + return col; } - throw new IllegalArgumentException( - "operator CONTAINS_KEY/KEYVALUE supports multi value column only"); + throw new IllegalArgumentException("operator supports multi value column only"); } private String convertExpressionToMapValueColumn(Expression expression) { - if ((expression.getValueCase() == COLUMNIDENTIFIER) - || (expression.getValueCase() == ATTRIBUTE_EXPRESSION && !isComplexAttribute(expression))) { - String col = viewDefinition.getValueColumnNameForMap(getLogicalColumnName(expression)); - if (col != null && col.length() > 0) { - return col; - } - } - throw new IllegalArgumentException( - "operator CONTAINS_KEY/KEYVALUE supports multi value column only"); - } - - private String getLogicalColumnName(Expression expression) { - switch (expression.getValueCase()) { - case COLUMNIDENTIFIER: - return expression.getColumnIdentifier().getColumnName(); - case ATTRIBUTE_EXPRESSION: - return expression.getAttributeExpression().getAttributeId(); - default: - throw new IllegalArgumentException( - "Supports " - + ATTRIBUTE_EXPRESSION - + " and " - + COLUMNIDENTIFIER - + " expression type only"); + String col = viewDefinition.getValueColumnNameForMap(getLogicalColumnName(expression)); + if (col != null && col.length() > 0) { + return col; } + throw new IllegalArgumentException("operator supports multi value column only"); } private LiteralConstant[] convertExpressionToMapLiterals(Expression expression) { diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java index 5868e333..558afd96 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java @@ -3,17 +3,20 @@ import static io.reactivex.rxjava3.core.Single.zip; import static org.hypertrace.core.query.service.QueryRequestUtil.createBooleanLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; import static org.hypertrace.core.query.service.QueryRequestUtil.createDoubleLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createLongLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullNumberLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullStringLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createStringLiteralExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.isAttributeExpressionWithSubpath; import io.reactivex.rxjava3.core.Maybe; import io.reactivex.rxjava3.core.Observable; import io.reactivex.rxjava3.core.Single; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import javax.inject.Inject; import org.hypertrace.core.attribute.service.cachingclient.CachingAttributeClient; import org.hypertrace.core.attribute.service.projection.AttributeProjection; @@ -25,11 +28,13 @@ import org.hypertrace.core.attribute.service.v1.ProjectionExpression; import org.hypertrace.core.attribute.service.v1.ProjectionOperator; import org.hypertrace.core.query.service.QueryFunctionConstants; +import org.hypertrace.core.query.service.QueryRequestUtil; import org.hypertrace.core.query.service.QueryTransformation; import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Filter; import org.hypertrace.core.query.service.api.Function; +import org.hypertrace.core.query.service.api.Operator; import org.hypertrace.core.query.service.api.OrderByExpression; import org.hypertrace.core.query.service.api.QueryRequest; import org.slf4j.Logger; @@ -59,7 +64,7 @@ public Single transform( this.transformExpressionList(queryRequest.getGroupByList()), this.transformOrderByList(queryRequest.getOrderByList()), (selections, aggregations, filter, groupBys, orderBys) -> - this.rebuildRequestOmittingDefaults( + this.rebuildRequest( queryRequest, selections, aggregations, filter, groupBys, orderBys)) .doOnSuccess(transformed -> this.debugLogIfRequestTransformed(queryRequest, transformed)); } @@ -315,19 +320,21 @@ private Filter rebuildFilterOmittingDefaults( return builder.clearChildFilter().addAllChildFilter(childFilters).build(); } - private QueryRequest rebuildRequestOmittingDefaults( + private QueryRequest rebuildRequest( QueryRequest original, List selections, List aggregations, - Filter filter, + Filter originalFilter, List groupBys, List orderBys) { + QueryRequest.Builder builder = original.toBuilder(); + Filter updatedFilter = rebuildFilterForComplexAttributeExpression(originalFilter, orderBys); - if (Filter.getDefaultInstance().equals(filter)) { + if (Filter.getDefaultInstance().equals(updatedFilter)) { builder.clearFilter(); } else { - builder.setFilter(filter); + builder.setFilter(updatedFilter); } return builder @@ -341,4 +348,73 @@ private QueryRequest rebuildRequestOmittingDefaults( .addAllOrderBy(orderBys) .build(); } + + /* + * We need the CONTAINS_KEY filter in all filters and order bys dealing with complex + * attribute expressions as Pinot gives error if particular key is absent. Rest all work fine. + * To handle order bys, we add the corresponding filter at the top and 'AND' it with the main filter. + * To handle filter, we modify each filter (say filter1) as : "CONTAINS_KEY AND filter1". + */ + private Filter rebuildFilterForComplexAttributeExpression( + Filter originalFilter, List orderBys) { + + Filter updatedFilter = updateFilterForComplexAttributeExpressionFromFilter(originalFilter); + List filterList = createFilterForComplexAttributeExpressionFromOrderBy(orderBys); + + if (filterList.isEmpty()) { + return updatedFilter; + } + + if (!updatedFilter.equals(Filter.getDefaultInstance())) { + return Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(updatedFilter) + .addAllChildFilter(filterList) + .build(); + } + + if (filterList.size() > 1) { + return Filter.newBuilder().setOperator(Operator.AND).addAllChildFilter(filterList).build(); + } else { + return filterList.get(0); + } + } + + private Filter updateFilterForComplexAttributeExpressionFromFilter(Filter originalFilter) { + /* + * If childFilter is present, then the expected operators comprise the logical operators. + * If childFilter is absent, then the filter is a leaf filter which will have lhs and rhs. + */ + if (originalFilter.getChildFilterCount() > 0) { + Filter.Builder builder = Filter.newBuilder(); + builder.setOperator(originalFilter.getOperator()); + originalFilter + .getChildFilterList() + .forEach( + childFilter -> + builder.addChildFilter( + updateFilterForComplexAttributeExpressionFromFilter(childFilter))); + return builder.build(); + } else if (isAttributeExpressionWithSubpath(originalFilter.getLhs())) { + Filter childFilter = + createContainsKeyFilter(originalFilter.getLhs().getAttributeExpression()); + return Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(originalFilter) + .addChildFilter(childFilter) + .build(); + } else { + return originalFilter; + } + } + + private List createFilterForComplexAttributeExpressionFromOrderBy( + List orderByExpressionList) { + return orderByExpressionList.stream() + .map(OrderByExpression::getExpression) + .filter(QueryRequestUtil::isAttributeExpressionWithSubpath) + .map(Expression::getAttributeExpression) + .map(QueryRequestUtil::createContainsKeyFilter) + .collect(Collectors.toList()); + } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java index 84e6060a..b596d652 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java @@ -1,6 +1,6 @@ package org.hypertrace.core.query.service.prometheus; -import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnNameForSimpleColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnName; import com.google.protobuf.ByteString; import java.util.List; @@ -27,9 +27,8 @@ void convertFilterToString( childFilter, timeFilterColumn, expressionToColumnConverter, filterList); } } else { - if (QueryRequestUtil.isSimpleColumnExpression(filter.getLhs()) - && timeFilterColumn.equals( - getLogicalColumnNameForSimpleColumnExpression(filter.getLhs()))) { + if (QueryRequestUtil.isSimpleAttributeExpression(filter.getLhs()) + && timeFilterColumn.equals(getLogicalColumnName(filter.getLhs()))) { return; } StringBuilder builder = new StringBuilder(); diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java index 9f766331..a52f0e1e 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java @@ -137,8 +137,7 @@ private LinkedHashSet prepareSelectionColumnSet( switch (valueCase) { case ATTRIBUTE_EXPRESSION: case COLUMNIDENTIFIER: - selectionColumnSet.add( - QueryRequestUtil.getLogicalColumnNameForSimpleColumnExpression(expression)); + selectionColumnSet.add(QueryRequestUtil.getLogicalColumnName(expression)); break; case FUNCTION: if (QueryRequestUtil.isDateTimeFunction(expression)) { diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java index 19476638..6e1ba89b 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java @@ -7,8 +7,7 @@ class PrometheusUtils { static String getColumnNameForMetricFunction(Expression functionExpression) { String functionName = functionExpression.getFunction().getFunctionName().toUpperCase(); String columnName = - QueryRequestUtil.getLogicalColumnNameForSimpleColumnExpression( - functionExpression.getFunction().getArguments(0)); + QueryRequestUtil.getLogicalColumnName(functionExpression.getFunction().getArguments(0)); return String.join(":", functionName, columnName); } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java index 4b271810..2968d711 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java @@ -1,6 +1,6 @@ package org.hypertrace.core.query.service.prometheus; -import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnNameForSimpleColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnName; import com.google.common.base.Preconditions; import java.util.List; @@ -47,7 +47,7 @@ QueryCost calculateCost(QueryRequest queryRequest, ExecutionContext executionCon // all selection including group by and aggregations should be either on column or attribute if (executionContext.getAllSelections().stream() .filter(Predicate.not(QueryRequestUtil::isDateTimeFunction)) - .anyMatch(Predicate.not(QueryRequestUtil::isSimpleColumnExpression))) { + .anyMatch(Predicate.not(QueryRequestUtil::isSimpleAttributeExpression))) { return QueryCost.UNSUPPORTED; } @@ -88,13 +88,13 @@ private boolean selectionAndGroupByOnDifferentColumn( Set selections = selectionList.stream() - .map(QueryRequestUtil::getLogicalColumnNameForSimpleColumnExpression) + .map(QueryRequestUtil::getLogicalColumnName) .collect(Collectors.toSet()); Set groupBys = groupByList.stream() .filter(Predicate.not(QueryRequestUtil::isDateTimeFunction)) - .map(QueryRequestUtil::getLogicalColumnNameForSimpleColumnExpression) + .map(QueryRequestUtil::getLogicalColumnName) .collect(Collectors.toSet()); return !selections.equals(groupBys); } @@ -113,7 +113,7 @@ private boolean areAggregationsNotSupported(List aggregationList) { return true; } Expression functionArgument = function.getArgumentsList().get(0); - String attributeId = getLogicalColumnNameForSimpleColumnExpression(functionArgument); + String attributeId = getLogicalColumnName(functionArgument); if (!PrometheusFunctionConverter.supportedFunctions.contains( function.getFunctionName())) { return true; @@ -143,7 +143,7 @@ private boolean isFilterNotSupported(Filter filter) { } // lhs condition of filter should be column or simple attribute - if (!QueryRequestUtil.isSimpleColumnExpression(filter.getLhs())) { + if (!QueryRequestUtil.isSimpleAttributeExpression(filter.getLhs())) { return true; } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java index 4c6969ab..4f48f9c3 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java @@ -1,6 +1,6 @@ package org.hypertrace.core.query.service.prometheus; -import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnNameForSimpleColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnName; import java.time.Duration; import java.util.ArrayList; @@ -160,12 +160,11 @@ private String buildQuery( private MetricConfig getMetricConfigForFunction(Expression functionSelection) { return prometheusViewDefinition.getMetricConfigForLogicalMetricName( - getLogicalColumnNameForSimpleColumnExpression( - functionSelection.getFunction().getArgumentsList().get(0))); + getLogicalColumnName(functionSelection.getFunction().getArgumentsList().get(0))); } private String convertColumnAttributeToString(Expression expression) { return prometheusViewDefinition.getPhysicalColumnNameForLogicalColumnName( - getLogicalColumnNameForSimpleColumnExpression(expression)); + getLogicalColumnName(expression)); } } diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java index 44472fb5..ee91b207 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java @@ -1,5 +1,7 @@ package org.hypertrace.core.query.service; +import static org.hypertrace.core.query.service.QueryRequestUtil.createStringArrayLiteralValueExpression; + import java.util.Arrays; import java.util.List; import org.hypertrace.core.query.service.api.AttributeExpression; @@ -25,6 +27,13 @@ public static Expression.Builder createSimpleAttributeExpression(String columnNa .setAttributeExpression(AttributeExpression.newBuilder().setAttributeId(columnName)); } + public static Expression.Builder createComplexAttributeExpression( + String attributeId, String subPath) { + return Expression.newBuilder() + .setAttributeExpression( + AttributeExpression.newBuilder().setAttributeId(attributeId).setSubpath(subPath)); + } + public static Expression createAliasedColumnExpression(String columnName, String alias) { return Expression.newBuilder() .setColumnIdentifier( @@ -169,17 +178,6 @@ public static Expression createStringLiteralValueExpression(String value) { .build(); } - public static Expression createStringArrayLiteralValueExpression(List values) { - return Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder() - .addAllStringArray(values) - .setValueType(ValueType.STRING_ARRAY))) - .build(); - } - public static Expression createLongLiteralValueExpression(long value) { return Expression.newBuilder() .setLiteral( diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java index 7cc4c18d..ab780039 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java @@ -2,12 +2,15 @@ import static java.util.Objects.requireNonNull; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedFunctionExpressionWithSimpleAttribute; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createComplexAttributeExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createCompositeFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createCountByColumnSelectionWithSimpleAttribute; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFunctionExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createOrderByExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createSimpleAttributeExpression; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringArrayLiteralValueExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringLiteralValueExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createTimeFilterWithSimpleAttribute; +import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -15,7 +18,6 @@ import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; import java.util.LinkedHashSet; -import java.util.List; import java.util.Map.Entry; import org.apache.pinot.client.Connection; import org.apache.pinot.client.Request; @@ -38,11 +40,7 @@ public class MigrationTest { private static final String TENANT_ID = "__default"; private static final String TENANT_COLUMN_NAME = "tenant_id"; - private static final String TEST_REQUEST_HANDLER_CONFIG_FILE = "request_handler.conf"; - private static final String TEST_SERVICE_REQUEST_HANDLER_CONFIG_FILE = - "service_request_handler.conf"; - private Connection connection; private ExecutionContext executionContext; @@ -91,6 +89,44 @@ public void testQuery() { executionContext); } + @Test + public void testQuerySelectionUsingMapAttributeWithSubPath() { + Builder builder = QueryRequest.newBuilder(); + builder.addSelection(createComplexAttributeExpression("Span.tags", "span.kind")); + ViewDefinition viewDefinition = getDefaultViewDefinition(); + defaultMockingForExecutionContext(); + + assertPQLQuery( + builder.build(), + "Select mapValue(tags__KEYS,'span.kind',tags__VALUES) FROM SpanEventView " + + "where " + + viewDefinition.getTenantIdColumn() + + " = '" + + TENANT_ID + + "'", + viewDefinition, + executionContext); + } + + @Test + public void testQuerySelectionUsingMapAttributeWithoutSubPath() { + Builder builder = QueryRequest.newBuilder(); + builder.addSelection(createSimpleAttributeExpression("Span.tags")); + ViewDefinition viewDefinition = getDefaultViewDefinition(); + defaultMockingForExecutionContext(); + + assertPQLQuery( + builder.build(), + "Select tags__KEYS, tags__VALUES FROM SpanEventView " + + "where " + + viewDefinition.getTenantIdColumn() + + " = '" + + TENANT_ID + + "'", + viewDefinition, + executionContext); + } + @Test public void testQueryMultipleDistinctSelection() { Builder builder = QueryRequest.newBuilder(); @@ -233,36 +269,166 @@ public void testQueryWithDistinctCountAggregation() { } @Test - public void testQueryWithContainsKeyValueOperator() { + public void testQueryWithEQFilterForMapAttribute() { Builder builder = QueryRequest.newBuilder(); - Expression spanTag = createSimpleAttributeExpression("Span.tags").build(); + Expression spanTag = createComplexAttributeExpression("Span.tags", "FLAGS").build(); builder.addSelection(spanTag); - Expression tag = createStringArrayLiteralValueExpression(List.of("FLAGS", "0")); - Filter likeFilter = + Filter equalFilter = Filter.newBuilder() - .setOperator(Operator.CONTAINS_KEYVALUE) + .setOperator(Operator.EQ) .setLhs(spanTag) - .setRhs(tag) + .setRhs(createStringLiteralValueExpression("0")) + .build(); + builder.setFilter( + createCompositeFilter( + Operator.AND, createContainsKeyFilter("Span.tags", "FLAGS"), equalFilter)); + + ViewDefinition viewDefinition = getDefaultViewDefinition(); + defaultMockingForExecutionContext(); + + assertPQLQuery( + builder.build(), + "SELECT mapValue(tags__keys,'flags',tags__values) FROM SpanEventView " + + "WHERE " + + viewDefinition.getTenantIdColumn() + + " = '" + + TENANT_ID + + "' " + + "AND ( tags__keys = 'flags' and mapvalue(tags__keys,'flags',tags__values) = '0' )", + viewDefinition, + executionContext); + } + + @Test + public void testQueryWithGTFilterForMapAttribute() { + Builder builder = QueryRequest.newBuilder(); + Expression spanKind = createComplexAttributeExpression("Span.tags", "span.kind").build(); + builder.addSelection(spanKind); + + Filter greaterThanFilter = + Filter.newBuilder() + .setOperator(Operator.GT) + .setLhs(spanKind) + .setRhs(createStringLiteralValueExpression("client")) .build(); - builder.setFilter(likeFilter); + builder.setFilter( + createCompositeFilter( + Operator.AND, createContainsKeyFilter("Span.tags", "span.kind"), greaterThanFilter)); ViewDefinition viewDefinition = getDefaultViewDefinition(); defaultMockingForExecutionContext(); assertPQLQuery( builder.build(), - "SELECT tags__keys, tags__values FROM SpanEventView " + "SELECT mapValue(tags__keys,'span.kind',tags__values) FROM SpanEventView " + "WHERE " + viewDefinition.getTenantIdColumn() + " = '" + TENANT_ID + "' " - + "AND tags__keys = 'flags' and tags__values = '0' and mapvalue(tags__keys,'flags',tags__values) = '0'", + + "AND ( tags__keys = 'span.kind' and mapvalue(tags__keys,'span.kind',tags__values) > 'client' )", viewDefinition, executionContext); } + @Test + public void testQueryWithOrderByWithMapAttribute() { + Builder builder = QueryRequest.newBuilder(); + Expression spanKind = createComplexAttributeExpression("Span.tags", "span.kind").build(); + builder.addSelection(spanKind); + + Filter greaterThanOrEqualToFilter = + Filter.newBuilder() + .setOperator(Operator.GE) + .setLhs(spanKind) + .setRhs(createStringLiteralValueExpression("client")) + .build(); + builder.setFilter( + createCompositeFilter( + Operator.AND, + createContainsKeyFilter("Span.tags", "span.kind"), + greaterThanOrEqualToFilter)); + builder.addOrderBy(createOrderByExpression(spanKind.toBuilder(), SortOrder.DESC)); + + ViewDefinition viewDefinition = getDefaultViewDefinition(); + defaultMockingForExecutionContext(); + + assertPQLQuery( + builder.build(), + "select mapValue(tags__KEYS,'span.kind',tags__VALUES) FROM spanEventView " + + "WHERE " + + viewDefinition.getTenantIdColumn() + + " = '" + + TENANT_ID + + "' " + + "AND ( tags__keys = 'span.kind' and mapvalue(tags__keys,'span.kind',tags__values) >= 'client' ) " + + "order by mapvalue(tags__KEYS,'span.kind',tags__VALUES) " + + "DESC ", + viewDefinition, + executionContext); + } + + @Test + public void testQueryWithGroupByWithMapAttribute() { + Builder builder = QueryRequest.newBuilder(buildGroupByMapAttributeQuery()); + ViewDefinition viewDefinition = getDefaultViewDefinition(); + defaultMockingForExecutionContext(); + assertPQLQuery( + builder.build(), + "select mapValue(tags__KEYS,'span.kind',tags__VALUES), AVG(duration_millis) FROM spanEventView" + + " where " + + viewDefinition.getTenantIdColumn() + + " = '" + + TENANT_ID + + "' " + + "AND ( start_time_millis > 1570658506605 AND start_time_millis < 1570744906673 " + + "AND tags__keys = 'span.kind' " + + "AND mapValue(tags__KEYS,'span.kind',tags__VALUES) != '' ) " + + "group by mapValue(tags__KEYS,'span.kind',tags__VALUES)", + viewDefinition, + executionContext); + } + + private QueryRequest buildGroupByMapAttributeQuery() { + Builder builder = QueryRequest.newBuilder(); + + Filter startTimeFilter = + createTimeFilterWithSimpleAttribute("Span.start_time_millis", Operator.GT, 1570658506605L); + Filter endTimeFilter = + createTimeFilterWithSimpleAttribute("Span.start_time_millis", Operator.LT, 1570744906673L); + Filter neqFilter = + Filter.newBuilder() + .setLhs(createComplexAttributeExpression("Span.tags", "span.kind")) + .setOperator(Operator.NEQ) + .setRhs(createStringLiteralValueExpression("")) + .build(); + Filter containsKeyFilter = createContainsKeyFilter("Span.tags", "span.kind"); + + Filter andFilter = + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(startTimeFilter) + .addChildFilter(endTimeFilter) + .addChildFilter(containsKeyFilter) + .addChildFilter(neqFilter) + .build(); + builder.setFilter(andFilter); + + Expression avg = + createAliasedFunctionExpressionWithSimpleAttribute( + "AVG", "Span.duration_millis", "avg_duration") + .build(); + builder.addSelection(avg); + + Expression mapAttributeSelection = + createComplexAttributeExpression("Span.tags", "span.kind").build(); + builder.addSelection(mapAttributeSelection); + + builder.addGroupBy(mapAttributeSelection); + return builder.build(); + } + private QueryRequest buildOrderByQuery() { Builder builder = QueryRequest.newBuilder(); Expression startTimeColumn = createSimpleAttributeExpression("Span.start_time_millis").build(); diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java index 6ddbfc04..03d9202c 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java @@ -1,5 +1,6 @@ package org.hypertrace.core.query.service.pinot; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createComplexAttributeExpression; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -1133,6 +1134,21 @@ public void testNullExecutionContextEmitsNPE() { .blockingSubscribe()); } + @Test + public void testInvalidAttributeExpressionQueryRequestThrowsIAE() { + QueryRequest queryRequest = + QueryRequest.newBuilder() + .addSelection(createComplexAttributeExpression("Trace.id", "trace.kind")) + .build(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + pinotBasedRequestHandler + .handleRequest(queryRequest, new ExecutionContext("test-tenant-id", queryRequest)) + .blockingSubscribe()); + } + @Test public void testNullTenantIdQueryRequestContextThrowsNPE() { Assertions.assertThrows( diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverterTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverterTest.java index a1dd1a12..67646ffa 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverterTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverterTest.java @@ -13,10 +13,11 @@ 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; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringLiteralValueExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createTimeFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createTimestampFilter; +import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; +import static org.hypertrace.core.query.service.QueryRequestUtil.createStringArrayLiteralValueExpression; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -546,13 +547,8 @@ public void testQueryWithLikeOperator() { @Test public void testQueryWithContainsKeyOperator() { Builder builder = QueryRequest.newBuilder(); - Expression spanTag = createColumnExpression("Span.tags").build(); - builder.addSelection(spanTag); - - Expression tag = createStringArrayLiteralValueExpression(List.of("FLAGS", "0")); - Filter likeFilter = - Filter.newBuilder().setOperator(Operator.CONTAINS_KEY).setLhs(spanTag).setRhs(tag).build(); - builder.setFilter(likeFilter); + builder.addSelection(createColumnExpression("Span.tags")); + builder.setFilter(createContainsKeyFilter("Span.tags", "FLAGS")); ViewDefinition viewDefinition = getDefaultViewDefinition(); defaultMockingForExecutionContext(); diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java index 0a9d8422..10346d4c 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java @@ -13,17 +13,20 @@ import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedColumnExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedFunctionExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createComplexAttributeExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createCompositeFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createEqualsFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFunctionExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createInFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createOrderByExpression; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringArrayLiteralValueExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringLiteralValueExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullNumberLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullStringLiteralExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createStringArrayLiteralValueExpression; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; import io.reactivex.rxjava3.core.Single; @@ -39,6 +42,8 @@ import org.hypertrace.core.attribute.service.v1.ProjectionExpression; import org.hypertrace.core.attribute.service.v1.ProjectionOperator; import org.hypertrace.core.query.service.QueryTransformation.QueryTransformationContext; +import org.hypertrace.core.query.service.api.Expression; +import org.hypertrace.core.query.service.api.Filter; import org.hypertrace.core.query.service.api.Operator; import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.SortOrder; @@ -73,6 +78,214 @@ void beforeEach() { new ProjectionTransformation(this.mockAttributeClient, new AttributeProjectionRegistry()); } + @Test + void transQueryWithComplexAttributeExpression_SingleFilter() { + this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); + + Expression spanTags = createComplexAttributeExpression("Span.tags", "span.kind").build(); + Filter filter = + Filter.newBuilder() + .setLhs(spanTags) + .setOperator(Operator.EQ) + .setRhs(createColumnExpression("server")) + .build(); + QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter( + Filter.newBuilder() + .setOperator(Operator.AND) + .addAllChildFilter( + List.of(filter, createContainsKeyFilter(spanTags.getAttributeExpression()))) + .build()) + .build(); + + assertEquals( + expectedTransform, + this.projectionTransformation + .transform(originalRequest, mockTransformationContext) + .blockingGet()); + } + + @Test + void transQueryWithComplexAttributeExpression_MultipleFilter() { + this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); + this.mockAttribute("0", AttributeMetadata.getDefaultInstance()); + + Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); + Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); + + Filter childFilter1 = + Filter.newBuilder() + .setLhs(spanTags1) + .setOperator(Operator.EQ) + .setRhs(createColumnExpression("0")) + .build(); + Filter childFilter2 = + Filter.newBuilder() + .setLhs(spanTags2) + .setOperator(Operator.EQ) + .setRhs(createColumnExpression("server")) + .build(); + + Filter.Builder filter = createCompositeFilter(Operator.AND, childFilter1, childFilter2); + QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); + + QueryRequest expectedTransform = + this.projectionTransformation + .transform(originalRequest, mockTransformationContext) + .blockingGet(); + List childFilterList = expectedTransform.getFilter().getChildFilterList(); + + assertTrue( + childFilterList + .get(0) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags1.getAttributeExpression()))); + assertTrue( + childFilterList + .get(1) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags2.getAttributeExpression()))); + } + + @Test + void transQueryWithComplexAttributeExpression_HierarchicalFilter() { + this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); + this.mockAttribute("0", AttributeMetadata.getDefaultInstance()); + this.mockAttribute(SIMPLE_ATTRIBUTE_ID, AttributeMetadata.getDefaultInstance()); + + Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); + Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); + + Filter filter1 = + Filter.newBuilder() + .setLhs(spanTags1) + .setOperator(Operator.EQ) + .setRhs(createColumnExpression("0")) + .build(); + Filter filter2 = + Filter.newBuilder() + .setLhs(spanTags2) + .setOperator(Operator.EQ) + .setRhs(createColumnExpression("server")) + .build(); + Filter filter = + createCompositeFilter( + Operator.AND, + createEqualsFilter(SIMPLE_ATTRIBUTE_ID, "otherValue"), + createCompositeFilter(Operator.AND, filter1, filter2).build()) + .build(); + + QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); + + QueryRequest expectedTransform = + this.projectionTransformation + .transform(originalRequest, mockTransformationContext) + .blockingGet(); + List childFilterList = + expectedTransform.getFilter().getChildFilterList().get(1).getChildFilterList(); + + assertTrue( + childFilterList + .get(0) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags1.getAttributeExpression()))); + assertTrue( + childFilterList + .get(1) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags2.getAttributeExpression()))); + } + + @Test + void transQueryWithComplexAttributeExpression_OrderByAndFilter() { + this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); + + Filter filter = + Filter.newBuilder() + .setLhs(spanTag) + .setOperator(Operator.EQ) + .setRhs(createColumnExpression("server")) + .build(); + Filter containsKeyFilter = createContainsKeyFilter("Span.tags", "span.kind"); + + QueryRequest originalRequest = + QueryRequest.newBuilder() + .setFilter(filter) + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter( + createCompositeFilter( + Operator.AND, + createCompositeFilter(Operator.AND, filter, containsKeyFilter).build(), + containsKeyFilter)) + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + assertEquals( + expectedTransform, + this.projectionTransformation + .transform(originalRequest, mockTransformationContext) + .blockingGet()); + } + + @Test + void transQueryWithComplexAttributeExpression_SingleOrderBy() { + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); + + QueryRequest originalRequest = + QueryRequest.newBuilder() + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter(createContainsKeyFilter("Span.tags", "span.kind")) + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + assertEquals( + expectedTransform, + this.projectionTransformation + .transform(originalRequest, mockTransformationContext) + .blockingGet()); + } + + @Test + void transQueryWithComplexAttributeExpression_MultipleOrderBy() { + Expression.Builder spanTag1 = createComplexAttributeExpression("Span.tags", "span.kind"); + Expression.Builder spanTag2 = createComplexAttributeExpression("Span.tags", "FLAGS"); + + QueryRequest originalRequest = + QueryRequest.newBuilder() + .addOrderBy(createOrderByExpression(spanTag1, SortOrder.ASC)) + .addOrderBy(createOrderByExpression(spanTag2, SortOrder.ASC)) + .build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter( + createCompositeFilter( + Operator.AND, + createContainsKeyFilter("Span.tags", "span.kind"), + createContainsKeyFilter("Span.tags", "FLAGS"))) + .addOrderBy(createOrderByExpression(spanTag1, SortOrder.ASC)) + .addOrderBy(createOrderByExpression(spanTag2, SortOrder.ASC)) + .build(); + + assertEquals( + expectedTransform, + this.projectionTransformation + .transform(originalRequest, mockTransformationContext) + .blockingGet()); + } + @Test void transformsBasicAliasProjection() { this.mockAttribute(PROJECTED_ATTRIBUTE_ID, this.attributeMetadata); diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java index 77290347..eb69b8f9 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java @@ -1,5 +1,6 @@ package org.hypertrace.core.query.service; +import org.hypertrace.core.query.service.api.AttributeExpression; import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Filter; @@ -101,4 +102,11 @@ public static Expression createStringLiteralValueExpression(String value) { .setValue(Value.newBuilder().setString(value).setValueType(ValueType.STRING))) .build(); } + + public static Expression.Builder createComplexAttributeExpression( + String attributeId, String subPath) { + return Expression.newBuilder() + .setAttributeExpression( + AttributeExpression.newBuilder().setAttributeId(attributeId).setSubpath(subPath)); + } } diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java index 5a00dee1..21b31688 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java @@ -1,6 +1,10 @@ package org.hypertrace.core.query.service.htqueries; +import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; +import static org.hypertrace.core.query.service.QueryServiceTestUtils.createComplexAttributeExpression; import static org.hypertrace.core.query.service.QueryServiceTestUtils.createFilter; +import static org.hypertrace.core.query.service.QueryServiceTestUtils.createOrderByExpression; +import static org.hypertrace.core.query.service.QueryServiceTestUtils.createStringLiteralValueExpression; import java.time.Duration; import org.hypertrace.core.query.service.api.ColumnIdentifier; @@ -11,6 +15,7 @@ import org.hypertrace.core.query.service.api.Operator; import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.QueryRequest.Builder; +import org.hypertrace.core.query.service.api.SortOrder; import org.hypertrace.core.query.service.api.Value; import org.hypertrace.core.query.service.api.ValueType; @@ -93,4 +98,46 @@ static QueryRequest buildQuery1() { builder.addGroupBy(Expression.newBuilder().setFunction(dateTimeConvert).build()); return builder.build(); } + + static QueryRequest buildQuery2() { + Builder builder = QueryRequest.newBuilder(); + + Expression apiTraceTags = + createComplexAttributeExpression("API_TRACE.tags", "span.kind").build(); + builder.addSelection(apiTraceTags); + + Filter equalFilter = + Filter.newBuilder() + .setOperator(Operator.EQ) + .setLhs(apiTraceTags) + .setRhs(createStringLiteralValueExpression("client")) + .build(); + + Filter startTimeFilter = + createFilter( + "API_TRACE.startTime", + Operator.GE, + ValueType.LONG, + System.currentTimeMillis() - Duration.ofHours(1).toMillis()); + + Filter endTimeFilter = + createFilter( + "API_TRACE.startTime", + Operator.LT, + ValueType.LONG, + System.currentTimeMillis() + Duration.ofHours(1).toMillis()); + + builder.setFilter( + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(startTimeFilter) + .addChildFilter(endTimeFilter) + .addChildFilter(equalFilter) + .addChildFilter(createContainsKeyFilter("API_TRACE.tags", "span.kind")) + .build()); + + builder.addOrderBy(createOrderByExpression(apiTraceTags, SortOrder.DESC)); + + return builder.build(); + } } 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 05ce1b75..f38b3f70 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 @@ -393,4 +393,15 @@ public void testExplorerQueries() { // COUNT_API_TRACE.calls_[] is 13 assertEquals("13", rows.get(0).getColumn(1).getString()); } + + @Test + public void testExplorerQueriesForAttributeExpression() { + LOG.info("Explorer queries for attribute expression"); + Iterator itr = + queryServiceClient.executeQuery(ExplorerQueries.buildQuery2(), TENANT_ID_MAP, 10000); + List list = Streams.stream(itr).collect(Collectors.toList()); + List rows = list.get(0).getRowList(); + assertEquals(10, rows.size()); + assertEquals("client", rows.get(0).getColumn(0).getString()); + } }