Revert back to separate methods, this fixes #7474 (Incorrect condition evaluation)

src/dsql/BoolNodes.h

@@ -51,6 +51,13 @@ public:
 	virtual BoolExprNode* dsqlPass(DsqlCompilerScratch* dsqlScratch);
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 
+	virtual bool ignoreNulls(const StreamList& streams) const
+	{
+		return (blrOp == blr_or) ?
+			arg1->ignoreNulls(streams) && arg2->ignoreNulls(streams) :
+			BoolExprNode::ignoreNulls(streams);
+	}
+
 	virtual BoolExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
 	virtual bool dsqlMatch(DsqlCompilerScratch* dsqlScratch, const ExprNode* other, bool ignoreMapCast) const;
 	virtual bool sameAs(const ExprNode* other, bool ignoreStreams) const;
@@ -96,11 +103,14 @@ public:
 	virtual BoolExprNode* dsqlPass(DsqlCompilerScratch* dsqlScratch);
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 
-	virtual bool possiblyUnknown(const StreamList& streams) const
+	virtual bool possiblyUnknown() const
 	{
-		return (blrOp == blr_equiv) ?
-			arg1->containsAnyStream(streams) || arg2->containsAnyStream(streams) :
-			BoolExprNode::possiblyUnknown(streams);
+		return (blrOp == blr_equiv) ? true : BoolExprNode::possiblyUnknown();
+	}
+
+	virtual bool ignoreNulls(const StreamList& streams) const
+	{
+		return (blrOp == blr_equiv) ? false : BoolExprNode::ignoreNulls(streams);
 	}
 
 	virtual BoolExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
@@ -146,9 +156,14 @@ public:
 	virtual BoolExprNode* dsqlPass(DsqlCompilerScratch* dsqlScratch);
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 
-	virtual bool possiblyUnknown(const StreamList& streams) const
+	virtual bool possiblyUnknown() const
 	{
-		return arg->containsAnyStream(streams);
+		return true;
+	}
+
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
+		return false;
 	}
 
 	virtual BoolExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
@@ -222,11 +237,16 @@ public:
 		return true;
 	}
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
+	virtual bool possiblyUnknown() const
 	{
 		return true;
 	}
 
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
+		return false;
+	}
+
 	virtual BoolExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
 	virtual bool dsqlMatch(DsqlCompilerScratch* dsqlScratch, const ExprNode* other, bool ignoreMapCast) const;
 	virtual bool sameAs(const ExprNode* other, bool ignoreStreams) const;

src/dsql/ExprNodes.cpp

@@ -289,14 +289,28 @@ bool ExprNode::sameAs(const ExprNode* other, bool ignoreStreams) const
 	return true;
 }
 
-bool ExprNode::possiblyUnknown(const StreamList& streams) const
+bool ExprNode::possiblyUnknown() const
 {
 	NodeRefsHolder holder;
 	getChildren(holder, false);
 
 	for (auto i : holder.refs)
 	{
-		if (*i && (*i)->possiblyUnknown(streams))
+		if (*i && (*i)->possiblyUnknown())
+			return true;
+	}
+
+	return false;
+}
+
+bool ExprNode::ignoreNulls(const StreamList& streams) const
+{
+	NodeRefsHolder holder;
+	getChildren(holder, false);
+
+	for (auto i : holder.refs)
+	{
+		if (*i && (*i)->ignoreNulls(streams))
 			return true;
 	}
 

src/dsql/ExprNodes.h

@@ -312,14 +312,13 @@ public:
 	virtual ValueExprNode* pass2(thread_db* tdbb, CompilerScratch* csb);
 	virtual dsc* execute(thread_db* tdbb, Request* request) const;
 
-	virtual bool possiblyUnknown(const StreamList& streams) const
+	virtual bool possiblyUnknown() const
 	{
-		for (const auto& item : args->items)
-		{
-			if (item->containsAnyStream(streams))
-				return true;
-		}
+		return true;
+	}
 
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
 		return false;
 	}
 
@@ -667,11 +666,6 @@ public:
 		fb_assert(false);
 	}
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
-	{
-		return true;
-	}
-
 	virtual void collectStreams(SortedStreamList& streamList) const;
 
 	virtual bool computable(CompilerScratch* csb, StreamType stream,
@@ -787,11 +781,16 @@ public:
 		dsqlDesc = desc;
 	}
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
+	virtual bool possiblyUnknown() const
 	{
 		return false;
 	}
 
