Skip to content

Conversation

@elliVM
Copy link
Contributor

@elliVM elliVM commented Dec 12, 2024

For #120 and #123
Clears up ElementCondition logic by splitting streaming and non streaming conditions into own classes.

Fixes issue where an error message was thrown if a recognized element name was encountered in a streaming query like <earliest>. These elements now result to a DSL.noCondition that functions as a "where is true" value in a query as a pass through condition, and will not have effect the query.

ConditionWalker is refactored not to negate a DSL.noCondition as it would result into a condition that is never true i.e. "where not true".

Exception is thrown if a unknown element name is encountered like <random>.

Refactor IndexStatement to not use a base condition from constructor as this was not needed.

@elliVM elliVM self-assigned this Dec 12, 2024
@elliVM elliVM requested a review from eemhu December 12, 2024 11:49
@elliVM elliVM requested a review from eemhu December 16, 2024 06:40
@elliVM elliVM force-pushed the element-condition-refactoring branch from f1627ab to bce62f0 Compare December 16, 2024 06:59
@elliVM
Copy link
Contributor Author

elliVM commented Dec 16, 2024

rebased

@elliVM elliVM requested a review from kortemik December 17, 2024 08:16
@kortemik
Copy link
Member

There are two changes in the same pr and therefore review process takes time. Please separate the exception fix and pr it first as it's a very simple change.


import java.util.Objects;

public final class StreamingCondition implements QueryCondition {
Copy link
Member

Choose a reason for hiding this comment

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

class name is misleading. all queries are micro-batch-streaming queries. this most likely refers streamdb query which is a non-time-range query where it's relevant pair journaldb query has time bounds. please refactor.

private final String operation;
private final ConditionConfig config;

public NonStreamingCondition(String tag, String value, String operation, ConditionConfig config) {
Copy link
Member

Choose a reason for hiding this comment

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

see comment on StreamingCondition

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.

3 participants