Skip to content
This repository has been archived by the owner on Jun 7, 2021. It is now read-only.

[TRAFODION-3190] expression involving NULL should be treated as FALSE, not TRUE. #1741

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wenjun-zhu
Copy link
Contributor

No description provided.

@Traf-Jenkins
Copy link

Can one of the admins verify this patch?

@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3027/

@Traf-Jenkins
Copy link

Can one of the admins verify this patch?

1 similar comment
@Traf-Jenkins
Copy link

Can one of the admins verify this patch?

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3035/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3036/

@Traf-Jenkins
Copy link

Copy link
Contributor

@anoopsharma00 anoopsharma00 left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@DaveBirdsall DaveBirdsall left a comment

Choose a reason for hiding this comment

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

-1 I don't believe this fix is correct. See comments inline.

@@ -96,7 +96,7 @@ ex_expr::exp_return_type ex_branch_clause::eval(char *op_data[],
switch (getOperType())
{
case ITM_AND:
if (*(Lng32 *)op_data[1] == 0)
if (*(Lng32 *)op_data[1] == 0 || *(Lng32 *)op_data[1] == -1) // null treated as false
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe this is correct. Consider the query, "select a from t1x where not(b = 0 and c = 0)". When B and C are both null, both equal predicates evaluate to null, and the AND evaluates to null. The NOT then also evaluates to null. The WHERE clause should treat the result of the NOT as false. But with this fix, the result of the AND will be false, making the NOT true. There needs to be three cases here for ITM_AND: If the first operand is false, then the AND is false. If the first operand is true, then the result is the second operand. If the first operand is null, then if the second operand is false, the result is false otherwise the result is null. Similar logic needs to be added to the ITM_OR case.

@anoopsharma00
Copy link
Contributor

Dave is correct. Thanks for pointing it out.

Branch clause need to evaluate the second leg if the first leg is 'not false' which means
it is 'true or null'. With this change, it will branch out by treating null as false.

Similar comments were added to an earlier PR 1705 for this same jira
and those comments apply here as well.

For the original issue where CASE WHEN stmt was not proceeding to ELSE leg, a change
need to be added so boolean result is evaluated to at the very end of THEN clause.
At that time, NULL can be treated as FALSE and control to move to ELSE part.
This is same as a WHERE predicate where only at the end of predicate evaluation can
one determine if the result is true or false/null.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants