Fixed CORE-1295: Bad optimization of queries with DB_KEY.

src/jrd/Optimizer.cpp

@@ -1216,7 +1216,25 @@ InversionCandidate* OptimizerRetrieval::generateInversion(IndexTableScan** rsb)
 					optimizer->opt_conjuncts.end();
 
 	InversionCandidateList inversions;
-	inversions.shrink(0);
+	InversionCandidate* invCandidate = NULL;
+
+	// Check for any DB_KEY comparisons
+	for (OptimizerBlk::opt_conjunct* tail = opt_begin; tail < opt_end; tail++)
+	{
+		if (tail->opt_conjunct_flags & opt_conjunct_matched) {
+			continue;
+		}
+		jrd_nod* const node = tail->opt_conjunct_node;
+		if (!(tail->opt_conjunct_flags & opt_conjunct_used) && node)
+		{
+			invCandidate = matchDbKey(node);
+			if (invCandidate)
+			{
+				invCandidate->boolean = node;
+				inversions.add(invCandidate);
+			}
+		}
+	}
 
 	// First, handle "AND" comparisons (all nodes except nod_or)
 	for (OptimizerBlk::opt_conjunct* tail = opt_begin; tail < opt_end; tail++)
@@ -1230,6 +1248,7 @@ InversionCandidate* OptimizerRetrieval::generateInversion(IndexTableScan** rsb)
 			matchOnIndexes(&indexScratches, node, 1);
 		}
 	}
+
 	getInversionCandidates(&inversions, &indexScratches, 1);
 
 	if (sort && rsb) {
@@ -1237,7 +1256,6 @@ InversionCandidate* OptimizerRetrieval::generateInversion(IndexTableScan** rsb)
 	}
 
 	// Second, handle "OR" comparisons
