Skip to content

Commit

Permalink
Fixes faulty calculation of width in TableExportQueryConverter (#3597)
Browse files Browse the repository at this point in the history 
Fixes faulty calculation of width in TableExportQueryConverter by using unified method from TableExportQuery#calculateWidth

---------

Co-authored-by: Jonas Arnhold <[email protected]>
  • Loading branch information
@awildturtok @jnsrnhld
awildturtok and jnsrnhld authored Oct 10, 2024
1 parent c0ac392 commit 7c965e2
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 104 deletions.
64 changes: 31 additions & 33 deletions backend/src/main/java/com/bakdata/conquery/apiv1/query/TableExportQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.OptionalInt;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotEmpty;
Expand Down Expand Up @@ -55,6 +55,7 @@
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import lombok.extern.slf4j.Slf4j;


Expand All @@ -72,24 +73,30 @@
@Slf4j
@Getter
@Setter
@ToString(onlyExplicitlyIncluded = false)
@CPSType(id = "TABLE_EXPORT", base = QueryDescription.class)
@RequiredArgsConstructor(onConstructor_ = {@JsonCreator})
public class TableExportQuery extends Query {

@Valid
@NotNull
@NonNull
@ToString.Include
protected final Query query;

@NotNull
@ToString.Include
private Range<LocalDate> dateRange = Range.all();

@NotEmpty
@Valid
@ToString.Include
private List<CQConcept> tables;

/**
* @see TableExportQueryPlan#isRawConceptValues()
*/
@ToString.Include
private boolean rawConceptValues = true;

/**
Expand Down Expand Up @@ -121,13 +128,7 @@ public TableExportQueryPlan createQueryPlan(QueryPlanContext context) {
}
}

return new TableExportQueryPlan(
query.createQueryPlan(context),
CDateSet.create(CDateRange.of(dateRange)),
filterQueryNodes,
positions,
rawConceptValues
);
return new TableExportQueryPlan(query.createQueryPlan(context), CDateSet.create(CDateRange.of(dateRange)), filterQueryNodes, positions, rawConceptValues);
}

@Override
Expand All @@ -143,15 +144,6 @@ public void resolve(QueryResolveContext context) {
// First is dates, second is source id
final AtomicInteger currentPosition = new AtomicInteger(2);

secondaryIdPositions = calculateSecondaryIdPositions(currentPosition);

final Set<ValidityDate> validityDates = tables.stream()
.map(CQConcept::getTables)
.flatMap(Collection::stream)
.map(CQTable::findValidityDate)
.filter(Objects::nonNull)
.collect(Collectors.toSet());

// We need to know if a column is a concept column, so we can prioritize it, if it is also a SecondaryId
conceptColumns = tables.stream()
.map(CQConcept::getTables)
Expand All @@ -162,18 +154,25 @@ public void resolve(QueryResolveContext context) {
.filter(Objects::nonNull)
.collect(Collectors.toSet());

positions = calculateColumnPositions(currentPosition, tables, secondaryIdPositions, conceptColumns, validityDates);
secondaryIdPositions = calculateSecondaryIdPositions(currentPosition, conceptColumns, tables);

final Set<ValidityDate> validityDates =
tables.stream().map(CQConcept::getTables).flatMap(Collection::stream).map(CQTable::findValidityDate).filter(Objects::nonNull).collect(Collectors.toSet());


positions = calculateColumnPositions(currentPosition, tables, secondaryIdPositions, conceptColumns, validityDates);
}

private Map<SecondaryIdDescriptionId, Integer> calculateSecondaryIdPositions(AtomicInteger currentPosition) {
private static Map<SecondaryIdDescriptionId, Integer> calculateSecondaryIdPositions(
AtomicInteger currentPosition, Set<ColumnId> conceptColumns, List<CQConcept> tables) {
final Map<SecondaryIdDescriptionId, Integer> secondaryIdPositions = new HashMap<>();

// SecondaryIds are pulled to the front and grouped over all tables
tables.stream()
.flatMap(con -> con.getTables().stream())
.flatMap(table -> Arrays.stream(table.getConnector().resolve().getResolvedTable().getColumns()))
// Concept Columns are placed separately so they won't provide a secondaryId
.filter(Predicate.not(conceptColumns::contains))
.map(Column::getSecondaryId)
.filter(Objects::nonNull)
.map(SecondaryIdDescriptionId::resolve)
Expand All @@ -190,8 +189,7 @@ private static Map<ColumnId, Integer> calculateColumnPositions(
List<CQConcept> tables,
Map<SecondaryIdDescriptionId, Integer> secondaryIdPositions,
Collection<ColumnId> conceptColumns,
Collection<ValidityDate> validityDates
) {
Collection<ValidityDate> validityDates) {
final Map<ColumnId, Integer> positions = new HashMap<>();


Expand Down Expand Up @@ -232,11 +230,8 @@ public List<ResultInfo> getResultInfos() {

private List<ResultInfo> createResultInfos(Set<ColumnId> conceptColumns) {

OptionalInt max = positions.values().stream().mapToInt(i -> i).max();
if (max.isEmpty()) {
throw new IllegalStateException("Unable to determine maximum position");
}
final int size = max.getAsInt() + 1;
final int size = calculateWidth(positions);
;

final ResultInfo[] infos = new ResultInfo[size];

Expand All @@ -252,13 +247,12 @@ private List<ResultInfo> createResultInfos(Set<ColumnId> conceptColumns) {
}


final Map<Column, Concept<?>> connectorColumns =
tables.stream()
.flatMap(con -> con.getTables().stream())
.map(CQTable::getConnector)
.map(ConnectorId::resolve)
.filter(con -> con.getColumn() != null)
.collect(Collectors.toMap(con -> con.getColumn().resolve(), Connector::getConcept));
final Map<Column, Concept<?>> connectorColumns = tables.stream()
.flatMap(con -> con.getTables().stream())
.map(CQTable::getConnector)
.map(ConnectorId::resolve)
.filter(con -> con.getColumn() != null)
.collect(Collectors.toMap(con -> con.getColumn().resolve(), Connector::getConcept));


for (Map.Entry<ColumnId, Integer> entry : positions.entrySet()) {
Expand Down Expand Up @@ -304,6 +298,10 @@ private List<ResultInfo> createResultInfos(Set<ColumnId> conceptColumns) {
return List.of(infos);
}

public static int calculateWidth(Map<?, Integer> positions) {
return positions.values().stream().max(Integer::compareTo).orElse(0) + 1;
}

@Override
public void visit(Consumer<Visitable> visitor) {
visitor.accept(this);
Expand Down
3 changes: 2 additions & 1 deletion backend/src/main/java/com/bakdata/conquery/models/query/queryplan/TableExportQueryPlan.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Map;
import java.util.Optional;

import com.bakdata.conquery.apiv1.query.TableExportQuery;
import com.bakdata.conquery.apiv1.query.concept.filter.CQTable;
import com.bakdata.conquery.models.common.CDateSet;
import com.bakdata.conquery.models.common.daterange.CDateRange;
Expand Down Expand Up @@ -69,7 +70,7 @@ public Optional<MultilineEntityResult> execute(QueryExecutionContext ctx, Entity

final List<Object[]> results = new ArrayList<>();

final int totalColumns = positions.values().stream().mapToInt(i -> i).max().getAsInt() + 1;
final int totalColumns = TableExportQuery.calculateWidth(positions);
final String entityId = entity.getId();

for (Map.Entry<CQTable, QPNode> entry : tables.entrySet()) {
Expand Down
Loading

0 comments on commit 7c965e2

Please sign in to comment.