Skip to content

Commit

Permalink
feat: add support for query that have aggregation but missing group by (
Browse files Browse the repository at this point in the history
#124)

* fix: adding support for default group by

* trying out with different aggregation w/o group by

* adding distinct count query w/o group by

* feat: add query tranformer for handling selection

* add tests for corrosponding scenarios

* applied spotless
  • Loading branch information
kotharironak authored Aug 30, 2022
1 parent a42561f commit 8b831d6
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,39 @@ public void testQueryQ1AggregationFilterWithStringInFilterAlongWithNonAliasField
"mongo/test_string_in_filter_aggr_alias_distinct_count_response.json");
}

@ParameterizedTest
@MethodSource("databaseContextBoth")
void testQueryQ1DistinctCountAggregationWithOnlyFilter(String dataStoreName) throws IOException {
Datastore datastore = datastoreMap.get(dataStoreName);
Collection collection = datastore.getCollection(COLLECTION_NAME);
org.hypertrace.core.documentstore.query.Query query =
org.hypertrace.core.documentstore.query.Query.builder()
.addSelection(
AggregateExpression.of(DISTINCT_COUNT, IdentifierExpression.of("quantity")),
"qty_count")
.addSelection(IdentifierExpression.of("item"))
.addSelection(IdentifierExpression.of("price"))
.setFilter(
LogicalExpression.builder()
.operator(AND)
.operand(
RelationalExpression.of(
IdentifierExpression.of("price"), LTE, ConstantExpression.of(10)))
.operand(
RelationalExpression.of(
IdentifierExpression.of("item"),
IN,
ConstantExpression.ofStrings(
List.of("Mirror", "Comb", "Shampoo", "Bottle"))))
.build())
.build();

try (CloseableIterator<Document> resultDocs = collection.aggregate(query)) {
Utils.assertDocsAndSizeEqualWithoutOrder(
dataStoreName, resultDocs, 1, "mongo/test_aggr_only_with_fliter_response.json");
}
}

@ParameterizedTest
@MethodSource("databaseContextBoth")
public void testQueryV1ForSimpleWhereClause(String dataStoreName) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[
{
"qty_count":3
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.hypertrace.core.documentstore.UpdateResult;
import org.hypertrace.core.documentstore.commons.DocStoreConstants;
import org.hypertrace.core.documentstore.postgres.internal.BulkUpdateSubDocsInternalResult;
import org.hypertrace.core.documentstore.postgres.query.v1.transformer.PostgresQueryTransformer;
import org.hypertrace.core.documentstore.postgres.utils.PostgresUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -782,7 +783,7 @@ private CloseableIterator<Document> executeQueryV1(
final org.hypertrace.core.documentstore.query.Query query) {
org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser queryParser =
new org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser(
collectionName, query);
collectionName, transformAndLog(query));
String sqlQuery = queryParser.parse();
try {
PreparedStatement preparedStatement =
Expand All @@ -801,6 +802,14 @@ private CloseableIterator<Document> executeQueryV1(
}
}

private org.hypertrace.core.documentstore.query.Query transformAndLog(
org.hypertrace.core.documentstore.query.Query query) {
LOGGER.debug("Original query before transformation: {}", query);
query = PostgresQueryTransformer.transform(query);
LOGGER.debug("Query after transformation: {}", query);
return query;
}

private boolean isValidType(Object v) {
Set<Class<?>> validClassez =
new HashSet<>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import lombok.Setter;
import org.hypertrace.core.documentstore.postgres.Params;
import org.hypertrace.core.documentstore.postgres.Params.Builder;
import org.hypertrace.core.documentstore.postgres.PostgresCollection;
import org.hypertrace.core.documentstore.postgres.query.v1.transformer.FieldToPgColumnTransformer;
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresAggregationFilterTypeExpressionVisitor;
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresFilterTypeExpressionVisitor;
Expand All @@ -17,8 +18,12 @@
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresUnnestFilterTypeExpressionVisitor;
import org.hypertrace.core.documentstore.query.Pagination;
import org.hypertrace.core.documentstore.query.Query;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class PostgresQueryParser {
private static final Logger LOGGER = LoggerFactory.getLogger(PostgresCollection.class);

@Getter private final String collection;
@Getter private final Query query;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.hypertrace.core.documentstore.postgres.query.v1.transformer;

import com.google.common.collect.ImmutableList;
import java.util.List;
import org.hypertrace.core.documentstore.query.Query;
import org.hypertrace.core.documentstore.query.transform.QueryTransformer;

public class PostgresQueryTransformer {

// Transform the query in the listed below order
private static final List<QueryTransformer> TRANSFORMERS =
new ImmutableList.Builder<QueryTransformer>()
.add(new PostgresSelectionQueryTransformer())
.build();

public static Query transform(final Query query) {
Query transformedQuery = query;

for (QueryTransformer transformer : TRANSFORMERS) {
transformedQuery = transformer.transform(transformedQuery);
}

return transformedQuery;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package org.hypertrace.core.documentstore.postgres.query.v1.transformer;

import java.util.List;
import java.util.stream.Collectors;
import org.hypertrace.core.documentstore.expression.impl.AggregateExpression;
import org.hypertrace.core.documentstore.expression.impl.ConstantExpression;
import org.hypertrace.core.documentstore.expression.impl.FunctionExpression;
import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression;
import org.hypertrace.core.documentstore.parser.SelectTypeExpressionVisitor;
import org.hypertrace.core.documentstore.query.Query;
import org.hypertrace.core.documentstore.query.SelectionSpec;
import org.hypertrace.core.documentstore.query.transform.QueryTransformer;
import org.hypertrace.core.documentstore.query.transform.TransformedQueryBuilder;

/*
* Postgres doesn't support the selection of attributes and aggregation w/o group by expression.
* e.g
* SELECT COUNT(DISTINCT document->>'quantity' ) AS QTY, document->'price' AS price
* FROM testCollection
* WHERE (CAST (document->>'price' AS NUMERIC) <= 10)
*
* So, if group by clause is missing, and selection contains any aggregation expression,
* this transformer removes all the non-aggregated expressions. So, the above query will be transformed
* to:
*
* SELECT COUNT(DISTINCT document->>'quantity' ) AS QTY
* FROM testCollection
* WHERE (CAST (document->>'price' AS NUMERIC) <= 10)
*
* This is the similar behavior supported in our other document store implementation (e.g Mongo)
* */
public class PostgresSelectionQueryTransformer
implements QueryTransformer, SelectTypeExpressionVisitor {

@Override
public Query transform(Query query) {
// no-op if group by clause exits
if (!query.getAggregations().isEmpty()) return query;

// check for all selections, remove non-aggregated selections.
List<SelectionSpec> finalSelectionSpecs =
query.getSelections().stream()
.filter(selectionSpec -> selectionSpec.getExpression().accept(this))
.collect(Collectors.toUnmodifiableList());

return finalSelectionSpecs.size() > 0
? new TransformedQueryBuilder(query).setSelections(finalSelectionSpecs).build()
: query;
}

@Override
public Boolean visit(AggregateExpression expression) {
return true;
}

@Override
public Boolean visit(ConstantExpression expression) {
return false;
}

@Override
public Boolean visit(FunctionExpression expression) {
return false;
}

@Override
public Boolean visit(IdentifierExpression expression) {
return false;
}
}
Loading

0 comments on commit 8b831d6

Please sign in to comment.