Merge pull request #2173 from bakdata/fix/secondId-external-empty-dates

fixes empty date ranges in secondaryID query with ExternalNode

backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java

@@ -11,6 +11,7 @@
 
 import com.bakdata.conquery.apiv1.query.concept.filter.CQTable;
 import com.bakdata.conquery.apiv1.query.concept.specific.CQConcept;
+import com.bakdata.conquery.apiv1.query.concept.specific.external.CQExternal;
 import com.bakdata.conquery.io.cps.CPSType;
 import com.bakdata.conquery.io.jackson.InternalOnly;
 import com.bakdata.conquery.io.jackson.serializer.NsIdRef;
@@ -77,17 +78,17 @@ public void collectRequiredQueries(Set<ManagedExecution<?>> requiredQueries) {
 	}
 
 	@Override
-	public void resolve(QueryResolveContext context) {
+	public void resolve(final QueryResolveContext context) {
 
 		DateAggregationMode resolvedDateAggregationMode = dateAggregationMode;
 		if (context.getDateAggregationMode() != null) {
 			log.trace("Overriding date aggregation mode ({}) with mode from context ({})", dateAggregationMode, context.getDateAggregationMode());
 			resolvedDateAggregationMode = context.getDateAggregationMode();
 		}
-		context = context.withDateAggregationMode(resolvedDateAggregationMode);
+		final QueryResolveContext resolvedContext = context.withDateAggregationMode(resolvedDateAggregationMode);
 
 		this.query = new ConceptQuery(root);
-		query.resolve(context);
+		query.resolve(resolvedContext);
 
 		withSecondaryId = new HashSet<>();
 		withoutSecondaryId = new HashSet<>();
@@ -98,6 +99,8 @@ public void resolve(QueryResolveContext context) {
 		// partition tables by their holding of the requested SecondaryId.
 		// This assumes that from the root, only ConceptNodes hold TableIds we are interested in.
 		query.visit(queryElement -> {
+			// We cannot check for CQExternal here and add the ALL_IDS Table because it is not serializable at the moment
+
 			if (!(queryElement instanceof CQConcept)) {
 				return;
 			}

backend/src/main/java/com/bakdata/conquery/apiv1/query/concept/specific/external/CQExternal.java

@@ -94,7 +94,7 @@ public QPNode createQueryPlan(QueryPlanContext context, ConceptQueryPlan plan) {
 		}
 
 		// Allocate at once, the maximum possible size
-		final Map<String, ConstantValueAggregator> extraAggregators = new HashMap<>(format.size());
+		final Map<String, ConstantValueAggregator<List<String>>> extraAggregators = new HashMap<>(format.size());
 
 		if (extra != null) {
 			for (int col = 0; col < format.size(); col++) {

backend/src/main/java/com/bakdata/conquery/models/query/queryplan/SecondaryIdQueryPlan.java

@@ -72,15 +72,18 @@ public Optional<MultilineEntityResult> execute(QueryExecutionContext ctx, Entity
 			return Optional.empty();
 		}
 
-		//first execute only tables with secondaryIds, creating all sub-queries
+		// First execute only tables with secondaryIds, creating all sub-queries
 		for (Column entry : tablesWithSecondaryId) {
 			executeQueriesWithSecondaryId(ctx, entity, entry);
 		}
-		//afterwards the remaining tables, since we now spawned all children
+		// Afterwards the remaining tables, since we now spawned all children
 		for (Table currentTable : tablesWithoutSecondaryId) {
 			executeQueriesWithoutSecondaryId(ctx, entity, currentTable);
 		}
 
+		// Do a last run with the ALL_IDS Table to trigger ExternalNodes
+		executeQueriesWithoutSecondaryId(ctx, entity, ctx.getStorage().getDataset().getAllIdsTable());
+
 		return createResult(entity);
 	}
 
@@ -117,7 +120,6 @@ private Optional<MultilineEntityResult> createResult(Entity entity) {
 	public void init(QueryExecutionContext ctx, Entity entity) {
 		queryPlan.init(ctx, entity);
 
-
 		// Dump the created children into reuse-pool
 		childPlanReusePool.addAll(childPerKey.values());
 

backend/src/main/java/com/bakdata/conquery/models/query/queryplan/aggregators/specific/ConstantValueAggregator.java

@@ -16,14 +16,14 @@
 @Getter
 @AllArgsConstructor
 @ToString
-public class ConstantValueAggregator extends Aggregator<Object> {
+public class ConstantValueAggregator<T> extends Aggregator<T> {
 
 	@Setter
-	private Object value;
+	private T value;
 	private final ResultType type;
 
 	@Override
-	public Object createAggregationResult() {
+	public T createAggregationResult() {
 		return value;
 	}
 

backend/src/main/java/com/bakdata/conquery/models/query/queryplan/specific/ExternalNode.java

@@ -8,9 +8,11 @@
 
 import javax.validation.constraints.NotEmpty;
 
+import com.bakdata.conquery.io.jackson.serializer.CDateSetDeserializer;
 import com.bakdata.conquery.models.common.CDateSet;
 import com.bakdata.conquery.models.datasets.Table;
 import com.bakdata.conquery.models.events.Bucket;
+import com.bakdata.conquery.models.externalservice.ResultType;
 import com.bakdata.conquery.models.query.QueryExecutionContext;
 import com.bakdata.conquery.models.query.entity.Entity;
 import com.bakdata.conquery.models.query.queryplan.QPNode;
@@ -24,18 +26,18 @@
 public class ExternalNode extends QPNode {
 
 	private final Table table;
-	private SpecialDateUnion dateUnion = new SpecialDateUnion();
+	private CDateSet dateUnion = CDateSet.create();
 
 	@NotEmpty
 	@NonNull
 	private final Map<Integer, CDateSet> includedEntities;
 
 	private final Map<Integer, Map<String, List<String>>> extraData;
-	private final Map<String, ConstantValueAggregator> extraAggregators;
+	private final Map<String, ConstantValueAggregator<List<String>>> extraAggregators;
 
 	private CDateSet contained;
 
-	public ExternalNode(Table table, Map<Integer, CDateSet> includedEntities, Map<Integer, Map<String, List<String>>> extraData, Map<String, ConstantValueAggregator> extraAggregators) {
+	public ExternalNode(Table table, Map<Integer, CDateSet> includedEntities, Map<Integer, Map<String, List<String>>> extraData, Map<String, ConstantValueAggregator<List<String>>> extraAggregators) {
 		this.includedEntities = includedEntities;
 		this.table = table;
 		this.extraData = extraData;
@@ -46,15 +48,15 @@ public ExternalNode(Table table, Map<Integer, CDateSet> includedEntities, Map<In
 	public void init(Entity entity, QueryExecutionContext context) {
 		super.init(entity, context);
 		contained = includedEntities.get(entity.getId());
-		dateUnion.init(entity, context);
+		dateUnion.clear();
 
-		for (ConstantValueAggregator aggregator : extraAggregators.values()) {
+		for (ConstantValueAggregator<?> aggregator : extraAggregators.values()) {
 			aggregator.setValue(null);
 		}
 
-		for (Map.Entry<String, ConstantValueAggregator> colAndAgg : extraAggregators.entrySet()) {
+		for (Map.Entry<String, ConstantValueAggregator<List<String>>> colAndAgg : extraAggregators.entrySet()) {
 			final String col = colAndAgg.getKey();
-			final ConstantValueAggregator agg = colAndAgg.getValue();
+			final ConstantValueAggregator<List<String>> agg = colAndAgg.getValue();
 
 			// Clear if entity has no value for the column
 			if (!extraData.getOrDefault(entity.getId(), Collections.emptyMap()).containsKey(col)) {
@@ -67,21 +69,17 @@ public void init(Entity entity, QueryExecutionContext context) {
 
 	@Override
 	public void nextTable(QueryExecutionContext ctx, Table currentTable) {
-		if (contained != null) {
-			CDateSet newSet = CDateSet.create(ctx.getDateRestriction());
-			newSet.retainAll(contained);
-			ctx = ctx.withDateRestriction(newSet);
-		}
-
 		super.nextTable(ctx, currentTable);
-		dateUnion.nextTable(getContext(), currentTable);
+
+		if (table.equals(currentTable) && contained != null){
+			dateUnion.addAll(contained);
+			dateUnion.retainAll(ctx.getDateRestriction());
+		}
 	}
 
 	@Override
 	public void acceptEvent(Bucket bucket, int event) {
-		if (contained != null) {
-			dateUnion.acceptEvent(bucket, event);
-		}
+		// Nothing to do
 	}
 
 	@Override
@@ -91,7 +89,7 @@ public boolean isContained() {
 
 	@Override
 	public Collection<Aggregator<CDateSet>> getDateAggregators() {
-		return Set.of(dateUnion);
+		return Set.of(new ConstantValueAggregator<>(dateUnion, new ResultType.ListT(ResultType.DateRangeT.INSTANCE)));
 	}
 
 	@Override

backend/src/test/resources/tests/query/SECONDARY_ID_EXTERNAL_LOGICAL/SECONDARY_IDS.test.json

@@ -0,0 +1,98 @@
+{
+	"type": "QUERY_TEST",
+	"label": "SECONDARY_ID Test",
+	"expectedCsv": "tests/query/SECONDARY_ID_EXTERNAL_LOGICAL/expected.csv",
+	"query": {
+		"type": "SECONDARY_ID_QUERY",
+		"root": {
+			"type": "AND",
+			"children": [
+				{
+					"type": "EXTERNAL",
+					"format": [
+						"ID",
+						"START_DATE",
+						"END_DATE"
+					],
+					"values": [
+						[
+							"pid",
+							"START_DATUM",
+							"END_DATUM"
+						],
+						[
+							"a",
+							"01.01.2016",
+							"30.06.2016"
+						]
+					]
+				},
+				{
+					"ids": [
+						"number"
+					],
+					"type": "CONCEPT",
+					"excludeFromSecondaryIdQuery": false,
+					"tables": [
+						{
+							"id": "number.number_connector"
+						}
+					]
+				}
+			]
+		},
+    "dateAggregationMode": "LOGICAL",
+    "secondaryId": "secondary"
+	},
+	"concepts": [
+		{
+			"label": "number",
+			"type": "TREE",
+			"connectors": [
+				{
+					"label": "number_connector",
+					"table": "table1",
+					"validityDates": {
+						"label": "datum",
+						"column": "table1.datum"
+					}
+				}
+			]
+		}
+	],
+	"content": {
+		"secondaryIds": [
+			{
+				"name": "secondary"
+			},
+			{
+				"name": "ignored"
+			}
+		],
+		"tables": [
+			{
+				"csv": "tests/query/SECONDARY_ID_EXTERNAL_LOGICAL/content.csv",
+				"name": "table1",
+				"primaryColumn": {
+					"name": "pid",
+					"type": "STRING"
+				},
+				"columns": [
+					{
+						"name": "sid",
+						"type": "STRING",
+						"secondaryId": "secondary"
+					},
+					{
+						"name": "value",
+						"type": "REAL"
+					},
+					{
+						"name": "datum",
+						"type": "DATE_RANGE"
+					}
+				]
+			}
+		]
+	}
+}

backend/src/test/resources/tests/query/SECONDARY_ID_EXTERNAL_LOGICAL/content.csv

@@ -0,0 +1,6 @@
+pid,sid,value,datum
+a,f_a1,1,"2016-06-30/2016-06-30"
+a,f_a2,1,"2014-06-30/2015-06-30"
+a,,1,"2010-06-30/2010-06-30"
+b,f_b1,1,"2015-02-03/2015-06-30"
+a,f_a1,1,"2014-06-30/2015-06-30"

backend/src/test/resources/tests/query/SECONDARY_ID_EXTERNAL_LOGICAL/expected.csv

@@ -0,0 +1,3 @@
+result,secondary,dates
+a,f_a1,{2016-06-30/2016-06-30}
+a,f_a2,{}

docs/Concept JSONs.md

@@ -685,19 +685,19 @@ Supported Fields:
 | [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/identifiable/NamedImpl.java#L17) | name | `String` | ? |  |  | 
 </p></details>
 
-### QUARTER<sup><sub><sup> [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/connector/specific/QuarterSelect.java#L14-L16)</sup></sub></sup>
+### QUARTER<sup><sub><sup> [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/concept/specific/QuarterSelect.java#L15-L17)</sup></sub></sup>
 Output first, last or random Year-Quarter in time
 
 <details><summary>Details</summary><p>
 
-Java Type: `com.bakdata.conquery.models.datasets.concepts.select.connector.specific.QuarterSelect`
+Java Type: `com.bakdata.conquery.models.datasets.concepts.select.concept.specific.QuarterSelect`
 
 Supported Fields:
 
 |  | Field | Type | Default | Example | Description |
 | --- | --- | --- | --- | --- | --- |
 | [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/Select.java#L35) | description | `String` | ? |  |  | 
-| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/connector/specific/QuarterSelect.java#L22) | sample | one of EARLIEST, LATEST, RANDOM | ? |  |  | 
+| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/concept/specific/QuarterSelect.java#L23) | sample | one of EARLIEST, LATEST, RANDOM | ? |  |  | 
 | [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/identifiable/Labeled.java#L25-L29) | label | `String` | ? | "someLabel" | shown in the frontend | 
 | [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/identifiable/NamedImpl.java#L17) | name | `String` | ? |  |  | 
 </p></details>
@@ -863,5 +863,6 @@ A Marker UniversalSelect is any of:
 * [EVENT_DATE_UNION](#EVENT_DATE_UNION)
 * [EVENT_DURATION_SUM](#EVENT_DURATION_SUM)
 * [EXISTS](#EXISTS)
+* [QUARTER](#QUARTER)
 
 </p></details>

docs/REST API JSONs.md

@@ -354,7 +354,7 @@ Supported Fields:
 | [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/models/forms/managed/RelativeFormQuery.java#L49) | timeUnit | one of DAYS, QUARTERS, YEARS | ? |  |  | 
 </p></details>
 
-### SECONDARY_ID_QUERY<sup><sub><sup> [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L36)</sup></sub></sup>
+### SECONDARY_ID_QUERY<sup><sub><sup> [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L37)</sup></sub></sup>
 
 
 <details><summary>Details</summary><p>
@@ -365,12 +365,12 @@ Supported Fields:
 
 |  | Field | Type | Default | Example | Description |
 | --- | --- | --- | --- | --- | --- |
-| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L49) | dateAggregationMode | one of NONE, MERGE, INTERSECT, LOGICAL | `"MERGE"` |  |  | 
-| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L53-L55) | query | [CONCEPT_QUERY](#CONCEPT_QUERY) | ␀ |  |  | 
-| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L42) | root | [@NotNull CQElement](#Base-CQElement) | `null` |  |  | 
-| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L45) | secondaryId | ID of `@NsIdRef @NotNull SecondaryIdDescription` | `null` |  |  | 
-| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L59) | withSecondaryId | list of ID of `@NsIdRefCollection Set<Column>` | ␀ |  |  | 
-| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L63) | withoutSecondaryId | list of ID of `@NsIdRefCollection Set<Table>` | ␀ |  |  | 
+| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L50) | dateAggregationMode | one of NONE, MERGE, INTERSECT, LOGICAL | `"MERGE"` |  |  | 
+| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L54-L56) | query | [CONCEPT_QUERY](#CONCEPT_QUERY) | ␀ |  |  | 
+| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L43) | root | [@NotNull CQElement](#Base-CQElement) | `null` |  |  | 
+| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L46) | secondaryId | ID of `@NsIdRef @NotNull SecondaryIdDescription` | `null` |  |  | 
+| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L60) | withSecondaryId | list of ID of `@NsIdRefCollection Set<Column>` | ␀ |  |  | 
+| [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/SecondaryIdQuery.java#L64) | withoutSecondaryId | list of ID of `@NsIdRefCollection Set<Table>` | ␀ |  |  | 
 </p></details>
 
 ### TABLE_EXPORT<sup><sub><sup> [✎](https://github.com/bakdata/conquery/edit/develop/backend/src/main/java/com/bakdata/conquery/apiv1/query/TableExportQuery.java#L49-L51)</sup></sub></sup>