Implement #6992: Transform OUTER joins into INNER ones if the WHERE condition violates the outer join rules

src/dsql/BoolNodes.h

@@ -96,9 +96,11 @@ public:
 	virtual BoolExprNode* dsqlPass(DsqlCompilerScratch* dsqlScratch);
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& streams) const
 	{
-		return blrOp == blr_equiv ? true : BoolExprNode::possiblyUnknown();
+		return (blrOp == blr_equiv) ?
+			arg1->containsAnyStream(streams) || arg2->containsAnyStream(streams) :
+			BoolExprNode::possiblyUnknown(streams);
 	}
 
 	virtual BoolExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
@@ -144,9 +146,9 @@ public:
 	virtual BoolExprNode* dsqlPass(DsqlCompilerScratch* dsqlScratch);
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& streams) const
 	{
-		return true;
+		return arg->containsAnyStream(streams);
 	}
 
 	virtual BoolExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
@@ -177,11 +179,6 @@ public:
 	virtual BoolExprNode* dsqlPass(DsqlCompilerScratch* dsqlScratch);
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 
-	virtual bool possiblyUnknown() const
-	{
-		return true;
-	}
-
 	virtual BoolExprNode* copy(thread_db* tdbb, NodeCopier& copier) const;
 	virtual BoolExprNode* pass1(thread_db* tdbb, CompilerScratch* csb);
 	virtual bool execute(thread_db* tdbb, Request* request) const;
@@ -225,7 +222,7 @@ public:
 		return true;
 	}
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
 	{
 		return true;
 	}

src/dsql/ExprNodes.cpp

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

src/dsql/ExprNodes.h

@@ -312,9 +312,15 @@ public:
 	virtual ValueExprNode* pass2(thread_db* tdbb, CompilerScratch* csb);
 	virtual dsc* execute(thread_db* tdbb, Request* request) const;
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& streams) const
 	{
-		return true;
+		for (const auto& item : args->items)
+		{
+			if (item->containsAnyStream(streams))
+				return true;
+		}
+
+		return false;
 	}
 
 public:
@@ -776,7 +782,7 @@ public:
 		dsqlDesc = desc;
 	}
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
 	{
 		return false;
 	}
@@ -1650,7 +1656,7 @@ public:
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 	virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
 	{
 		return false;
 	}
@@ -1913,7 +1919,7 @@ public:
 		return false;
 	}
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
 	{
 		return true;
 	}
@@ -2122,7 +2128,7 @@ public:
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 	virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
 	{
 		return true;
 	}
@@ -2171,9 +2177,11 @@ public:
 	virtual void genBlr(DsqlCompilerScratch* dsqlScratch);
 	virtual void make(DsqlCompilerScratch* dsqlScratch, dsc* desc);
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& streams) const
 	{
-		return true;
+		return condition->containsAnyStream(streams) ||
+			trueValue->containsAnyStream(streams) ||
+			falseValue->containsAnyStream(streams);
 	}
 
 	virtual void getDesc(thread_db* tdbb, CompilerScratch* csb, dsc* desc);

src/dsql/Nodes.h

@@ -658,7 +658,7 @@ public:
 	}
 
 	// Check if expression could return NULL or expression can turn NULL into a true/false.
-	virtual bool possiblyUnknown() const;
+	virtual bool possiblyUnknown(const StreamList& streams) const;
 
 	// Verify if this node is allowed in an unmapped boolean.
 	virtual bool unmappable(const MapNode* mapNode, StreamType shellStream) const;
@@ -666,12 +666,26 @@ public:
 	// Return all streams referenced by the expression.
 	virtual void collectStreams(SortedStreamList& streamList) const;
 
-	virtual bool containsStream(StreamType stream) const
+	bool containsStream(StreamType stream) const
 	{
-		SortedStreamList streams;
-		collectStreams(streams);
+		SortedStreamList nodeStreams;
+		collectStreams(nodeStreams);
 
-		return streams.exist(stream);
+		return nodeStreams.exist(stream);
+	}
+
+	bool containsAnyStream(const StreamList& streams) const
+	{
+		SortedStreamList nodeStreams;
+		collectStreams(nodeStreams);
+
+		for (const auto stream : streams)
+		{
+			if (nodeStreams.exist(stream))
+				return true;
+		}
+
+		return false;
 	}
 
 	virtual bool dsqlMatch(DsqlCompilerScratch* dsqlScratch, const ExprNode* other, bool ignoreMapCast) const;
@@ -1026,7 +1040,7 @@ public:
 
 	virtual AggNode* pass2(thread_db* tdbb, CompilerScratch* csb);
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
 	{
 		return true;
 	}
@@ -1155,12 +1169,13 @@ public:
 	virtual RecordSourceNode* pass2(thread_db* tdbb, CompilerScratch* csb) = 0;
 	virtual void pass2Rse(thread_db* tdbb, CompilerScratch* csb) = 0;
 	virtual bool containsStream(StreamType checkStream) const = 0;
+
 	virtual void genBlr(DsqlCompilerScratch* /*dsqlScratch*/)
 	{
 		fb_assert(false);
 	}
 
-	virtual bool possiblyUnknown() const
+	virtual bool possiblyUnknown(const StreamList& /*streams*/) const
 	{
 		return true;
 	}

src/jrd/RecordSourceNodes.cpp

@@ -2810,10 +2810,16 @@ RseNode* RseNode::pass1(thread_db* tdbb, CompilerScratch* csb)
 	ValueExprNode* skip = rse_skip;
 	PlanNode* plan = rse_plan;
 
+	if (!rse_jointype)
+		csb->csb_inner_booleans.push(rse_boolean);
+
 	// zip thru RseNode expanding views and inner joins
 	for (auto sub : rse_relations)
 		processSource(tdbb, csb, this, sub, &boolean, stack);
 
+	if (!rse_jointype)
+		csb->csb_inner_booleans.pop();
+
 	// Now, rebuild the RseNode block.
 
 	rse_relations.resize(stack.getCount());
@@ -2880,6 +2886,63 @@ RseNode* RseNode::pass1(thread_db* tdbb, CompilerScratch* csb)
 void RseNode::pass1Source(thread_db* tdbb, CompilerScratch* csb, RseNode* rse,
 	BoolExprNode** boolean, RecordSourceNodeStack& stack)
 {
+	if (rse_jointype)
+	{
+		// Check whether any of the upper level booleans (those belonging to the WHERE clause)
+		// is able to filter out rows from the "inner" streams. If this is the case,
+		// transform the join type accordingly (LEFT -> INNER, FULL -> LEFT or INNER).
+
+		fb_assert(rse_relations.getCount() == 2);
+
+		const auto rse1 = rse_relations[0];
+		const auto rse2 = rse_relations[1];
+		fb_assert(rse1 && rse2);
+
+		StreamList streams;
+
+		// First check the left stream of the full outer join
+		if (rse_jointype == blr_full)
+		{
+			rse1->computeRseStreams(streams);
+
+			for (const auto boolean : csb->csb_inner_booleans)
+			{
+				if (boolean &&
+					boolean->containsAnyStream(streams) &&
+					!boolean->possiblyUnknown(streams))
+				{
+					rse_jointype = blr_left;
+					break;
+				}
+			}
+		}
+
+		// Then check the right stream of both left and full outer joins
+		streams.clear();
+		rse2->computeRseStreams(streams);
+
+		for (const auto boolean : csb->csb_inner_booleans)
+		{
+			if (boolean &&
+				boolean->containsAnyStream(streams) &&
+				!boolean->possiblyUnknown(streams))
+			{
+				if (rse_jointype == blr_full)
+				{
+					// We should transform FULL join to RIGHT join,
+					// but as we don't allow them inside the engine
+					// just swap the sides and insist it's LEFT join
+					std::swap(rse_relations[0], rse_relations[1]);
+					rse_jointype = blr_left;
+				}
+				else
+					rse_jointype = blr_inner;
+
+				break;
+			}
+		}
+	}
+
 	// in the case of an RseNode, it is possible that a new RseNode will be generated,
 	// so wait to process the source before we push it on the stack (bug 8039)
 

src/jrd/exe.h

@@ -479,6 +479,7 @@ public:
 		csb_current_nodes(p),
 		csb_current_for_nodes(p),
 		csb_computing_fields(p),
+		csb_inner_booleans(p),
 		csb_variables_used_in_subroutines(p),
 		csb_pool(p),
 		csb_map_field_info(p),
@@ -557,6 +558,7 @@ public:
 												// candidates within whose scope we are
 	Firebird::Array<ForNode*> csb_current_for_nodes;
 	Firebird::SortedArray<jrd_fld*> csb_computing_fields;	// Computed fields being compiled
+	Firebird::Array<BoolExprNode*> csb_inner_booleans;	// Inner booleans at the current scope
 	Firebird::SortedArray<USHORT> csb_variables_used_in_subroutines;
 	StreamType		csb_n_stream;				// Next available stream
 	USHORT			csb_msg_number;				// Highest used message number

src/jrd/optimizer/Optimizer.cpp

@@ -682,9 +682,31 @@ RecordSource* Optimizer::compile(BoolExprNodeStack* parentStack)
 		for (BoolExprNodeStack::iterator iter(*parentStack);
 			iter.hasData() && conjunctCount < MAX_CONJUNCTS; ++iter)
 		{
-			BoolExprNode* const node = iter.object();
+			const auto node = iter.object();
 
-			if (!isInnerJoin() && node->possiblyUnknown())
+			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))
 			{
 				// parent missing conjunctions shouldn't be
 				// distributed to FULL OUTER JOIN streams at all