Merge pull request #2709 from bakdata/hotfix/revert-multi-column-dist…

…inct Hotfix/revert multi column distinct
ingef · Sep 6, 2022 · 4369fd5 · 4369fd5
2 parents 2beee33 + cfcdf1e
commit 4369fd5
Show file tree

Hide file tree

Showing 13 changed files with 243 additions and 306 deletions.
diff --git a/...main/java/com/bakdata/conquery/models/datasets/concepts/filters/specific/CountFilter.java b/...main/java/com/bakdata/conquery/models/datasets/concepts/filters/specific/CountFilter.java
@@ -1,38 +1,53 @@
 package com.bakdata.conquery.models.datasets.concepts.filters.specific;
 
-import java.util.List;
+import java.util.Objects;
+import java.util.stream.Stream;
 
 import javax.validation.Valid;
-import javax.validation.constraints.NotEmpty;
+import javax.validation.constraints.NotNull;
 
 import com.bakdata.conquery.apiv1.frontend.FEFilterConfiguration;
 import com.bakdata.conquery.apiv1.frontend.FEFilterType;
 import com.bakdata.conquery.io.cps.CPSType;
+import com.bakdata.conquery.io.jackson.serializer.NsIdRef;
 import com.bakdata.conquery.io.jackson.serializer.NsIdRefCollection;
 import com.bakdata.conquery.models.common.Range;
 import com.bakdata.conquery.models.datasets.Column;
 import com.bakdata.conquery.models.datasets.concepts.filters.Filter;
 import com.bakdata.conquery.models.query.filter.RangeFilterNode;
 import com.bakdata.conquery.models.query.queryplan.aggregators.DistinctValuesWrapperAggregator;
+import com.bakdata.conquery.models.query.queryplan.aggregators.MultiDistinctValuesWrapperAggregator;
 import com.bakdata.conquery.models.query.queryplan.aggregators.specific.CountAggregator;
 import com.bakdata.conquery.models.query.queryplan.filter.FilterNode;
-import com.fasterxml.jackson.annotation.JsonIgnore;
-import io.dropwizard.validation.ValidationMethod;
-import lombok.Data;
-import lombok.NoArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.commons.lang3.ArrayUtils;
 
+/**
+ * This filter represents a select in the front end. This means that the user can select one or more values from a list of values.
+ */
+@Getter
+@Setter
 @CPSType(id = "COUNT", base = Filter.class)
-@NoArgsConstructor
-@Data
 public class CountFilter extends Filter<Range.LongRange> {
 
 	@Valid
-	@NotEmpty
-	@NsIdRefCollection
-	private List<Column> column;
+	@NotNull
+	@Getter
+	@Setter
+	@NsIdRef
+	private Column column;
 
 	private boolean distinct;
 
+	// todo FK: don't think the array notation is used anywhere. Del?
+	@Valid
+	@Getter
+	@Setter
+	@NsIdRefCollection
+	private Column[] distinctByColumn;
+
+
 	@Override
 	public void configureFrontend(FEFilterConfiguration.Top f) {
 		f.setType(FEFilterType.Fields.INTEGER_RANGE);
@@ -43,24 +58,28 @@ public void configureFrontend(FEFilterConfiguration.Top f) {
 	@SuppressWarnings({"unchecked", "rawtypes"})
 	@Override
 	public FilterNode createFilterNode(Range.LongRange value) {
-		if (distinct) {
-			return new RangeFilterNode(value, new DistinctValuesWrapperAggregator(new CountAggregator(), getColumn()));
+		if (distinct || distinctByColumn != null) {
+			if (ArrayUtils.isEmpty(distinctByColumn) || distinctByColumn.length < 2) {
+				return new RangeFilterNode(
+						value,
+						new DistinctValuesWrapperAggregator(
+								new CountAggregator(getColumn()),
+								ArrayUtils.isEmpty(getDistinctByColumn()) ?
+								getColumn()
+																		  :
+								getDistinctByColumn()[0]
+						)
+				);
+			}
+			return new RangeFilterNode(value, new MultiDistinctValuesWrapperAggregator(new CountAggregator(getColumn()), getDistinctByColumn()));
 		}
-		return new RangeFilterNode(value, new CountAggregator(getColumn().get(0)));
+		return new RangeFilterNode(value, new CountAggregator(getColumn()));
 	}
 
 	@Override
 	public Column[] getRequiredColumns() {
-		return getColumn().toArray(Column[]::new);
-	}
-
-	@JsonIgnore
-	@ValidationMethod(message = "Cannot use multiple columns, when distinct is not set.")
-	public boolean isMultiOnlyWhenDistinct() {
-		if(!isDistinct()){
-			return getColumn().size() == 1;
-		}
-
-		return true;
+		return Stream.of(getColumn(), (distinct && !ArrayUtils.isEmpty(distinctByColumn)) ? distinctByColumn[0] : null)
+					 .filter(Objects::nonNull)
+					 .toArray(Column[]::new);
 	}
 }
diff --git a/...c/main/java/com/bakdata/conquery/models/datasets/concepts/filters/specific/SumFilter.java b/...c/main/java/com/bakdata/conquery/models/datasets/concepts/filters/specific/SumFilter.java
@@ -1,17 +1,15 @@
 package com.bakdata.conquery.models.datasets.concepts.filters.specific;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.Objects;
+import java.util.stream.Stream;
 
-import javax.annotation.Nullable;
+import javax.validation.Valid;
 import javax.validation.constraints.NotNull;
 
 import com.bakdata.conquery.apiv1.frontend.FEFilterConfiguration;
 import com.bakdata.conquery.apiv1.frontend.FEFilterType;
 import com.bakdata.conquery.io.cps.CPSType;
 import com.bakdata.conquery.io.jackson.serializer.NsIdRef;
-import com.bakdata.conquery.io.jackson.serializer.NsIdRefCollection;
 import com.bakdata.conquery.models.common.IRange;
 import com.bakdata.conquery.models.common.Range;
 import com.bakdata.conquery.models.datasets.Column;
@@ -30,35 +28,43 @@
 import com.bakdata.conquery.models.query.queryplan.aggregators.specific.sum.MoneySumAggregator;
 import com.bakdata.conquery.models.query.queryplan.aggregators.specific.sum.RealSumAggregator;
 import com.bakdata.conquery.models.query.queryplan.filter.FilterNode;
-import com.fasterxml.jackson.annotation.JsonIgnore;
-import lombok.Data;
-import lombok.NoArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 
 /**
  * This filter represents a filter on the sum of one integer column.
  */
+@Getter
+@Setter
 @Slf4j
-@NoArgsConstructor
-@Data
 @CPSType(id = "SUM", base = Filter.class)
 public class SumFilter<RANGE extends IRange<? extends Number, ?>> extends Filter<RANGE> {
 
+
+	@Valid
 	@NotNull
+	@Getter
+	@Setter
 	@NsIdRef
 	private Column column;
 
+	@Valid
+	@Getter
+	@Setter
 	@NsIdRef
-	@Nullable
 	private Column subtractColumn;
 
-	@NsIdRefCollection
-	@NotNull
-	private List<Column> distinctByColumn = Collections.emptyList();
+	@Valid
+	@Getter
+	@Setter
+	@NsIdRef
+	private Column distinctByColumn;
 
 	@Override
 	public void configureFrontend(FEFilterConfiguration.Top f) throws ConceptConfigurationException {
-		switch (getColumn().getType()) {
+		Column column = getColumn();
+		switch (column.getType()) {
 			case MONEY:
 				f.setType(FEFilterType.Fields.MONEY_RANGE);
 				return;
@@ -71,40 +77,32 @@ public void configureFrontend(FEFilterConfiguration.Top f) throws ConceptConfigu
 				return;
 			}
 			default:
-				throw new ConceptConfigurationException(getConnector(), "NUMBER filter is incompatible with columns of type " + getColumn().getType());
+				throw new ConceptConfigurationException(getConnector(), "NUMBER filter is incompatible with columns of type " + column.getType());
 		}
 	}
 
 	@Override
 	public Column[] getRequiredColumns() {
-		List<Column> out = new ArrayList<>();
-
-		out.add(getColumn());
-		out.addAll(getDistinctByColumn());
-
-		if(getSubtractColumn() != null){
-			out.add(getSubtractColumn());
-		}
-
-		return out.toArray(Column[]::new);
+		return Stream.of(getColumn(), getSubtractColumn(), getDistinctByColumn())
+					 .filter(Objects::nonNull)
+					 .toArray(Column[]::new);
 	}
 
 	@Override
 	public FilterNode createFilterNode(RANGE value) {
-		IRange<? extends Number, ?> range = value;
+		ColumnAggregator<?> aggregator = getAggregator();
 
-		if (getColumn().getType() == MajorTypeId.REAL) {
-			range = Range.DoubleRange.fromNumberRange(value);
+		if (distinctByColumn != null) {
+			return new RangeFilterNode(value, new DistinctValuesWrapperAggregator(aggregator, getDistinctByColumn()));
 		}
 
-		if (!distinctByColumn.isEmpty()) {
-			return new RangeFilterNode(range, new DistinctValuesWrapperAggregator(getAggregator(), getDistinctByColumn()));
+		if (getColumn().getType() == MajorTypeId.REAL) {
+			return new RangeFilterNode(Range.DoubleRange.fromNumberRange(value), aggregator);
 		}
 
-		return new RangeFilterNode(range, getAggregator());
+		return new RangeFilterNode(value, aggregator);
 	}
 
-	@JsonIgnore
 	private ColumnAggregator<?> getAggregator() {
 		if (getSubtractColumn() == null) {
 			switch (getColumn().getType()) {

diff --git a/.../com/bakdata/conquery/models/datasets/concepts/select/connector/specific/CountSelect.java b/.../com/bakdata/conquery/models/datasets/concepts/select/connector/specific/CountSelect.java
@@ -1,56 +1,54 @@
 package com.bakdata.conquery.models.datasets.concepts.select.connector.specific;
 
-import java.util.List;
+import java.util.Objects;
+import java.util.stream.Stream;
 
 import javax.validation.constraints.NotNull;
 
 import com.bakdata.conquery.io.cps.CPSType;
-import com.bakdata.conquery.io.jackson.serializer.NsIdRefCollection;
+import com.bakdata.conquery.io.jackson.serializer.NsIdRef;
 import com.bakdata.conquery.models.datasets.Column;
 import com.bakdata.conquery.models.datasets.concepts.select.Select;
 import com.bakdata.conquery.models.query.queryplan.aggregators.Aggregator;
 import com.bakdata.conquery.models.query.queryplan.aggregators.DistinctValuesWrapperAggregator;
 import com.bakdata.conquery.models.query.queryplan.aggregators.specific.CountAggregator;
-import com.fasterxml.jackson.annotation.JsonIgnore;
-import io.dropwizard.validation.ValidationMethod;
-import lombok.Data;
+import lombok.Getter;
 import lombok.NoArgsConstructor;
+import lombok.Setter;
 import org.jetbrains.annotations.Nullable;
 
 @CPSType(id = "COUNT", base = Select.class)
 @NoArgsConstructor
-@Data
 public class CountSelect extends Select {
 
+	@Getter
+	@Setter
 	private boolean distinct = false;
 
-	@NsIdRefCollection
-	@NotNull
-	private List<Column> column;
+	@Getter
+	@Setter
+	@NsIdRef
+	private Column distinctByColumn;
 
+	@Getter
+	@Setter
+	@NsIdRef
+	@NotNull
+	private Column column;
 
 	@Override
 	public Aggregator<?> createAggregator() {
-		if (distinct) {
-			return new DistinctValuesWrapperAggregator<>(new CountAggregator(), getColumn());
+		if (distinct || distinctByColumn != null) {
+			return new DistinctValuesWrapperAggregator<>(new CountAggregator(getColumn()), getDistinctByColumn() == null ? getColumn() : getDistinctByColumn());
 		}
-		return new CountAggregator(getColumn().get(0));
+		return new CountAggregator(getColumn());
 	}
 
 	@Nullable
 	@Override
 	public Column[] getRequiredColumns() {
-		return getColumn().toArray(Column[]::new);
-	}
-
-
-	@JsonIgnore
-	@ValidationMethod(message = "Cannot use multiple columns, when distinct is not set.")
-	public boolean isMultiOnlyWhenDistinct() {
-		if(!isDistinct()){
-			return getColumn().size() == 1;
-		}
-
-		return true;
+		return Stream.of(getColumn(), getDistinctByColumn())
+					 .filter(Objects::nonNull)
+					 .toArray(Column[]::new);
 	}
 }