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.

Sign up for GitHub

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

truncate positive infinity to today, don't output if negative infinity is part of a count #2820

Merged
merged 4 commits into from
Nov 24, 2022
Merged

truncate positive infinity to today, don't output if negative infinity is part of a count #2820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3709c6b
truncate positive infinity to today, don't output if negative infinit…
awildturtok Nov 23, 2022
7a53075
adds test for truncateMax
awildturtok Nov 23, 2022
0d6b0d7
adds shorthand for open CDateRange in DateReader
awildturtok Nov 23, 2022
4d24f69
adds testcase for atMost and DurationSum
awildturtok Nov 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions backend/src/main/java/com/bakdata/conquery/models/common/CDateSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,11 @@ private void putRange(CDateRange range) {
}


public void maskedAdd(CDateRange toAdd, CDateSet mask){
public void maskedAdd(CDateRange toAdd, CDateSet mask) {
maskedAdd(toAdd, mask, CDateRange.POSITIVE_INFINITY);
}

public void maskedAdd(CDateRange toAdd, CDateSet mask, int truncateMax){
if(mask.isEmpty()){
return;
}
Expand Down Expand Up @@ -317,27 +321,29 @@ public void maskedAdd(CDateRange toAdd, CDateSet mask){

search = mask.rangesByLowerBound.higherKey(search);

int min = range.getMinValue();
int max = range.getMaxValue();
int lowerBound = range.getMinValue();
int upperBound = range.getMaxValue();

if(max < toAdd.getMinValue()){
if(upperBound < toAdd.getMinValue()){
continue;
}

if(min < toAdd.getMinValue()){
min = toAdd.getMinValue();
if(lowerBound < toAdd.getMinValue()){
lowerBound = toAdd.getMinValue();
}

if(max > toAdd.getMaxValue()){
max = toAdd.getMaxValue();
if(upperBound > toAdd.getMaxValue()){
upperBound = toAdd.getMaxValue();
}

upperBound = Math.min(upperBound, truncateMax);

// value was not contained
if(min > max){
if(lowerBound > upperBound){
continue;
}

add(CDateRange.of(min, max));
add(CDateRange.of(lowerBound, upperBound));
}
}

Expand Down
6 changes: 5 additions & 1 deletion backend/src/main/java/com/bakdata/conquery/models/query/QueryExecutionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
package com.bakdata.conquery.models.query;

import java.time.LocalDate;
import java.util.List;
import java.util.Optional;

import com.bakdata.conquery.io.storage.ModificationShieldedWorkerStorage;
import com.bakdata.conquery.models.common.CDate;
import com.bakdata.conquery.models.common.CDateSet;
import com.bakdata.conquery.models.datasets.concepts.Connector;
import com.bakdata.conquery.models.datasets.Column;
import com.bakdata.conquery.models.datasets.SecondaryIdDescription;
import com.bakdata.conquery.models.datasets.Table;
import com.bakdata.conquery.models.datasets.concepts.Connector;
import com.bakdata.conquery.models.events.Bucket;
import com.bakdata.conquery.models.events.BucketManager;
import com.bakdata.conquery.models.identifiable.ids.specific.ManagedExecutionId;
Expand Down Expand Up @@ -44,6 +46,8 @@ public class QueryExecutionContext {
*/
private SecondaryIdDescription activeSecondaryId = null;

private final int today = CDate.ofLocalDate(LocalDate.now());

public List<Bucket> getEntityBucketsForTable(Entity entity, Table table) {
return bucketManager.getEntityBucketsForTable(entity, table);
}
Expand Down
11 changes: 5 additions & 6 deletions ...com/bakdata/conquery/models/query/queryplan/aggregators/specific/DateUnionAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ public class DateUnionAggregator extends SingleColumnAggregator<CDateSet> {
private CDateSet set = CDateSet.create();
private CDateSet dateRestriction;

private int realUpperBound;

public DateUnionAggregator(Column column) {
super(column);
}

@Override
public void init(Entity entity, QueryExecutionContext context) {
set.clear();
realUpperBound = context.getToday();
}

@Override
Expand All @@ -40,13 +43,9 @@ public void acceptEvent(Bucket bucket, int event) {
return;
}

CDateRange value = bucket.getAsDateRange(event, getColumn());
//otherwise the result would be something weird
if (value.isOpen()) {
return;
}
final CDateRange value = bucket.getAsDateRange(event, getColumn());

set.maskedAdd(value, dateRestriction);
set.maskedAdd(value, dateRestriction, realUpperBound);
}

@Override
Expand Down
16 changes: 9 additions & 7 deletions ...m/bakdata/conquery/models/query/queryplan/aggregators/specific/DurationSumAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.bakdata.conquery.models.query.queryplan.aggregators.specific;

import com.bakdata.conquery.models.common.CDate;
import com.bakdata.conquery.models.common.CDateSet;
import com.bakdata.conquery.models.common.daterange.CDateRange;
import com.bakdata.conquery.models.datasets.Column;
Expand All @@ -20,13 +21,16 @@ public class DurationSumAggregator extends SingleColumnAggregator<Long> {
private CDateSet set = CDateSet.create();
private CDateSet dateRestriction;

private int realUpperBound;

public DurationSumAggregator(Column column) {
super(column);
}

@Override
public void init(Entity entity, QueryExecutionContext context) {
set.clear();
realUpperBound = context.getToday();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich denke nicht, dass das das ist was wir wollen, so kann eine Anfrage von gestern schon ganz andere Ergebnisse haben wenn sie Heute oder demnächst wiederverwendet wird.

Um das zu verhindern, sollte die upperBound mit persistiert werden, bzw. eine DateRestiction gesetzt sein. Sonst sehe ich Probleme bei der Reproduzierbarkeit von Ergebnissen

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das machen wir bei DateDistance genauso, und ist auch genau das Ziel des PRs

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jap, ich weiß, aber das könnte Probleme mit sich bringen

}

@Override
Expand All @@ -42,17 +46,15 @@ public void acceptEvent(Bucket bucket, int event) {

final CDateRange value = bucket.getAsDateRange(event, getColumn());

if (value.isOpen()) {
return;
}


set.maskedAdd(value, dateRestriction);
set.maskedAdd(value, dateRestriction, realUpperBound);
}

@Override
public Long createAggregationResult() {
return set.isEmpty() ? null : set.countDays();
if (set.isEmpty() || CDate.isNegativeInfinity(set.getMinValue())) {
return null;
}
return set.countDays();
}

@Override
Expand Down
11 changes: 4 additions & 7 deletions ...data/conquery/models/query/queryplan/aggregators/specific/EventDurationSumAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,18 @@
@ToString(onlyExplicitlyIncluded = true)
public class EventDurationSumAggregator extends Aggregator<Long> {

private Optional<Aggregator<CDateSet>> queryDateAggregator = Optional.empty();
private final CDateSet set = CDateSet.create();

private Optional<Aggregator<CDateSet>> queryDateAggregator = Optional.empty();
@CheckForNull
private CDateSet dateRestriction;
@CheckForNull
private Column validityDateColumn;
private int realUpperBound;

@Override
public void init(Entity entity, QueryExecutionContext context) {
set.clear();
realUpperBound = context.getToday();
}

@Override
Expand All @@ -53,12 +54,8 @@ public void acceptEvent(Bucket bucket, int event) {

final CDateRange value = bucket.getAsDateRange(event, validityDateColumn);

if (value.isOpen()) {
return;
}


set.maskedAdd(value, dateRestriction);
set.maskedAdd(value, dateRestriction, realUpperBound);
}

@Override
Expand Down
21 changes: 13 additions & 8 deletions ...a/com/bakdata/conquery/models/query/queryplan/aggregators/specific/QuarterAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public class QuarterAggregator extends Aggregator<String> {

private Column column;

private int realUpperBound;

public QuarterAggregator(TemporalSamplerFactory samplerFactory) {
this.samplerFactory = samplerFactory;
}
Expand All @@ -41,6 +43,7 @@ public QuarterAggregator(TemporalSamplerFactory samplerFactory) {
public void init(Entity entity, QueryExecutionContext context) {
set.clear();
sampler = samplerFactory.sampler();
realUpperBound = context.getToday();
}

@Override
Expand All @@ -57,11 +60,7 @@ public void acceptEvent(Bucket bucket, int event) {

final CDateRange value = bucket.getAsDateRange(event, getColumn());

if (value.isOpen()) {
return;
}

set.maskedAdd(value, dateRestriction);
set.maskedAdd(value, dateRestriction, realUpperBound);
}

@Override
Expand All @@ -70,13 +69,19 @@ public String createAggregationResult() {
return null;
}

final OptionalInt sampled = sampler.sample(set);
final OptionalInt maybeSampled = sampler.sample(set);

if (maybeSampled.isEmpty()) {
return null;
}

final int sampled = maybeSampled.getAsInt();

if (sampled.isEmpty()) {
if (CDate.isNegativeInfinity(sampled) || CDate.isPositiveInfinity(sampled)) {
return null;
}

final LocalDate date = CDate.toLocalDate(sampled.getAsInt());
final LocalDate date = CDate.toLocalDate(sampled);
final int quarter = QuarterUtils.getQuarter(date);
final int year = date.getYear();

Expand Down
11 changes: 5 additions & 6 deletions ...va/com/bakdata/conquery/models/query/queryplan/aggregators/specific/SpecialDateUnion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,20 @@ public void nextTable(QueryExecutionContext ctx, Table table) {

@Override
public void acceptEvent(Bucket bucket, int event) {
if (currentColumn != null && bucket.has(event, currentColumn)) {
set.maskedAdd(bucket.getAsDateRange(event, currentColumn), dateRestriction);
if (currentColumn == null || !bucket.has(event, currentColumn)) {
set.addAll(dateRestriction);
return;
}

if(!dateRestriction.isEmpty()) {
set.addAll(dateRestriction);
}
set.maskedAdd(bucket.getAsDateRange(event, currentColumn), dateRestriction);
}

/**
* Helper method to insert dates from outside.
*
* @param other CDateSet to be included.
*/
public void merge(CDateSet other){
public void merge(CDateSet other) {
set.addAll(other);
}

Expand Down
Loading