[GLUTEN-5682][VL] Fix incorrect result when isNull & isNotNull coexist in filter

[zjuwangg]

What changes were proposed in this pull request?

Fix the bug founded in issue (Fixes: #5682)

How was this patch tested?

Add ut to cover the change!

[github-actions]

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

• Other pull requests

[FelixYBW]

Is it only a Spark UT issue? or observed in some SQL?

[zjuwangg]

Is it only a Spark UT issue? or observed in some SQL?

It's not just a Spark UT issue. We encounter this bug in our production environment causing result not mismatch.
We can reproduce this bug by executing following sql in VL module.
select l_orderkey from lineitem where l_comment is null and l_comment is not null

[github-actions]

#5682

[zjuwangg]

velox related change has been merged via facebookincubator/velox#9718

[philo-he]

Thanks for your fix! Just one comment. Thanks!

[philo-he]

Is forbidsNullSet_ logically equivalent to !nullAllowed? And isNullSet_ equivalent to isNull_? If true, we can just use the existing flags (maybe, should also check initialized_ == true?). Thanks!

[zjuwangg]

Actually not. initialized_ can be set other place not just here two null related setter method.

[zjuwangg]

Actually not. initialized_ can be set other place not just here two null related setter method.

So we can not according to initialized_ variable to determine whether setNull() or forbidsNull() has been called first.
And now setNull method will set nullAllowed_ to true that could cause unexpected filter behavior.

IMO, forbidsNullSet_ and isNullSet_ make code more clean and readable without other performance cost.

[philo-he]

Looks good! Thanks!