-	InversionCandidate* invCandidate = NULL;
 	for (OptimizerBlk::opt_conjunct* tail = opt_begin; tail < opt_end; tail++)
 	{
 		if (tail->opt_conjunct_flags & opt_conjunct_matched) {
@@ -1690,6 +1708,52 @@ void OptimizerRetrieval::getInversionCandidates(InversionCandidateList* inversio
 	}
 }
 
+jrd_nod* OptimizerRetrieval::findDbKey(jrd_nod* dbkey, USHORT stream, SLONG* position) const
+{
+/**************************************
+ *
+ *	f i n d D b K e y
+ *
+ **************************************
+ *
+ * Functional description
+ *	Search a dbkey (possibly a concatenated one) for
+ *	a dbkey for specified stream.
+ *
+ **************************************/
+
+	if (dbkey->nod_type == nod_dbkey)
+	{
+		if ((USHORT)(IPTR) dbkey->nod_arg[0] == stream)
+		{
+			return dbkey;
+		}
+
+		*position = *position + 1;
+		return NULL;
+	}
+
+	ConcatenateNode* concatNode = ExprNode::as<ConcatenateNode>(dbkey);
+
+	if (concatNode)
+	{
+		jrd_nod* dbkey_temp = findDbKey(concatNode->arg1, stream, position);
+		if (dbkey_temp)
+		{
+			return dbkey_temp;
+		}
+
+		dbkey_temp = findDbKey(concatNode->arg2, stream, position);
+		if (dbkey_temp)
+		{
+			return dbkey_temp;
+		}
+	}
+
+	return NULL;
+}
+
+
 jrd_nod* OptimizerRetrieval::makeIndexNode(const index_desc* idx) const
 {
 /**************************************
@@ -2530,6 +2594,91 @@ bool OptimizerRetrieval::matchBoolean(IndexScratch* indexScratch, jrd_nod* boole
 	return (count >= 1);
 }
 
+InversionCandidate* OptimizerRetrieval::matchDbKey(jrd_nod* boolean) const
+{
+/**************************************
+ *
+ *	m a t c h D b K e y
+ *
+ **************************************
+ *
+ * Functional description
+ *  Check whether a boolean is a DB_KEY based comparison.
+ *
+ **************************************/
+	// If this isn't an equality, it isn't even interesting
+
+	if (boolean->nod_type != nod_eql)
+	{
+		return NULL;
+	}
+
+	// Find the side of the equality that is potentially a dbkey.  If
+	// neither, make the obvious deduction
+
+	jrd_nod* dbkey = boolean->nod_arg[0];
+	jrd_nod* value = boolean->nod_arg[1];
+
+	if (dbkey->nod_type != nod_dbkey && !ExprNode::is<ConcatenateNode>(dbkey))
+	{
+		if (value->nod_type != nod_dbkey && !ExprNode::is<ConcatenateNode>(value))
+		{
+			return NULL;
+		}
+
+		dbkey = value;
+		value = boolean->nod_arg[0];
+	}
+
+	// If the value isn't computable, this has been a waste of time
+
+	if (!OPT_computable(csb, value, stream, false, false))
+	{
+		return NULL;
+	}
+
+	// If this is a concatenation, find an appropriate dbkey
+
+	SLONG n = 0;
+	if (ExprNode::is<ConcatenateNode>(dbkey))
+	{
+		dbkey = findDbKey(dbkey, stream, &n);
+		if (!dbkey)
+		{
+			return NULL;
+		}
+	}
+
+	// Make sure we have the correct stream
+
+	if ((USHORT)(IPTR) dbkey->nod_arg[0] != stream)
+	{
+		return NULL;
+	}
+
+	// If this is a dbkey for the appropriate stream, it's invertable
+
+	InversionCandidate* const invCandidate = FB_NEW(pool) InversionCandidate(pool);
+	invCandidate->indexes = 0;
+	invCandidate->selectivity = 1 / csb->csb_rpt[stream].csb_cardinality;
+	invCandidate->cost = 0;
+	invCandidate->unique = true;
+	invCandidate->matches.add(boolean);
+
+	if (createIndexScanNodes)
+	{
+		jrd_nod* const inversion = PAR_make_node(tdbb, 2);
+		inversion->nod_count = 1;
+		inversion->nod_type = nod_bit_dbkey;
+		inversion->nod_arg[0] = value;
+		inversion->nod_arg[1] = (jrd_nod*)(IPTR) n;
+		inversion->nod_impure = CMP_impure(csb, sizeof(impure_inversion));
+		invCandidate->inversion = inversion;
+	}
+
+	return invCandidate;
+}
+
 InversionCandidate* OptimizerRetrieval::matchOnIndexes(
 	IndexScratchList* inputIndexScratches, jrd_nod* boolean, USHORT scope) const
 {

src/jrd/Optimizer.h

@@ -193,8 +193,10 @@ protected:
 	jrd_nod* makeIndexScanNode(IndexScratch* indexScratch) const;
 	InversionCandidate* makeInversion(InversionCandidateList* inversions) const;
 	bool matchBoolean(IndexScratch* indexScratch, jrd_nod* boolean, USHORT scope) const;
+	InversionCandidate* matchDbKey(jrd_nod* boolean) const;
 	InversionCandidate* matchOnIndexes(IndexScratchList* indexScratches,
 		jrd_nod* boolean, USHORT scope) const;
+	jrd_nod* findDbKey(jrd_nod*, USHORT, SLONG*) const;
 
 #ifdef OPT_DEBUG_RETRIEVAL
 	void printCandidate(const InversionCandidate* candidate) const;

src/jrd/opt.cpp

@@ -639,7 +639,6 @@ static bool check_for_nod_from(const jrd_nod*);
 static SLONG decompose(thread_db*, jrd_nod*, NodeStack&, CompilerScratch*);
 static USHORT distribute_equalities(NodeStack&, CompilerScratch*, USHORT);
 static void find_index_relationship_streams(thread_db*, OptimizerBlk*, const UCHAR*, UCHAR*, UCHAR*);
-static jrd_nod* find_dbkey(jrd_nod*, USHORT, SLONG*);
 static void form_rivers(thread_db*, OptimizerBlk*, const UCHAR*, RiverList&, jrd_nod**, jrd_nod*);
 static bool form_river(thread_db*, OptimizerBlk*, USHORT, USHORT, UCHAR*, RiverList&, jrd_nod**);
 static RecordSource* gen_aggregate(thread_db*, OptimizerBlk*, jrd_nod*, NodeStack*, UCHAR);
@@ -651,7 +650,6 @@ static RecordSource* gen_residual_boolean(thread_db*, OptimizerBlk*, RecordSourc
 static RecordSource* gen_retrieval(thread_db*, OptimizerBlk*, SSHORT, jrd_nod**, bool, bool, jrd_nod**);
 static bool gen_equi_join(thread_db*, OptimizerBlk*, RiverList&);
 static RecordSource* gen_union(thread_db*, OptimizerBlk*, jrd_nod*, UCHAR *, USHORT, NodeStack*, UCHAR);
-static jrd_nod* make_dbkey(thread_db*, OptimizerBlk*, jrd_nod*, USHORT);
 static jrd_nod* make_inference_node(CompilerScratch*, jrd_nod*, jrd_nod*, jrd_nod*);
 static bool map_equal(const jrd_nod*, const jrd_nod*, const jrd_nod*);
 static void mark_indices(CompilerScratch::csb_repeat*, SSHORT);
@@ -2334,46 +2332,6 @@ static void find_index_relationship_streams(thread_db* tdbb,
 }
 
 
-static jrd_nod* find_dbkey(jrd_nod* dbkey, USHORT stream, SLONG* position)
-{
-/**************************************
- *
- *	f i n d _ d b k e y
- *
- **************************************
- *
- * Functional description
- *	Search a dbkey (possibly a concatenated one) for
- *	a dbkey for specified stream.
- *
- **************************************/
-	DEV_BLKCHK(dbkey, type_nod);
-	if (dbkey->nod_type == nod_dbkey)
-	{
-		if ((USHORT)(IPTR) dbkey->nod_arg[0] == stream)
-			return dbkey;
-
-		*position = *position + 1;
-		return NULL;
-	}
-
-	ConcatenateNode* concatNode = ExprNode::as<ConcatenateNode>(dbkey);
-
-	if (concatNode)
-	{
-		jrd_nod* dbkey_temp = find_dbkey(concatNode->arg1, stream, position);
-		if (dbkey_temp)
-			return dbkey_temp;
-
-		dbkey_temp = find_dbkey(concatNode->arg2, stream, position);
-		if (dbkey_temp)
-			return dbkey_temp;
-	}
-
-	return NULL;
-}
-
-
 static void form_rivers(thread_db*		tdbb,
 						OptimizerBlk*	opt,
 						const UCHAR*	streams,
@@ -3187,10 +3145,6 @@ static RecordSource* gen_retrieval(thread_db*     tdbb,
 	for (; tail < opt_end; tail++)
 	{
 		jrd_nod* const node = tail->opt_conjunct_node;
-		if (!relation->rel_file && !relation->isVirtual())
-		{
-			compose(&inversion, make_dbkey(tdbb, opt, node, stream), nod_bit_and);
-		}
 		if (!(tail->opt_conjunct_flags & opt_conjunct_used) &&
 			OPT_computable(csb, node, -1, false, false))
 		{
@@ -3965,82 +3919,6 @@ static RecordSource* gen_union(thread_db* tdbb,
 }
 
 
-jrd_nod* make_dbkey(thread_db* tdbb, OptimizerBlk* opt, jrd_nod* boolean, USHORT stream)
-{
-/**************************************
- *
- *	m a k e _ d b k e y
- *
- **************************************
- *
- * Functional description
- *	If boolean is an equality comparison on the proper dbkey,
- *	make a "bit_dbkey" operator (makes bitmap out of dbkey
- *	expression.
- *
- *	This is a little hairy, since view dbkeys are expressed as
- *	concatenations of primitive dbkeys.
- *
- **************************************/
-	SET_TDBB(tdbb);
-
-	DEV_BLKCHK(opt, type_opt);
-	DEV_BLKCHK(boolean, type_nod);
-
-	// If this isn't an equality, it isn't even interesting
-
-	if (boolean->nod_type != nod_eql)
-		return NULL;
-
-	// Find the side of the equality that is potentially a dbkey.  If
-	// neither, make the obvious deduction
-
-	jrd_nod* dbkey = boolean->nod_arg[0];
-	jrd_nod* value = boolean->nod_arg[1];
-
-	if (dbkey->nod_type != nod_dbkey && !ExprNode::is<ConcatenateNode>(dbkey))
-	{
-		if (value->nod_type != nod_dbkey && !ExprNode::is<ConcatenateNode>(value))
-			return NULL;
-
-		dbkey = value;
-		value = boolean->nod_arg[0];
-	}
-
-	// If the value isn't computable, this has been a waste of time
-
-	CompilerScratch* csb = opt->opt_csb;
-	if (!OPT_computable(csb, value, stream, false, false))
-		return NULL;
-
-	// If this is a concatenation, find an appropriate dbkey
-
-	SLONG n = 0;
-	if (ExprNode::is<ConcatenateNode>(dbkey))
-	{
-		dbkey = find_dbkey(dbkey, stream, &n);
-		if (!dbkey)
-			return NULL;
-	}
-
-	// Make sure we have the correct stream
-
-	if ((USHORT)(IPTR) dbkey->nod_arg[0] != stream)
-		return NULL;
-
-	// If this is a dbkey for the appropriate stream, it's invertable
-
-	dbkey = PAR_make_node(tdbb, 2);
-	dbkey->nod_count = 1;
-	dbkey->nod_type = nod_bit_dbkey;
-	dbkey->nod_arg[0] = value;
-	dbkey->nod_arg[1] = (jrd_nod*)(IPTR) n;
-	dbkey->nod_impure = CMP_impure(csb, sizeof(impure_inversion));
-
-	return dbkey;
-}
-
-
 static jrd_nod* make_inference_node(CompilerScratch* csb, jrd_nod* boolean,
 									jrd_nod* arg1, jrd_nod* arg2)
 {