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

Implement ConceptColumnSelect conversion #3465

Merged
merged 14 commits into from
Jun 13, 2024
Merged

Conversation

jnsrnhld
Copy link
Collaborator

@jnsrnhld jnsrnhld commented Jun 6, 2024

Leider bietet HANA nicht die Möglichkeit einer array_agg(), daher musste ich auf eine string_agg zurückgreifen.

jnsrnhld and others added 8 commits May 28, 2024 17:16
…and-filter-conversion

# Conflicts:
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/Select.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/concept/specific/EventDateUnionSelect.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/concept/specific/EventDurationSumSelect.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/concept/specific/ExistsSelect.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/connector/specific/CountQuartersSelect.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/connector/specific/CountSelect.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/connector/specific/DateDistanceSelect.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/connector/specific/FlagSelect.java
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/select/connector/specific/SumSelect.java
@jnsrnhld jnsrnhld requested a review from awildturtok June 6, 2024 11:13
String alias,
SelectContext<TreeConcept, ConceptSqlTables> selectContext
) {
// a ConceptColumn select uses all connectors a Concept has, even if they are not part of the CQConcept
Copy link
Collaborator

Choose a reason for hiding this comment

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

Die entsprechenden Connectoren müssen schon ausgewhählt sein im CQTables.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Vlt verstehe ich da aber auch was falsch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@awildturtok Ah, dann habe ich das anscheinend falsch verstanden. Der Code hier https://github.com/bakdata/conquery-sql-backend/blob/sql-backend/backend/src/main/java/com/bakdata/conquery/models/query/queryplan/aggregators/specific/value/ConceptElementsAggregator.java#L40 hatte mich zur Annahme verleitet, dass ich immer alle Connectoren brauche, egal ob gewählt oder nicht.

Copy link
Collaborator

Choose a reason for hiding this comment

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

mh, aus dem code heraus hast du recht. Das ist aber nicht richtig so, ich schau es mir an.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, ich habs mir nochmal im Detail angeschaut, und es funktioniert schon so wie es soll: Es werden nur die ids Werte ausgegeben von auch ausgewählten Connectoren.

Der Grund warum das so ist, ist weil die Concept Aggregatoren einmalig erstellt werden und dann an alle ConceptNode angehängt werden. Die requiredTables sind hier etwas ungünstig implementiert, vlt Sorgen die auch dafür, dass wir uns mehr anschauen als wir müssen, aber die Ergebnisse werden dadurch nicht verfälscht.

Base automatically changed from sql/refactoring/rework-select-and-filter-conversion to develop June 13, 2024 07:34
jnsrnhld and others added 3 commits June 13, 2024 09:45
# Conflicts:
#	backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/filters/specific/BigMultiSelectFilter.java
#	backend/src/main/java/com/bakdata/conquery/sql/conversion/cqelement/concept/CQConceptConverter.java
#	backend/src/main/java/com/bakdata/conquery/sql/conversion/cqelement/concept/EventFilterCte.java
#	backend/src/main/java/com/bakdata/conquery/sql/conversion/model/aggregator/SumSqlAggregator.java
#	backend/src/main/java/com/bakdata/conquery/sql/conversion/model/select/ConnectorSqlSelects.java
@jnsrnhld jnsrnhld enabled auto-merge (squash) June 13, 2024 14:08
@jnsrnhld jnsrnhld merged commit 67ed3ee into develop Jun 13, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants