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

Pushdown domains with many ranges in Oracle connector #4918

Merged
merged 2 commits into from
Aug 25, 2020

Conversation

losipiuk
Copy link
Member

If domain has more that 1000 ranges it is simplified, before pushdown
attempt. Without this query would fails with:
ORA-01795: maximum number of expressions in a list is 1000

@@ -128,7 +128,7 @@ protected TestTable createTableWithDefaultColumns()
@Override
public void testLargeIn()
{
int numberOfElements = 1000;
int numberOfElements = 5000;
Copy link
Member

Choose a reason for hiding this comment

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

can we remove override now?

Copy link
Member

Choose a reason for hiding this comment

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

yes, this was overridden only to change the number of elements..

Copy link
Member Author

Choose a reason for hiding this comment

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

There is still this extra part in Oracle test which is not present in superclass:

        String arrayValues = range(0, numberOfElements)
                .mapToObj(i -> format("ARRAY[%s, %s, %s]", i, i + 1, i + 2))
                .collect(joining(", "));
        assertQuery("SELECT ARRAY[0, 0, 0] in (ARRAY[0, 0, 0], " + arrayValues + ")", "values true");
        assertQuery("SELECT ARRAY[0, 0, 0] in (" + arrayValues + ")", "values false");

I can call out to super anyway though, even if we still want test with arrays.

Copy link
Member

@eskabetxe eskabetxe Aug 24, 2020

Choose a reason for hiding this comment

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

@losipiuk that code isnt oracle especific, that method have that code before..
that was removed here 0c13d16

image

}
else {
// full pushdown
return new PredicatePushdownController.DomainPushdownResult(domain, Domain.all(domain.getType()));
Copy link
Member

Choose a reason for hiding this comment

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

You eliminate a filter here. Do we have a predicate pushdown test for Oracle, like we have for postgres?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

@losipiuk
Copy link
Member Author

ac

@losipiuk losipiuk force-pushed the lo/oracle-simplify-domain branch 3 times, most recently from ab0a9a9 to 422bd3c Compare August 24, 2020 18:46
If domain has more that 1000 ranges it is simplified, before pushdown
attempt. Without this query would fails with:
ORA-01795: maximum number of expressions in a list is 1000
@losipiuk
Copy link
Member Author

#4885

@losipiuk losipiuk merged commit caa67b8 into trinodb:master Aug 25, 2020
@losipiuk losipiuk mentioned this pull request Sep 3, 2020
9 tasks
@martint martint added this to the 341 milestone Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants