Fix bucket pruning when value is null and the predicate is 'IS NULL'.

[kaikalur]

When filtering in buckets with matching values, handle NULL properly.

== RELEASE NOTES ==
Fixed bucket pruning when the predicate is IS NULL and the table actually has rows with that value which we were previously not considering.

[mbasmanova]

@kaikalur The fix looks good, but the test should go into TestTupleDomain.

[kaikalur]

I disagree and I generally dislike unit tests. In fact, that function should not even be in there. They should have just called the other function that gives a map and extracted it. Too many ways to do the same thing is just bad.

[kaikalur]

I disagree and I generally dislike unit tests. In fact, that function should not even be in there. They should have just called the other function that gives a map and extracted it. Too many ways to do the same thing is just bad.

Also, what if someone found a way to do this bucket pruning different way and missed the same thing. We will miss it again. This test is good like here.

[bhhari]

one more test case
assertQuery(session, "SELECT COUNT() FROM table_with_null_buckets WHERE key IN (NULL,1)", "SELECT 1");

[kaikalur]

@kaikalur @mbasmanova the fix still fails for this case
assertQuery(session, "SELECT COUNT() FROM table_with_null_buckets WHERE key IN (NULL,1)", "SELECT 2");

It should be 1 because IN uses equality and key in (null,1) is same as key in (1). But good catch about missing test though!

[rongrong]

nits: space after comma in NULL,1.

[kaikalur]

fixed

[rongrong]

@kaikalur might be a good idea to also add unit test to TestTupleDomain in addition. Won't hurt.

[kaikalur]

@kaikalur might be a good idea to also add unit test to TestTupleDomain in addition. Won't hurt.

It hurts me to do it :) and I'm also principally against unit testing.

If you ask me, this function does not belong here in this class.

[kaikalur]

This is not good enough :(

        assertQuery(session, "SELECT COUNT() FROM table_with_null_buckets WHERE key IS NULL OR key = 1", "SELECT 2");

fails

[kaikalur]

@mbasmanova can you check if this new fix works better? It fixed the failed test. I couldn't find an API that gives me a value for null so just adding null when it's nullAllowed seems to have fixed it.

[mbasmanova]

@kaikalur The new fix looks fine, but this kind of low-level changes really required a unit test. Without one it is not possible to fully understand whether all the possible scenarios are implemented correctly.