Skip to content

[multistage] Run ExpandSearch Rule After All Filter Pushdown Rules#10627

Merged
walterddr merged 2 commits intoapache:masterfrom
ankitsultana:fix-expand-search
Apr 18, 2023
Merged

[multistage] Run ExpandSearch Rule After All Filter Pushdown Rules#10627
walterddr merged 2 commits intoapache:masterfrom
ankitsultana:fix-expand-search

Conversation

@ankitsultana
Copy link
Contributor

@ankitsultana ankitsultana commented Apr 17, 2023

In #10409 I had added support for properly supporting predicate pushdown. However it introduced a bug where we could get "Range is not implemented yet" error because the filter pushdown rules could create a Sarg of their own.

This can be reproduced using the COLOCATED_JOIN Quickstart with the following query. It can also be reproduced using the unit-test case I have added.

select count(*) from userAttributes_OFFLINE 
  WHERE deviceOS NOT IN ('macos') 
  AND daysSinceFirstTrip > -4294967296 and daysSinceFirstTrip <= 10 
  AND (userUUID NOT IN (select userUUID FROM userGroups_OFFLINE WHERE groupUUID = 'group-1')) AND (userUUID NOT IN (SELECT userUUID FROM userGroups_OFFLINE WHERE groupUUID = 'group-2'))

cc: @walterddr

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2023

Codecov Report

Merging #10627 (170e48c) into master (27a8a26) will increase coverage by 44.08%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master   #10627       +/-   ##
=============================================
+ Coverage     24.49%   68.58%   +44.08%     
- Complexity       49     6498     +6449     
=============================================
  Files          2090     2106       +16     
  Lines        112578   113074      +496     
  Branches      16972    17042       +70     
=============================================
+ Hits          27578    77553    +49975     
+ Misses        82089    30001    -52088     
- Partials       2911     5520     +2609     
Flag Coverage Δ
integration1 24.44% <ø> (-0.05%) ⬇️
unittests1 68.02% <ø> (?)
unittests2 13.89% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1508 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

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

this actually make ense.

{
"psql": "7.2.1.3",
"description": "sub-query to multiple anti semi-join queries with star results, using NOT IN clause",
"sql": "SELECT * FROM {tbl1} WHERE (num > -10 and num < 10) AND (name NOT IN (SELECT val FROM {tbl2} WHERE num = 3)) AND (name NOT IN (SELECT val from {tbl2} WHERE num = 4))"
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add additional test for the range predicate also inside the NOT-IN subquery?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Can you check again?

@walterddr walterddr merged commit 45190cf into apache:master Apr 18, 2023
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