Skip to content

chore(api): Introduce separation of EventFilter / AggregationFilter on API-level - #3931

Open
awildturtok wants to merge 4 commits into
developfrom
feature/ai/seperate-event-filter-and-aggregation-filters
Open

chore(api): Introduce separation of EventFilter / AggregationFilter on API-level#3931
awildturtok wants to merge 4 commits into
developfrom
feature/ai/seperate-event-filter-and-aggregation-filters

Conversation

@awildturtok

@awildturtok awildturtok commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Seperates EventFilter / AggregaionFilter on the API-class level

Co-Authored-By: CODEX

@awildturtok
awildturtok marked this pull request as ready for review August 4, 2026 12:53
@awildturtok
awildturtok requested a review from thoniTUB as a code owner August 4, 2026 12:53
@awildturtok awildturtok changed the title Introduce separation of EventFilter / AggregationFilter on API-level … chore(api): Introduce separation of EventFilter / AggregationFilter on API-level Aug 5, 2026
@awildturtok
awildturtok force-pushed the feature/ai/seperate-event-filter-and-aggregation-filters branch from 3c1ea93 to 8ea0439 Compare August 5, 2026 15:05
@awildturtok
awildturtok force-pushed the feature/ai/seperate-event-filter-and-aggregation-filters branch from 8ea0439 to 114b675 Compare August 5, 2026 15:08
public QPNode createQueryPlan(QueryPlanContext context, ConceptQueryPlan plan) {

final List<Aggregator<?>> conceptAggregators = createAggregators(plan, selects, context);
final List<Aggregator<?>> conceptAggregators = createAggregators(plan, selects, context.isDisableAggregators());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bubble up

Suggested change
final List<Aggregator<?>> conceptAggregators = createAggregators(plan, selects, context.isDisableAggregators());
final List<Aggregator<?>> conceptAggregators = context.isDisableAggregators() ? Collections.emptyList() : createAggregators(plan, selects);

@SuppressWarnings({"unchecked", "rawtypes"})
@Override
public FilterNode createFilterNode(Range.LongRange value) {
public AggregationResultFilterNode<?, ?> createFilterNode(Range.LongRange value) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ich würde Result droppen, da du die andere Klasse AggregationFilter genannt hast anstatt AggregationResultFilter

Suggested change
public AggregationResultFilterNode<?, ?> createFilterNode(Range.LongRange value) {
public AggregationFilterNode<?, ?> createFilterNode(Range.LongRange value) {

Comment on lines +70 to +80
@JsonIgnore
@ValidationMethod(message = "Columns do not match required Type.")
public boolean isValidColumnType() {

if (column != null) {
return ColumnUtils.assertValidColumnTypes(this, column, Set.of(MajorTypeId.DATE, DATE_RANGE));
}

return ColumnUtils.assertValidColumnTypes(this, startColumn, Set.of(MajorTypeId.DATE)) &&
ColumnUtils.assertValidColumnTypes(this, endColumn, Set.of(MajorTypeId.DATE));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Gleich bitte mal mit den Validierungen in DaterangeSelectOrFilter ab die hier auch applied werden sollten. Sieht für mich doppelt aus

Comment on lines 101 to 103
@JsonIgnore
@ValidationMethod(message = "Columns do not match required Type.")
public boolean isValidColumnType() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Kann wahrscheinlich auch gelöscht werden da es schon durch DaterangeSelectOrFilter abgedeckt ist

@Slf4j
@EqualsAndHashCode(callSuper = true)
public abstract class Filter<FILTER_VALUE> extends LabeledNamespaceIdentifiable<FilterId> {
public abstract sealed class Filter<FILTER_VALUE> extends LabeledNamespaceIdentifiable<FilterId>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Schick!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Das schöne ist sogar, dass ich das genau so machen wollte. Und Codex das von selbst so angewandt hat 🔥

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hattest du für codex ein coauthored gesetzt?

@awildturtok
awildturtok requested a review from thoniTUB August 6, 2026 12:13
@awildturtok
awildturtok enabled auto-merge (squash) August 6, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants