Skip to content

Commit

Permalink
Fixed CORE-6322 - IN predicate and quantified comparison predicates b…
Browse files Browse the repository at this point in the history 
…ehave incorrectly with NULL.
  • Loading branch information
@asfernandes
asfernandes committed Jun 3, 2020
1 parent 20f6f76 commit 596d397
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 453 deletions.
322 changes: 148 additions & 174 deletions src/dsql/BoolNodes.cpp
View file
Expand Up @@ -1438,21 +1438,6 @@ BoolExprNode* NotBoolNode::copy(thread_db* tdbb, NodeCopier& copier) const
return node;
}

BoolExprNode* NotBoolNode::pass1(thread_db* tdbb, CompilerScratch* csb)
{
RseBoolNode* rseBoolean = nodeAs<RseBoolNode>(arg);

if (rseBoolean)
{
if (rseBoolean->blrOp == blr_ansi_any)
rseBoolean->nodFlags |= FLAG_DEOPTIMIZE | FLAG_ANSI_NOT;
else if (rseBoolean->blrOp == blr_ansi_all)
rseBoolean->nodFlags |= FLAG_ANSI_NOT;
}

return BoolExprNode::pass1(tdbb, csb);
}

bool NotBoolNode::execute(thread_db* tdbb, jrd_req* request) const
{
bool value = arg->execute(tdbb, request);
Expand Down Expand Up @@ -1602,6 +1587,88 @@ DmlNode* RseBoolNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch*
RseBoolNode* node = FB_NEW_POOL(pool) RseBoolNode(pool, blrOp);
node->rse = PAR_rse(tdbb, csb);

if (blrOp == blr_ansi_any || blrOp == blr_ansi_all)
{
// We treat ANY and ALL in the same manner in execution - as ANY.
// For that, we invert ALL's operator to its negated complement and prefix RseBoolNode with NOT.

BoolExprNode* rseBoolNode = nullptr;
BinaryBoolNode* binNode = nullptr;
ComparativeBoolNode* cmpNode = nullptr;

if (node->rse->rse_boolean && (binNode = nodeAs<BinaryBoolNode>(node->rse->rse_boolean)) &&
binNode->blrOp == blr_and)
{
rseBoolNode = binNode->arg1;
cmpNode = nodeAs<ComparativeBoolNode>(binNode->arg2);
}

if (!cmpNode && node->rse->rse_boolean)
cmpNode = nodeAs<ComparativeBoolNode>(node->rse->rse_boolean);

if (!cmpNode)
PAR_syntax_error(csb, "blr_ansi condition");

auto valueNode = cmpNode->arg1;
auto columnNode = cmpNode->arg2;

SubExprNodeCopier copier(csb->csb_pool, csb);

auto* injectedBoolean =
FB_NEW_POOL(pool) BinaryBoolNode(pool, blr_or,
FB_NEW_POOL(pool) BinaryBoolNode(pool, blr_or,
cmpNode,
FB_NEW_POOL(pool) MissingBoolNode(pool, copier.copy(tdbb, columnNode))
),
FB_NEW_POOL(pool) MissingBoolNode(pool, copier.copy(tdbb, valueNode))
);

node->rse->rse_boolean = rseBoolNode ?
FB_NEW_POOL(pool) BinaryBoolNode(pool, blr_and, rseBoolNode, injectedBoolean) :
injectedBoolean;

if (blrOp == blr_ansi_all)
{
switch (cmpNode->blrOp)
{
case blr_eql:
// = to <>
cmpNode->blrOp = blr_neq;
break;

case blr_neq:
// <> to =
cmpNode->blrOp = blr_eql;
break;

case blr_gtr:
// > to <=
cmpNode->blrOp = blr_leq;
break;

case blr_geq:
// >= to <
cmpNode->blrOp = blr_lss;
break;

case blr_lss:
// < to >=
cmpNode->blrOp = blr_geq;
break;

case blr_leq:
// <= to >
cmpNode->blrOp = blr_gtr;
break;

default:
fb_assert(false);
PAR_syntax_error(csb, "blr_ansi operator");
break;
}
}
}

if (blrOp == blr_any || blrOp == blr_exists) // maybe for blr_unique as well?
node->rse->flags |= RseNode::FLAG_OPT_FIRST_ROWS;

Expand All @@ -1611,7 +1678,10 @@ DmlNode* RseBoolNode::parse(thread_db* tdbb, MemoryPool& pool, CompilerScratch*
if (csb->csb_currentDMLNode)
node->ownSavepoint = false;

return node;
if (blrOp == blr_ansi_all)
return FB_NEW_POOL(pool) NotBoolNode(pool, node);
else
return node;
}

string RseBoolNode::internalPrint(NodePrinter& printer) const
Expand Down Expand Up @@ -1687,56 +1757,7 @@ BoolExprNode* RseBoolNode::copy(thread_db* tdbb, NodeCopier& copier) const

BoolExprNode* RseBoolNode::pass1(thread_db* tdbb, CompilerScratch* csb)
{
switch (blrOp)
{
case blr_ansi_all:
{
BoolExprNode* newNode = convertNeqAllToNotAny(tdbb, csb);
if (newNode)
{
doPass1(tdbb, csb, &newNode);
return newNode;
}

nodFlags |= FLAG_DEOPTIMIZE;
}
// fall into

case blr_ansi_any:
{
bool deoptimize = false;

if (nodFlags & FLAG_DEOPTIMIZE)
{
nodFlags &= ~FLAG_DEOPTIMIZE;
deoptimize = true;
}

// Mark the injected boolean as residual, this is required
// to process quantified predicates correctly in some cases.
//
// If necessary, also deoptimize the injected boolean.
// ALL predicate should not be evaluated using an index scan.
// This fixes bug SF #543106.

BoolExprNode* boolean = rse->rse_boolean;
if (boolean)
{
BinaryBoolNode* const binaryNode = nodeAs<BinaryBoolNode>(boolean);
if (binaryNode && binaryNode->blrOp == blr_and)
boolean = binaryNode->arg2;

boolean->nodFlags |= FLAG_RESIDUAL | (deoptimize ? FLAG_DEOPTIMIZE : 0);
}
}
// fall into

case blr_any:
case blr_exists:
case blr_unique:
rse->ignoreDbKey(tdbb, csb);
break;
}
rse->ignoreDbKey(tdbb, csb);

return BoolExprNode::pass1(tdbb, csb);
}
Expand All @@ -1759,17 +1780,6 @@ void RseBoolNode::pass2Boolean2(thread_db* tdbb, CompilerScratch* csb)

RecordSource* const rsb = CMP_post_rse(tdbb, csb, rse);

// for ansi ANY clauses (and ALL's, which are negated ANY's)
// the unoptimized boolean expression must be used, since the
// processing of these clauses is order dependant (see FilteredStream.cpp)

if (blrOp == blr_ansi_any || blrOp == blr_ansi_all)
{
const bool ansiAny = blrOp == blr_ansi_any;
const bool ansiNot = nodFlags & FLAG_ANSI_NOT;
rsb->setAnyBoolean(rse->rse_boolean, ansiAny, ansiNot);
}

csb->csb_fors.add(rsb);

subQuery = FB_NEW_POOL(*tdbb->getDefaultPool()) SubQuery(rsb, rse->rse_invariants);
Expand All @@ -1789,7 +1799,7 @@ bool RseBoolNode::execute(thread_db* tdbb, jrd_req* request) const
{
// An invariant node has already been computed.

if (blrOp == blr_ansi_any && (*invariant_flags & VLU_null))
if ((blrOp == blr_ansi_any || blrOp == blr_ansi_all) && (*invariant_flags & VLU_null))
request->req_flags |= req_null;
else
request->req_flags &= ~req_null;
Expand All @@ -1801,120 +1811,84 @@ bool RseBoolNode::execute(thread_db* tdbb, jrd_req* request) const
StableCursorSavePoint savePoint(tdbb, request->req_transaction, ownSavepoint);

subQuery->open(tdbb);
bool value = subQuery->fetch(tdbb);

if (blrOp == blr_unique && value)
value = !subQuery->fetch(tdbb);

subQuery->close(tdbb);

if (blrOp == blr_any || blrOp == blr_unique)
request->req_flags &= ~req_null;

// If this is an invariant node, save the return value.

if (nodFlags & FLAG_INVARIANT)
{
*invariant_flags |= VLU_computed;

if ((blrOp == blr_ansi_any || blrOp == blr_ansi_all) && (request->req_flags & req_null))
*invariant_flags |= VLU_null;

impure->vlu_misc.vlu_short = value ? TRUE : FALSE;
}

return value;
}

// Try to convert nodes of expression:
// select ... from <t1>
// where <x> not in (select <y> from <t2>)
// (and its variants that uses the same BLR: {NOT (a = ANY b)} and {a <> ALL b})
// to:
// select ... from <t1>
// where not ((x is null and exists (select 1 from <t2>)) or
// exists (select <y> from <t2> where <y> = <x> or <y> is null))
//
// Because the second form can use indexes.
// Returns NULL when not converted, and a new node to be processed when converted.
BoolExprNode* RseBoolNode::convertNeqAllToNotAny(thread_db* tdbb, CompilerScratch* csb)
{
SET_TDBB(tdbb);
TriState value(subQuery->fetch(tdbb));

fb_assert(blrOp == blr_ansi_all);

RseNode* outerRse = rse; // blr_ansi_all rse
ComparativeBoolNode* outerRseNeq;

if (!outerRse || outerRse->type != RseNode::TYPE || outerRse->rse_relations.getCount() != 1 ||
!outerRse->rse_boolean ||
!(outerRseNeq = nodeAs<ComparativeBoolNode>(outerRse->rse_boolean)) ||
outerRseNeq->blrOp != blr_neq)
switch (blrOp)
{
return NULL;
}

RseNode* innerRse = static_cast<RseNode*>(outerRse->rse_relations[0].getObject()); // user rse

// If the rse is different than we expected, do nothing. Do nothing also if it uses FIRST or
// SKIP, as we can't inject booleans there without changing the behavior.
if (!innerRse || innerRse->type != RseNode::TYPE || innerRse->rse_first || innerRse->rse_skip)
return NULL;

NotBoolNode* newNode = FB_NEW_POOL(csb->csb_pool) NotBoolNode(csb->csb_pool);

BinaryBoolNode* orNode = FB_NEW_POOL(csb->csb_pool) BinaryBoolNode(csb->csb_pool, blr_or);

newNode->arg = orNode;

BinaryBoolNode* andNode = FB_NEW_POOL(csb->csb_pool) BinaryBoolNode(csb->csb_pool, blr_and);

orNode->arg1 = andNode;

MissingBoolNode* missNode = FB_NEW_POOL(csb->csb_pool) MissingBoolNode(csb->csb_pool);
missNode->arg = outerRseNeq->arg1;
case blr_unique:
if (value == true)
value = !subQuery->fetch(tdbb);
break;

andNode->arg1 = missNode;
case blr_ansi_all:
case blr_ansi_any:
// Check if we do have at least a record. ANY (empty) is FALSE.
if (value == true)
{
auto boolNode1 = nodeAs<BinaryBoolNode>(rse->rse_boolean);
fb_assert(boolNode1);

RseBoolNode* rseBoolNode = FB_NEW_POOL(csb->csb_pool) RseBoolNode(csb->csb_pool, blr_any);
rseBoolNode->rse = innerRse;
rseBoolNode->ownSavepoint = this->ownSavepoint;
if (boolNode1->blrOp == blr_and) // User's RSE has a WHERE?
{
boolNode1 = nodeAs<BinaryBoolNode>(boolNode1->arg2);
fb_assert(boolNode1);
}

andNode->arg2 = rseBoolNode;
auto boolNode2 = nodeAs<BinaryBoolNode>(boolNode1->arg1);
fb_assert(boolNode1);

RseNode* newInnerRse = innerRse->clone(csb->csb_pool);
newInnerRse->ignoreDbKey(tdbb, csb);
auto missingValueNode = nodeAs<MissingBoolNode>(boolNode1->arg2);
auto columnNode = boolNode2->arg2;
fb_assert(missingValueNode && columnNode);

rseBoolNode = FB_NEW_POOL(csb->csb_pool) RseBoolNode(csb->csb_pool, blr_any);
rseBoolNode->rse = newInnerRse;
rseBoolNode->ownSavepoint = this->ownSavepoint;
// Test our value. NULL op ANY (non empty) is UNKNOWN.
if (!EVL_expr(tdbb, request, missingValueNode->arg))
value.invalidate();
else
{
bool hasNull = false;

orNode->arg2 = rseBoolNode;
// If we found a record, immediately return TRUE.
// Otherwise we should check all records.
// If we had a NULL, return UNKNOWN.
// If we had not a NULL, return FALSE.
do
{
value = boolNode2->arg1->execute(tdbb, request);

BinaryBoolNode* boolean = FB_NEW_POOL(csb->csb_pool) BinaryBoolNode(csb->csb_pool, blr_or);
if (value == true)
break;
else if (value.isUnknown())
hasNull = true;
} while (subQuery->fetch(tdbb));

missNode = FB_NEW_POOL(csb->csb_pool) MissingBoolNode(csb->csb_pool);
missNode->arg = outerRseNeq->arg2;
if (value == false && hasNull)
value.invalidate();
}
}
break;
}

boolean->arg1 = missNode;
subQuery->close(tdbb);

boolean->arg2 = outerRse->rse_boolean;
outerRseNeq->blrOp = blr_eql;
// If this is an invariant node, save the return value.

// If there was a boolean on the stream, append (AND) the new one
if (newInnerRse->rse_boolean)
if (nodFlags & FLAG_INVARIANT)
{
andNode = FB_NEW_POOL(csb->csb_pool) BinaryBoolNode(csb->csb_pool, blr_and);
*invariant_flags |= VLU_computed;

if ((blrOp == blr_ansi_any || blrOp == blr_ansi_all) && value.isUnknown())
*invariant_flags |= VLU_null;

andNode->arg1 = newInnerRse->rse_boolean;
andNode->arg2 = boolean;
boolean = andNode;
impure->vlu_misc.vlu_short = value == true ? TRUE : FALSE;
}

newInnerRse->rse_boolean = boolean;
if (value.isUnknown())
request->req_flags |= req_null;
else
request->req_flags &= ~req_null;

SubExprNodeCopier copier(csb->csb_pool, csb);
return copier.copy(tdbb, static_cast<BoolExprNode*>(newNode));
return value.orElse(false);
}


Expand Down

0 comments on commit 596d397

Please sign in to comment.