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

Jump to bottom

Fix predicate pushdown in export and other small fixes #3599

Merged
merged 7 commits into from Oct 26, 2023
Merged

Fix predicate pushdown in export and other small fixes #3599

merged 7 commits into from Oct 26, 2023

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Oct 25, 2023 •
edited

This is five fixes in one PR:

  1. Predicate pushdown did not work in v4.3; this supersedes my previous attempt in Fix predicate pushdown for export | where … #3590
  2. The catalog expected expressions to be already normalized and validated, which for non-normalized expressions caused all partitions to be returned
  3. We introduced a race in Fix serve exiting prematurely #3562 that caused operators to sometimes hang on shutdown; this mostly occurred in release builds with high-volume pipelines at the process boundary (no changelog entry for this one as we never released with this bug)
  4. The long option --skip-empty for read lines now works as expected
  5. We now include the feature flags in show build

@dominiklohmann dominiklohmann added bug Incorrect behavior engine Core pipeline and storage engine improvement An incremental enhancement of an existing feature labels Oct 25, 2023
@dominiklohmann dominiklohmann mentioned this pull request Oct 25, 2023
Closed
tobim
tobim approved these changes Oct 25, 2023
View reviewed changes
Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

The changes look correct. The CI failure needs to be investigated still.

@dominiklohmann dominiklohmann force-pushed the topic/predicate-pushdown branch 3 times, most recently from 36453f5 to c527abd Compare October 25, 2023 17:37
@dominiklohmann
Fix predicate pushdown in export
5df4b66
@dominiklohmann
Relax assumptions about expressions in the catalog
69fec98
@dominiklohmann
Fix race in exec nodes that caused an infinite loop
6f94f36 
This was a tricky one: With the recent changes to the behavior of await
in CAF that we backported, we had to switch to using `send` for the
internal run loop. That, however, unlike the direct scheduling on the
clock we used previously, can keep an actor alive. The scheduling of a
new run that this commit removes was unnecessary, and in fact actively
harmful as it led to an infinite loop during shutdown of an operator
when the timing was just right.
@dominiklohmann
Align --skip-empty option for read lines with docs
702e0df
@dominiklohmann
Add feature flags to show build
effd790
@dominiklohmann
Fix concept resolution in export
4cda9aa
@dominiklohmann
Update pipeline manager test references
9aabae6 
The expected debug representation for an optimized `export | where …`
changed with this PR, so we must also update the references accordingly.
@dominiklohmann dominiklohmann merged commit 19d3faf into main Oct 26, 2023
40 checks passed
@dominiklohmann dominiklohmann deleted the topic/predicate-pushdown branch October 26, 2023 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobim tobim tobim approved these changes

Assignees
No one assigned
Labels
bug Incorrect behavior engine Core pipeline and storage engine improvement An incremental enhancement of an existing feature
Projects
None yet
Milestone
No milestone
2 participants
@dominiklohmann @tobim