Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0.x] ISPN-15254 Support count(*) other than count(someField) on aggregation queries #11440

Merged
merged 5 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package org.infinispan.client.hotrod.query.aggregation;

import static org.assertj.core.api.Assertions.assertThat;
import static org.infinispan.configuration.cache.IndexStorage.LOCAL_HEAP;
import static org.infinispan.query.aggregation.QueryAggregationCountTest.AGGREGATION_RESULT;
import static org.infinispan.query.aggregation.QueryAggregationCountTest.CHUNK_SIZE;
import static org.infinispan.query.aggregation.QueryAggregationCountTest.NUMBER_OF_DAYS;
import static org.infinispan.query.aggregation.QueryAggregationCountTest.REV_AGGREGATION_RESULT;
import static org.infinispan.query.aggregation.QueryAggregationCountTest.chunk;

import java.util.Optional;
import java.util.Random;

import org.infinispan.client.hotrod.RemoteCache;
import org.infinispan.client.hotrod.test.SingleHotRodServerTest;
import org.infinispan.configuration.cache.ConfigurationBuilder;
import org.infinispan.manager.EmbeddedCacheManager;
import org.infinispan.protostream.SerializationContextInitializer;
import org.infinispan.query.Search;
import org.infinispan.query.dsl.Query;
import org.infinispan.query.dsl.QueryFactory;
import org.infinispan.query.model.Sale;
import org.infinispan.test.fwk.TestCacheManagerFactory;
import org.testng.annotations.Test;

