Skip to content

Commit e1a8b0f

Browse files
committed
Fix CASE bug identified by Keith Parks: CASE didn't reliably
treat a NULL condition result as FALSE. Clean up some bogus comments here and there, too.
1 parent f9f5dfb commit e1a8b0f

File tree

1 file changed

+16
-31
lines changed

1 file changed

+16
-31
lines changed

src/backend/executor/execQual.c

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.58 1999/07/19 00:26:15 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.59 1999/09/18 23:26:37 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -616,8 +616,7 @@ ExecEvalFuncArgs(FunctionCachePtr fcache,
616616
bool *argIsDone)
617617
{
618618
int i;
619-
bool argIsNull,
620-
*nullVect;
619+
bool *nullVect;
621620
List *arg;
622621

623622
nullVect = fcache->nullVect;
@@ -631,23 +630,17 @@ ExecEvalFuncArgs(FunctionCachePtr fcache,
631630
* as arguments but we make an exception in the case of nested dot
632631
* expressions. We have to watch out for this case here.
633632
*/
634-
argV[i] = (Datum)
635-
ExecEvalExpr((Node *) lfirst(arg),
636-
econtext,
637-
&argIsNull,
638-
argIsDone);
639-
633+
argV[i] = ExecEvalExpr((Node *) lfirst(arg),
634+
econtext,
635+
& nullVect[i],
636+
argIsDone);
640637

641638
if (!(*argIsDone))
642639
{
643640
Assert(i == 0);
644641
fcache->setArg = (char *) argV[0];
645642
fcache->hasSetArg = true;
646643
}
647-
if (argIsNull)
648-
nullVect[i] = true;
649-
else
650-
nullVect[i] = false;
651644
i++;
652645
}
653646
}
@@ -1108,7 +1101,7 @@ ExecEvalAnd(Expr *andExpr, ExprContext *econtext, bool *isNull)
11081101
* ExecEvalCase
11091102
*
11101103
* Evaluate a CASE clause. Will have boolean expressions
1111-
* inside the WHEN clauses, and will have constants
1104+
* inside the WHEN clauses, and will have expressions
11121105
* for results.
11131106
* - thomas 1998-11-09
11141107
* ----------------------------------------------------------------
@@ -1118,7 +1111,6 @@ ExecEvalCase(CaseExpr *caseExpr, ExprContext *econtext, bool *isNull)
11181111
{
11191112
List *clauses;
11201113
List *clause;
1121-
CaseWhen *wclause;
11221114
Datum const_value = 0;
11231115
bool isDone;
11241116

@@ -1127,34 +1119,33 @@ ExecEvalCase(CaseExpr *caseExpr, ExprContext *econtext, bool *isNull)
11271119
/*
11281120
* we evaluate each of the WHEN clauses in turn, as soon as one is
11291121
* true we return the corresponding result. If none are true then we
1130-
* return the value of the default clause, or NULL.
1122+
* return the value of the default clause, or NULL if there is none.
11311123
*/
11321124
foreach(clause, clauses)
11331125
{
1126+
CaseWhen *wclause = lfirst(clause);
11341127

11351128
/*
11361129
* We don't iterate over sets in the quals, so pass in an isDone
11371130
* flag, but ignore it.
11381131
*/
1139-
1140-
wclause = lfirst(clause);
11411132
const_value = ExecEvalExpr((Node *) wclause->expr,
11421133
econtext,
11431134
isNull,
11441135
&isDone);
11451136

11461137
/*
11471138
* if we have a true test, then we return the result, since the
1148-
* case statement is satisfied.
1139+
* case statement is satisfied. A NULL result from the test is
1140+
* not considered true.
11491141
*/
1150-
if (DatumGetInt32(const_value) != 0)
1142+
if (DatumGetInt32(const_value) != 0 && ! *isNull)
11511143
{
11521144
const_value = ExecEvalExpr((Node *) wclause->result,
11531145
econtext,
11541146
isNull,
11551147
&isDone);
11561148
return (Datum) const_value;
1157-
11581149
}
11591150
}
11601151

@@ -1318,28 +1309,22 @@ ExecQualClause(Node *clause, ExprContext *econtext)
13181309
bool isNull;
13191310
bool isDone;
13201311

1321-
/* when there is a null clause, consider the qualification to be true */
1312+
/* when there is a null clause, consider the qualification to fail */
13221313
if (clause == NULL)
13231314
return true;
13241315

13251316
/*
13261317
* pass isDone, but ignore it. We don't iterate over multiple returns
13271318
* in the qualifications.
13281319
*/
1329-
expr_value = (Datum)
1330-
ExecEvalExpr(clause, econtext, &isNull, &isDone);
1320+
expr_value = ExecEvalExpr(clause, econtext, &isNull, &isDone);
13311321

13321322
/*
1333-
* this is interesting behaviour here. When a clause evaluates to
1334-
* null, then we consider this as passing the qualification. it seems
1335-
* kind of like, if the qual is NULL, then there's no qual..
1323+
* remember, we return true when the qualification fails;
1324+
* NULL is considered failure.
13361325
*/
13371326
if (isNull)
13381327
return true;
1339-
1340-
/*
1341-
* remember, we return true when the qualification fails..
1342-
*/
13431328
if (DatumGetInt32(expr_value) == 0)
13441329
return true;
13451330

0 commit comments

Comments
 (0)