+	virtual bool ignoreNulls(const StreamList& streams) const
+	{
+		return streams.exist(fieldStream);
+	}
+
 	virtual void collectStreams(SortedStreamList& streamList) const
 	{
 		if (!streamList.exist(fieldStream))
@@ -1661,11 +1660,16 @@ public:
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 	virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
+	virtual bool possiblyUnknown() const
 	{
 		return false;
 	}
 
+	virtual bool ignoreNulls(const StreamList& streams) const
+	{
+		return streams.exist(recStream);
+	}
+
 	virtual void collectStreams(SortedStreamList& streamList) const
 	{
 		if (!streamList.exist(recStream))
@@ -1924,11 +1928,16 @@ public:
 		return false;
 	}
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
+	virtual bool possiblyUnknown() const
 	{
 		return true;
 	}
 
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
+		return false;
+	}
+
 	virtual void collectStreams(SortedStreamList& streamList) const;
 
 	virtual bool computable(CompilerScratch* csb, StreamType stream,
@@ -2133,11 +2142,16 @@ public:
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 	virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
+	virtual bool possiblyUnknown() const
 	{
 		return true;
 	}
 
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
+		return false;
+	}
+
 	virtual void getDesc(thread_db* tdbb, CompilerScratch* csb, dsc* desc);
 	virtual ValueExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
 	virtual bool dsqlMatch(DsqlCompilerScratch* dsqlScratch, const ExprNode* other, bool ignoreMapCast) const;
@@ -2182,11 +2196,14 @@ public:
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 	virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
 
-	virtual bool possiblyUnknown(const StreamList& streams) const
+	virtual bool possiblyUnknown() const
 	{
-		return condition->containsAnyStream(streams) ||
-			trueValue->containsAnyStream(streams) ||
-			falseValue->containsAnyStream(streams);
+		return true;
+	}
+
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
+		return false;
 	}
 
 	virtual void getDesc(thread_db* tdbb, CompilerScratch* csb, dsc* desc);

src/dsql/Nodes.h

@@ -658,7 +658,10 @@ public:
 	}
 
 	// Check if expression could return NULL or expression can turn NULL into a true/false.
-	virtual bool possiblyUnknown(const StreamList& streams) const;
+	virtual bool possiblyUnknown() const;
+
+	// Check if expression is known to ignore NULLs
+	virtual bool ignoreNulls(const StreamList& streams) const;
 
 	// Verify if this node is allowed in an unmapped boolean.
 	virtual bool unmappable(const MapNode* mapNode, StreamType shellStream) const;
@@ -1042,11 +1045,16 @@ public:
 
 	virtual AggNode* pass2(thread_db* tdbb, CompilerScratch* csb);
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
+	virtual bool possiblyUnknown() const
 	{
 		return true;
 	}
 
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
+		return false;
+	}
+
 	virtual void collectStreams(SortedStreamList& /*streamList*/) const
 	{
 		// ASF: Although in v2.5 the visitor happens normally for the node childs, nod_count has
@@ -1177,11 +1185,16 @@ public:
 		fb_assert(false);
 	}
 
-	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
+	virtual bool possiblyUnknown() const
 	{
 		return true;
 	}
 
+	virtual bool ignoreNulls(const StreamList& /*streams*/) const
+	{
+		return false;
+	}
+
 	virtual bool unmappable(const MapNode* /*mapNode*/, StreamType /*shellStream*/) const
 	{
 		return false;

src/jrd/RecordSourceNodes.cpp

@@ -2907,9 +2907,7 @@ void RseNode::pass1Source(thread_db* tdbb, CompilerScratch* csb, RseNode* rse,
 
 			for (const auto boolean : csb->csb_inner_booleans)
 			{
-				if (boolean &&
-					boolean->containsAnyStream(streams) &&
-					!boolean->possiblyUnknown(streams))
+				if (boolean && boolean->ignoreNulls(streams))
 				{
 					rse_jointype = blr_left;
 					break;
@@ -2923,9 +2921,7 @@ void RseNode::pass1Source(thread_db* tdbb, CompilerScratch* csb, RseNode* rse,
 
 		for (const auto boolean : csb->csb_inner_booleans)
 		{
-			if (boolean &&
-				boolean->containsAnyStream(streams) &&
-				!boolean->possiblyUnknown(streams))
+			if (boolean && boolean->ignoreNulls(streams))
 			{
 				if (rse_jointype == blr_full)
 				{

src/jrd/optimizer/Optimizer.cpp

@@ -684,29 +684,7 @@ RecordSource* Optimizer::compile(BoolExprNodeStack* parentStack)
 		{
 			const auto node = iter.object();
 
-			StreamList streams;
-
-			if (!isInnerJoin())
-			{
-				fb_assert(rse->rse_relations.getCount() == 2);
-
-				const auto rse1 = rse->rse_relations[0];
-				const auto rse2 = rse->rse_relations[1];
-				fb_assert(rse1 && rse2);
-
-				if (isFullJoin())
-				{
-					rse1->computeRseStreams(streams);
-					rse2->computeRseStreams(streams);
-				}
-				else // left outer join
-				{
-					fb_assert(rse->rse_jointype == blr_left);
-					rse2->computeRseStreams(streams);
-				}
-			}
-
-			if (streams.hasData() && node->possiblyUnknown(streams))
+			if (!isInnerJoin() && node->possiblyUnknown())
 			{
 				// parent missing conjunctions shouldn't be
 				// distributed to FULL OUTER JOIN streams at all