@Test(groups = "functional", testName = "org.infinispan.client.hotrod.query.aggregation.RemoteQueryAggregationCountTest")
public class RemoteQueryAggregationCountTest extends SingleHotRodServerTest {

private final Random fixedSeedPseudoRandom = new Random(739);

@Override
protected EmbeddedCacheManager createCacheManager() throws Exception {
ConfigurationBuilder indexed = new ConfigurationBuilder();
indexed.indexing().enable()
.storage(LOCAL_HEAP)
.addIndexedEntity("Sale");

return TestCacheManagerFactory.createServerModeCacheManager(indexed);
}

@Override
protected SerializationContextInitializer contextInitializer() {
return Sale.SaleSchema.INSTANCE;
}

@Test
public void test() {
RemoteCache<Object, Object> remoteCache = remoteCacheManager.getCache();
for (int day = 1; day <= NUMBER_OF_DAYS; day++) {
remoteCache.putAll(chunk(day, fixedSeedPseudoRandom));
}

QueryFactory queryFactory = Search.getQueryFactory(cache);
Query<Object[]> query;

query = queryFactory.create("select status, count(code) from Sale where day = :day group by status order by status");
query.setParameter("day", NUMBER_OF_DAYS / 2);
assertThat(query.list()).containsExactly(AGGREGATION_RESULT);

query = queryFactory.create("select count(code), status from Sale where day = :day group by status order by status");
query.setParameter("day", NUMBER_OF_DAYS / 2);
assertThat(query.list()).containsExactly(REV_AGGREGATION_RESULT);

query = queryFactory.create("select status, count(code) from Sale where day = :day group by status");
query.setParameter("day", NUMBER_OF_DAYS / 2);
assertThat(query.list()).containsExactlyInAnyOrder(AGGREGATION_RESULT);

query = queryFactory.create("select status, count(code) from Sale group by status");
Optional<Integer> totalNotNullItems = query.list().stream()
.map(objects -> ((Long) objects[1]).intValue()).reduce(Integer::sum);
assertThat(totalNotNullItems).hasValue(CHUNK_SIZE * NUMBER_OF_DAYS);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.infinispan.client.hotrod.maxresult;
package org.infinispan.client.hotrod.query.maxresult;

import static org.assertj.core.api.Assertions.assertThat;
import static org.infinispan.configuration.cache.IndexStorage.LOCAL_HEAP;
Expand All @@ -18,7 +18,7 @@
import org.infinispan.test.fwk.TestCacheManagerFactory;
import org.testng.annotations.Test;

@Test(groups = "functional", testName = "org.infinispan.client.hotrod.maxresult.RemoteDefaultMaxResultTest")
@Test(groups = "functional", testName = "org.infinispan.client.hotrod.query.maxresult.RemoteDefaultMaxResultTest")
@TestForIssue(jiraKey = "ISPN-14194")
public class RemoteDefaultMaxResultTest extends SingleHotRodServerTest {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.infinispan.client.hotrod.maxresult;
package org.infinispan.client.hotrod.query.maxresult;

import static org.assertj.core.api.Assertions.assertThat;
import static org.infinispan.configuration.cache.IndexStorage.LOCAL_HEAP;
Expand All @@ -17,7 +17,7 @@
import org.infinispan.test.fwk.TestCacheManagerFactory;
import org.testng.annotations.Test;

@Test(groups = "functional", testName = "org.infinispan.client.hotrod.maxresult.RemoteHitCountAccuracyTest")
@Test(groups = "functional", testName = "org.infinispan.client.hotrod.query.maxresult.RemoteHitCountAccuracyTest")
@TestForIssue(jiraKey = "ISPN-14195")
public class RemoteHitCountAccuracyTest extends SingleHotRodServerTest {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ setFunction
| ^(AVG { delegate.activateAggregation(AggregationFunction.AVG); } numericValueExpression)
| ^(MAX { delegate.activateAggregation(AggregationFunction.MAX); } numericValueExpression)
| ^(MIN { delegate.activateAggregation(AggregationFunction.MIN); } numericValueExpression)
| ^(COUNT (ASTERISK { delegate.activateAggregation(AggregationFunction.COUNT); } | (DISTINCT { delegate.activateAggregation(AggregationFunction.COUNT_DISTINCT); } | ALL { delegate.activateAggregation(AggregationFunction.COUNT); }) countFunctionArguments))
| ^(COUNT (ASTERISK { delegate.activateAsteriskAggregation(AggregationFunction.COUNT); } | (DISTINCT { delegate.activateAggregation(AggregationFunction.COUNT_DISTINCT); } | ALL { delegate.activateAggregation(AggregationFunction.COUNT); }) countFunctionArguments))
;
versionFunction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ public String getOperator() {

void activateAggregation(AggregationFunction aggregationFunction);

void activateAsteriskAggregation(AggregationFunction aggregationFunction);

void deactivateAggregation();

void projectVersion();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public BooleanExpr visit(FullTextOccurExpr fullTextOccurExpr) {
@Override
public BooleanExpr visit(FullTextTermExpr fullTextTermExpr) {
PropertyValueExpr propertyValueExpr = (PropertyValueExpr) fullTextTermExpr.getChild();
if (fieldIndexingMetadata.isIndexed(propertyValueExpr.getPropertyPath().asArrayPath())) {
if (fieldIndexingMetadata.isSearchable(propertyValueExpr.getPropertyPath().asArrayPath())) {
foundIndexed = true;
} else {
predicatesToRemove.add(fullTextTermExpr);
Expand All @@ -64,7 +64,7 @@ public BooleanExpr visit(FullTextTermExpr fullTextTermExpr) {
@Override
public BooleanExpr visit(FullTextRegexpExpr fullTextRegexpExpr) {
PropertyValueExpr propertyValueExpr = (PropertyValueExpr) fullTextRegexpExpr.getChild();
if (fieldIndexingMetadata.isIndexed(propertyValueExpr.getPropertyPath().asArrayPath())) {
if (fieldIndexingMetadata.isSearchable(propertyValueExpr.getPropertyPath().asArrayPath())) {
foundIndexed = true;
} else {
predicatesToRemove.add(fullTextRegexpExpr);
Expand All @@ -75,7 +75,7 @@ public BooleanExpr visit(FullTextRegexpExpr fullTextRegexpExpr) {
@Override
public BooleanExpr visit(FullTextRangeExpr fullTextRangeExpr) {
PropertyValueExpr propertyValueExpr = (PropertyValueExpr) fullTextRangeExpr.getChild();
if (fieldIndexingMetadata.isIndexed(propertyValueExpr.getPropertyPath().asArrayPath())) {
if (fieldIndexingMetadata.isSearchable(propertyValueExpr.getPropertyPath().asArrayPath())) {
foundIndexed = true;
} else {
predicatesToRemove.add(fullTextRangeExpr);
Expand Down Expand Up @@ -113,7 +113,7 @@ public BooleanExpr visit(ConstantBooleanExpr constantBooleanExpr) {
@Override
public BooleanExpr visit(IsNullExpr isNullExpr) {
PropertyValueExpr propertyValueExpr = (PropertyValueExpr) isNullExpr.getChild();
if (fieldIndexingMetadata.isIndexed(propertyValueExpr.getPropertyPath().asArrayPath())) {
if (fieldIndexingMetadata.isSearchable(propertyValueExpr.getPropertyPath().asArrayPath())) {
foundIndexed = true;
} else {
predicatesToRemove.add(isNullExpr);
Expand All @@ -124,7 +124,7 @@ public BooleanExpr visit(IsNullExpr isNullExpr) {
@Override
public BooleanExpr visit(ComparisonExpr comparisonExpr) {
PropertyValueExpr propertyValueExpr = (PropertyValueExpr) comparisonExpr.getLeftChild();
if (fieldIndexingMetadata.isIndexed(propertyValueExpr.getPropertyPath().asArrayPath())) {
if (fieldIndexingMetadata.isSearchable(propertyValueExpr.getPropertyPath().asArrayPath())) {
foundIndexed = true;
} else {
predicatesToRemove.add(comparisonExpr);
Expand All @@ -135,7 +135,7 @@ public BooleanExpr visit(ComparisonExpr comparisonExpr) {
@Override
public BooleanExpr visit(LikeExpr likeExpr) {
PropertyValueExpr propertyValueExpr = (PropertyValueExpr) likeExpr.getChild();
if (fieldIndexingMetadata.isIndexed(propertyValueExpr.getPropertyPath().asArrayPath())) {
if (fieldIndexingMetadata.isSearchable(propertyValueExpr.getPropertyPath().asArrayPath())) {
foundIndexed = true;
} else {
predicatesToRemove.add(likeExpr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ interface FieldIndexingMetadata {
* @param propertyPath the path of the property
* @return {@code true} if the property is indexed, {@code false} otherwise.
*/
boolean isIndexed(String[] propertyPath);
boolean isSearchable(String[] propertyPath);

/**
* Checks if the property of the indexed entity is analyzed.
Expand All @@ -43,6 +43,14 @@ interface FieldIndexingMetadata {
*/
boolean isProjectable(String[] propertyPath);

/**
* Checks if the property of the indexed entity is aggregable.
*
* @param propertyPath the path of the property
* @return {@code true} if the property is aggregable, {@code false} otherwise.
*/
boolean isAggregable(String[] propertyPath);

/**
* Checks if the property of the indexed entity is sortable.
*
Expand All @@ -56,7 +64,7 @@ interface FieldIndexingMetadata {

FieldIndexingMetadata NO_INDEXING = new FieldIndexingMetadata() {
@Override
public boolean isIndexed(String[] propertyPath) {
public boolean isSearchable(String[] propertyPath) {
return false;
}

Expand All @@ -75,6 +83,11 @@ public boolean isProjectable(String[] propertyPath) {
return false;
}

@Override
public boolean isAggregable(String[] propertyPath) {
return false;
}

@Override
public boolean isSortable(String[] propertyPath) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* @author anistor@redhat.com
* @since 9.0
*/
public final class AggregationPropertyPath<TypeMetadata> extends PropertyPath<TypeDescriptor<TypeMetadata>> {
public class AggregationPropertyPath<TypeMetadata> extends PropertyPath<TypeDescriptor<TypeMetadata>> {

private static final Log log = Logger.getMessageLogger(Log.class, AggregationPropertyPath.class.getName());

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.infinispan.objectfilter.impl.syntax.parser;

import java.util.Collections;

import org.infinispan.objectfilter.impl.ql.AggregationFunction;
import org.infinispan.objectfilter.impl.ql.PropertyPath;
import org.infinispan.objectfilter.impl.syntax.parser.projection.CacheValuePropertyPath;

public class CacheValueAggregationPropertyPath<TypeDescriptor> extends AggregationPropertyPath<TypeDescriptor> {

CacheValueAggregationPropertyPath() {
super(AggregationFunction.COUNT, Collections.singletonList(
new PropertyPath.PropertyReference<>(CacheValuePropertyPath.VALUE_PROPERTY_NAME, null, true)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ private enum Phase {

private final String queryString;

private boolean asteriskCount = false;

QueryRendererDelegateImpl(String queryString, ObjectPropertyHelper<TypeMetadata> propertyHelper) {
this.queryString = queryString;
this.propertyHelper = propertyHelper;
Expand Down Expand Up @@ -429,7 +431,7 @@ private void checkAnalyzed(PropertyPath<?> propertyPath, boolean expectAnalyzed)
}

private void checkIndexed(PropertyPath<?> propertyPath) {
if (!fieldIndexingMetadata.isIndexed(propertyPath.asArrayPath())) {
if (!fieldIndexingMetadata.isSearchable(propertyPath.asArrayPath())) {
throw log.getFullTextQueryOnNotIndexedPropertyNotSupportedException(targetTypeName, propertyPath.asStringPath());
}
}
Expand Down Expand Up @@ -485,11 +487,14 @@ public void deactivateFullTextOccur() {
*/
@Override
public void setPropertyPath(PropertyPath<TypeDescriptor<TypeMetadata>> propertyPath) {
boolean aggregationPropertyPath = false;

if (aggregationFunction != null) {
if (propertyPath == null && aggregationFunction != AggregationFunction.COUNT && aggregationFunction != AggregationFunction.COUNT_DISTINCT) {
throw log.getAggregationCanOnlyBeAppliedToPropertyReferencesException(aggregationFunction.name());
}
propertyPath = new AggregationPropertyPath<>(aggregationFunction, propertyPath.getNodes());
aggregationPropertyPath = true;
}
if (phase == Phase.SELECT) {
if (projections == null) {
Expand All @@ -501,7 +506,12 @@ public void setPropertyPath(PropertyPath<TypeDescriptor<TypeMetadata>> propertyP
Class<?> propertyType;
Object nullMarker;
if (propertyPath.getLength() == 1 && propertyPath.isAlias()) {
projection = new CacheValuePropertyPath<>();
if (!aggregationPropertyPath) {
projection = new CacheValuePropertyPath<>();
} else {
projection = new CacheValueAggregationPropertyPath<>();
}

propertyType = null;
nullMarker = null;
} else {
Expand Down Expand Up @@ -529,6 +539,12 @@ public void activateAggregation(AggregationFunction aggregationFunction) {
propertyPath = null;
}

@Override
public void activateAsteriskAggregation(AggregationFunction aggregationFunction) {
activateAggregation(aggregationFunction);
asteriskCount = true;
}

@Override
public void deactivateAggregation() {
aggregationFunction = null;
Expand Down Expand Up @@ -581,6 +597,19 @@ public void groupingValue(String collateName) {
groupBy = new ArrayList<>(ARRAY_INITIAL_LENGTH);
}
groupBy.add(resolveAlias(propertyPath));

if (!asteriskCount) {
return;
}
if (projections == null) {
projections = new ArrayList<>(ARRAY_INITIAL_LENGTH);
projectedTypes = new ArrayList<>(ARRAY_INITIAL_LENGTH);
projectedNullMarkers = new ArrayList<>(ARRAY_INITIAL_LENGTH);
}
projections.add(new CacheValueAggregationPropertyPath<>());
projectedTypes.add(null);
projectedNullMarkers.add(null);
asteriskCount = false;
}

private Object parameterValue(String value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class BooleShannonExpansionTest {

private final BooleShannonExpansion booleShannonExpansion = new BooleShannonExpansion(3, new IndexedFieldProvider.FieldIndexingMetadata() {
@Override
public boolean isIndexed(String[] propertyPath) {
public boolean isSearchable(String[] propertyPath) {
String last = propertyPath[propertyPath.length - 1];
return !"number".equals(last) && !"license".equals(last);
}
Expand All @@ -39,12 +39,17 @@ public boolean isNormalized(String[] propertyPath) {

@Override
public boolean isProjectable(String[] propertyPath) {
return isIndexed(propertyPath);
return isSearchable(propertyPath);
}

@Override
public boolean isAggregable(String[] propertyPath) {
return false;
}

@Override
public boolean isSortable(String[] propertyPath) {
return isIndexed(propertyPath);
return isSearchable(propertyPath);
}

@Override
Expand Down