From bbaaf2dd5ac6fcfff37b226afa1197b02c48ce8b Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Tue, 9 Jan 2018 00:59:09 +0100
Subject: [PATCH 1/4] Pass all keys to BRIN consistent function at once

When building the bitmap, bringetbitmap was passing the scan keys
to consistent support procedure one by one. That works fine for
the simple default opclasses (minmax and inclusion), but not for
opclasses with more complicated summaries.

For example the multi-minmax opclass needs to see all the keys to
efficiently eliminate the partial ranges, otherwise it's mostly
just a more expensive minmax opclass.

So instead split the scan keys into per-attribute groups, and pass
the whole group at once.
---
 src/backend/access/brin/brin.c           |  97 +++++--
 src/backend/access/brin/brin_inclusion.c | 484 ++++++++++++++++++-------------
 src/backend/access/brin/brin_minmax.c    | 168 +++++++----
 3 files changed, 451 insertions(+), 298 deletions(-)

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 68b3371..b568786 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -367,6 +367,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 	BrinMemTuple *dtup;
 	BrinTuple  *btup = NULL;
 	Size		btupsz = 0;
+	ScanKey	  **keys;
+	int		   *nkeys;
+	int			keyno;
 
 	opaque = (BrinOpaque *) scan->opaque;
 	bdesc = opaque->bo_bdesc;
@@ -388,6 +391,53 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 	 */
 	consistentFn = palloc0(sizeof(FmgrInfo) * bdesc->bd_tupdesc->natts);
 
+	/*
+	 * Make room for per-attribute lists of scan keys that we'll pass to the
+	 * consistent support procedure.
+	 */
+	keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+	nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+	/*
+	 * Preprocess the scan keys - split them into per-attribute arrays.
+	 */
+	for (keyno = 0; keyno < scan->numberOfKeys; keyno++)
+	{
+		ScanKey		key = &scan->keyData[keyno];
+		AttrNumber	keyattno = key->sk_attno;
+
+		/*
+		 * The collation of the scan key must match the collation
+		 * used in the index column (but only if the search is not
+		 * IS NULL/ IS NOT NULL).  Otherwise we shouldn't be using
+		 * this index ...
+		 */
+		Assert((key->sk_flags & SK_ISNULL) ||
+			   (key->sk_collation ==
+				TupleDescAttr(bdesc->bd_tupdesc,
+							  keyattno - 1)->attcollation));
+
+		/* First time we see this index attribute, so init as needed. */
+		if (!keys[keyattno-1])
+		{
+			FmgrInfo   *tmp;
+
+			keys[keyattno-1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
+
+			/* First time this column, so look up consistent function */
+			Assert(consistentFn[keyattno - 1].fn_oid == InvalidOid);
+
+			tmp = index_getprocinfo(idxRel, keyattno,
+									BRIN_PROCNUM_CONSISTENT);
+			fmgr_info_copy(&consistentFn[keyattno - 1], tmp,
+						   CurrentMemoryContext);
+		}
+
+		/* Add key to the per-attribute array. */
+		keys[keyattno - 1][nkeys[keyattno - 1]] = key;
+		nkeys[keyattno - 1]++;
+	}
+
 	/* allocate an initial in-memory tuple, out of the per-range memcxt */
 	dtup = brin_new_memtuple(bdesc);
 
@@ -448,7 +498,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 			}
 			else
 			{
-				int			keyno;
+				int		attno;
 
 				/*
 				 * Compare scan keys with summary values stored for the range.
@@ -458,34 +508,26 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 				 * no keys.
 				 */
 				addrange = true;
-				for (keyno = 0; keyno < scan->numberOfKeys; keyno++)
+				for (attno = 1; attno <= bdesc->bd_tupdesc->natts; attno++)
 				{
-					ScanKey		key = &scan->keyData[keyno];
-					AttrNumber	keyattno = key->sk_attno;
-					BrinValues *bval = &dtup->bt_columns[keyattno - 1];
+					BrinValues *bval;
 					Datum		add;
+					Oid			collation;
+
+					/* skip attributes without any san keys */
+					if (!nkeys[attno - 1])
+						continue;
+
+					bval = &dtup->bt_columns[attno - 1];
+
+					Assert((nkeys[attno - 1] > 0) &&
+						   (nkeys[attno - 1] <= scan->numberOfKeys));
 
 					/*
-					 * The collation of the scan key must match the collation
-					 * used in the index column (but only if the search is not
-					 * IS NULL/ IS NOT NULL).  Otherwise we shouldn't be using
-					 * this index ...
+					 * Collation from the first key (has to be the same for
+					 * all keys for the same attribue).
 					 */
-					Assert((key->sk_flags & SK_ISNULL) ||
-						   (key->sk_collation ==
-							TupleDescAttr(bdesc->bd_tupdesc,
-										  keyattno - 1)->attcollation));
-
-					/* First time this column? look up consistent function */
-					if (consistentFn[keyattno - 1].fn_oid == InvalidOid)
-					{
-						FmgrInfo   *tmp;
-
-						tmp = index_getprocinfo(idxRel, keyattno,
-												BRIN_PROCNUM_CONSISTENT);
-						fmgr_info_copy(&consistentFn[keyattno - 1], tmp,
-									   CurrentMemoryContext);
-					}
+					collation = keys[attno - 1][0]->sk_collation;
 
 					/*
 					 * Check whether the scan key is consistent with the page
@@ -497,11 +539,12 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 					 * the range as a whole, so break out of the loop as soon
 					 * as a false return value is obtained.
 					 */
-					add = FunctionCall3Coll(&consistentFn[keyattno - 1],
-											key->sk_collation,
+					add = FunctionCall4Coll(&consistentFn[attno - 1],
+											collation,
 											PointerGetDatum(bdesc),
 											PointerGetDatum(bval),
-											PointerGetDatum(key));
+											PointerGetDatum(keys[attno - 1]),
+											Int32GetDatum(nkeys[attno - 1]));
 					addrange = DatumGetBool(add);
 					if (!addrange)
 						break;
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c
index 2542877..fb7f2b0 100644
--- a/src/backend/access/brin/brin_inclusion.c
+++ b/src/backend/access/brin/brin_inclusion.c
@@ -248,7 +248,8 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 {
 	BrinDesc   *bdesc = (BrinDesc *) PG_GETARG_POINTER(0);
 	BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1);
-	ScanKey		key = (ScanKey) PG_GETARG_POINTER(2);
+	ScanKey	   *keys = (ScanKey *) PG_GETARG_POINTER(2);
+	int			nkeys = PG_GETARG_INT32(3);
 	Oid			colloid = PG_GET_COLLATION(),
 				subtype;
 	Datum		unionval;
@@ -256,235 +257,300 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 	Datum		query;
 	FmgrInfo   *finfo;
 	Datum		result;
+	int			keyno;
+	bool		matches;
+	bool		regular_keys = false;
 
-	Assert(key->sk_attno == column->bv_attno);
-
-	/* Handle IS NULL/IS NOT NULL tests. */
-	if (key->sk_flags & SK_ISNULL)
+	/*
+	 * First check if there are any IS NULL scan keys, and if we're
+	 * violating them. In that case we can terminate early, without
+	 * inspecting the ranges.
+	 */
+	for (keyno = 0; keyno < nkeys; keyno++)
 	{
-		if (key->sk_flags & SK_SEARCHNULL)
-		{
-			if (column->bv_allnulls || column->bv_hasnulls)
-				PG_RETURN_BOOL(true);
-			PG_RETURN_BOOL(false);
-		}
+		ScanKey	key = keys[keyno];
 
-		/*
-		 * For IS NOT NULL, we can only skip ranges that are known to have
-		 * only nulls.
-		 */
-		if (key->sk_flags & SK_SEARCHNOTNULL)
-			PG_RETURN_BOOL(!column->bv_allnulls);
-
-		/*
-		 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
-		 * operators are strict and return false.
-		 */
-		PG_RETURN_BOOL(false);
-	}
+		Assert(key->sk_attno == column->bv_attno);
 
-	/* If it is all nulls, it cannot possibly be consistent. */
-	if (column->bv_allnulls)
-		PG_RETURN_BOOL(false);
+		/* handle IS NULL/IS NOT NULL tests */
+		if (key->sk_flags & SK_ISNULL)
+		{
+			if (key->sk_flags & SK_SEARCHNULL)
+			{
+				if (column->bv_allnulls || column->bv_hasnulls)
+					continue;	/* this key is fine, continue */
 
-	/* It has to be checked, if it contains elements that are not mergeable. */
-	if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE]))
-		PG_RETURN_BOOL(true);
+				PG_RETURN_BOOL(false);
+			}
 
-	attno = key->sk_attno;
-	subtype = key->sk_subtype;
-	query = key->sk_argument;
-	unionval = column->bv_values[INCLUSION_UNION];
-	switch (key->sk_strategy)
-	{
 			/*
-			 * Placement strategies
-			 *
-			 * These are implemented by logically negating the result of the
-			 * converse placement operator; for this to work, the converse
-			 * operator must be part of the opclass.  An error will be thrown
-			 * by inclusion_get_strategy_procinfo() if the required strategy
-			 * is not part of the opclass.
-			 *
-			 * These all return false if either argument is empty, so there is
-			 * no need to check for empty elements.
+			 * For IS NOT NULL, we can only skip ranges that are known to have
+			 * only nulls.
 			 */
+			if (key->sk_flags & SK_SEARCHNOTNULL)
+			{
+				if (column->bv_allnulls)
+					PG_RETURN_BOOL(false);
 
-		case RTLeftStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTOverRightStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		case RTOverLeftStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTRightStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		case RTOverRightStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTLeftStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		case RTRightStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTOverLeftStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		case RTBelowStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTOverAboveStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		case RTOverBelowStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTAboveStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		case RTOverAboveStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTBelowStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		case RTAboveStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTOverBelowStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+				continue;
+			}
 
 			/*
-			 * Overlap and contains strategies
-			 *
-			 * These strategies are simple enough that we can simply call the
-			 * operator and return its result.  Empty elements don't change
-			 * the result.
+			 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+			 * operators are strict and return false.
 			 */
+			PG_RETURN_BOOL(false);
+		}
+		else
+			/* note we have regular (non-NULL) scan keys */
+			regular_keys = true;
+	}
 
-		case RTOverlapStrategyNumber:
-		case RTContainsStrategyNumber:
-		case RTOldContainsStrategyNumber:
-		case RTContainsElemStrategyNumber:
-		case RTSubStrategyNumber:
-		case RTSubEqualStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													key->sk_strategy);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_DATUM(result);
-
-			/*
-			 * Contained by strategies
-			 *
-			 * We cannot just call the original operator for the contained by
-			 * strategies because some elements can be contained even though
-			 * the union is not; instead we use the overlap operator.
-			 *
-			 * We check for empty elements separately as they are not merged
-			 * to the union but contained by everything.
-			 */
+	/*
+	 * If the page range is all nulls, it cannot possibly be consistent if
+	 * there are some regular scan keys.
+	 */
+	if (column->bv_allnulls && regular_keys)
+		PG_RETURN_BOOL(false);
 
-		case RTContainedByStrategyNumber:
-		case RTOldContainedByStrategyNumber:
-		case RTSuperStrategyNumber:
-		case RTSuperEqualStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTOverlapStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			if (DatumGetBool(result))
-				PG_RETURN_BOOL(true);
+	/* If there are no regular keys, the page range is considered consistent. */
+	if (!regular_keys)
+		PG_RETURN_BOOL(true);
 
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+	/* It has to be checked, if it contains elements that are not mergeable. */
+	if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE]))
+		PG_RETURN_BOOL(true);
 
-			/*
-			 * Adjacent strategy
-			 *
-			 * We test for overlap first but to be safe we need to call the
-			 * actual adjacent operator also.
-			 *
-			 * An empty element cannot be adjacent to any other, so there is
-			 * no need to check for it.
-			 */
+	matches = true;
 
-		case RTAdjacentStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTOverlapStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			if (DatumGetBool(result))
-				PG_RETURN_BOOL(true);
+	for (keyno = 0; keyno < nkeys; keyno++)
+	{
+		ScanKey	key = keys[keyno];
 
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTAdjacentStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_DATUM(result);
+		/* ignore IS NULL/IS NOT NULL tests handled above */
+		if (key->sk_flags & SK_ISNULL)
+			continue;
 
-			/*
-			 * Basic comparison strategies
-			 *
-			 * It is straightforward to support the equality strategies with
-			 * the contains operator.  Generally, inequality strategies do not
-			 * make much sense for the types which will be used with the
-			 * inclusion BRIN family of opclasses, but is possible to
-			 * implement them with logical negation of the left-of and
-			 * right-of operators.
-			 *
-			 * NB: These strategies cannot be used with geometric datatypes
-			 * that use comparison of areas!  The only exception is the "same"
-			 * strategy.
-			 *
-			 * Empty elements are considered to be less than the others.  We
-			 * cannot use the empty support function to check the query is an
-			 * empty element, because the query can be another data type than
-			 * the empty support function argument.  So we will return true,
-			 * if there is a possibility that empty elements will change the
-			 * result.
-			 */
+		attno = key->sk_attno;
+		subtype = key->sk_subtype;
+		query = key->sk_argument;
+		unionval = column->bv_values[INCLUSION_UNION];
+		switch (key->sk_strategy)
+		{
+				/*
+				* Placement strategies
+				*
+				* These are implemented by logically negating the result of the
+				* converse placement operator; for this to work, the converse
+				* operator must be part of the opclass.  An error will be thrown
+				* by inclusion_get_strategy_procinfo() if the required strategy
+				* is not part of the opclass.
+				*
+				* These all return false if either argument is empty, so there is
+				* no need to check for empty elements.
+				*/
+
+			case RTLeftStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTOverRightStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			case RTOverLeftStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTRightStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			case RTOverRightStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTLeftStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			case RTRightStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTOverLeftStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			case RTBelowStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTOverAboveStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			case RTOverBelowStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTAboveStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			case RTOverAboveStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTBelowStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			case RTAboveStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTOverBelowStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+				/*
+				* Overlap and contains strategies
+				*
+				* These strategies are simple enough that we can simply call the
+				* operator and return its result.  Empty elements don't change
+				* the result.
+				*/
+
+			case RTOverlapStrategyNumber:
+			case RTContainsStrategyNumber:
+			case RTOldContainsStrategyNumber:
+			case RTContainsElemStrategyNumber:
+			case RTSubStrategyNumber:
+			case RTSubEqualStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														key->sk_strategy);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = DatumGetBool(result);
+				break;
+
+				/*
+				* Contained by strategies
+				*
+				* We cannot just call the original operator for the contained by
+				* strategies because some elements can be contained even though
+				* the union is not; instead we use the overlap operator.
+				*
+				* We check for empty elements separately as they are not merged
+				* to the union but contained by everything.
+				*/
+
+			case RTContainedByStrategyNumber:
+			case RTOldContainedByStrategyNumber:
+			case RTSuperStrategyNumber:
+			case RTSuperEqualStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTOverlapStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				if (DatumGetBool(result))
+					matches = true;
+				else
+					matches = DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+
+				break;
+
+				/*
+				* Adjacent strategy
+				*
+				* We test for overlap first but to be safe we need to call the
+				* actual adjacent operator also.
+				*
+				* An empty element cannot be adjacent to any other, so there is
+				* no need to check for it.
+				*/
+
+			case RTAdjacentStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTOverlapStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				if (DatumGetBool(result))
+					matches = (true);
+				else
+				{
+
+					finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+															RTAdjacentStrategyNumber);
+					result = FunctionCall2Coll(finfo, colloid, unionval, query);
+					matches = DatumGetBool(result);
+				}
+
+				break;
+
+				/*
+				* Basic comparison strategies
+				*
+				* It is straightforward to support the equality strategies with
+				* the contains operator.  Generally, inequality strategies do not
+				* make much sense for the types which will be used with the
+				* inclusion BRIN family of opclasses, but is possible to
+				* implement them with logical negation of the left-of and
+				* right-of operators.
+				*
+				* NB: These strategies cannot be used with geometric datatypes
+				* that use comparison of areas!  The only exception is the "same"
+				* strategy.
+				*
+				* Empty elements are considered to be less than the others.  We
+				* cannot use the empty support function to check the query is an
+				* empty element, because the query can be another data type than
+				* the empty support function argument.  So we will return true,
+				* if there is a possibility that empty elements will change the
+				* result.
+				*/
+
+			case RTLessStrategyNumber:
+			case RTLessEqualStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTRightStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				if (!DatumGetBool(result))
+					matches = (true);
+				else
+					matches = DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+				break;
+
+			case RTSameStrategyNumber:
+			case RTEqualStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTContainsStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				if (DatumGetBool(result))
+					matches = (true);
+				else
+					matches = DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+				break;
+
+			case RTGreaterEqualStrategyNumber:
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTLeftStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				if (!DatumGetBool(result))
+					matches = (true);
+				else
+					matches = DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+				break;
+
+			case RTGreaterStrategyNumber:
+				/* no need to check for empty elements */
+				finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
+														RTLeftStrategyNumber);
+				result = FunctionCall2Coll(finfo, colloid, unionval, query);
+				matches = (!DatumGetBool(result));
+				break;
+
+			default:
+				/* shouldn't happen */
+				elog(ERROR, "invalid strategy number %d", key->sk_strategy);
+				matches = (false);
+		}
 
-		case RTLessStrategyNumber:
-		case RTLessEqualStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTRightStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			if (!DatumGetBool(result))
-				PG_RETURN_BOOL(true);
-
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
-
-		case RTSameStrategyNumber:
-		case RTEqualStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTContainsStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			if (DatumGetBool(result))
-				PG_RETURN_BOOL(true);
-
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
-
-		case RTGreaterEqualStrategyNumber:
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTLeftStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			if (!DatumGetBool(result))
-				PG_RETURN_BOOL(true);
-
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
-
-		case RTGreaterStrategyNumber:
-			/* no need to check for empty elements */
-			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTLeftStrategyNumber);
-			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
-
-		default:
-			/* shouldn't happen */
-			elog(ERROR, "invalid strategy number %d", key->sk_strategy);
-			PG_RETURN_BOOL(false);
+		if (!matches)
+			break;
 	}
+
+	PG_RETURN_BOOL(matches);
 }
 
 /*
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 0f6aa33..e58fd16 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -147,86 +147,130 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
 {
 	BrinDesc   *bdesc = (BrinDesc *) PG_GETARG_POINTER(0);
 	BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1);
-	ScanKey		key = (ScanKey) PG_GETARG_POINTER(2);
+	ScanKey	   *keys = (ScanKey *) PG_GETARG_POINTER(2);
+	int			nkeys = PG_GETARG_INT32(3);
 	Oid			colloid = PG_GET_COLLATION(),
 				subtype;
 	AttrNumber	attno;
 	Datum		value;
 	Datum		matches;
 	FmgrInfo   *finfo;
+	int			keyno;
+	bool		regular_keys = false;
 
-	Assert(key->sk_attno == column->bv_attno);
-
-	/* handle IS NULL/IS NOT NULL tests */
-	if (key->sk_flags & SK_ISNULL)
+	/*
+	 * First check if there are any IS NULL scan keys, and if we're
+	 * violating them. In that case we can terminate early, without
+	 * inspecting the ranges.
+	 */
+	for (keyno = 0; keyno < nkeys; keyno++)
 	{
-		if (key->sk_flags & SK_SEARCHNULL)
+		ScanKey	key = keys[keyno];
+
+		Assert(key->sk_attno == column->bv_attno);
+
+		/* handle IS NULL/IS NOT NULL tests */
+		if (key->sk_flags & SK_ISNULL)
 		{
-			if (column->bv_allnulls || column->bv_hasnulls)
-				PG_RETURN_BOOL(true);
+			if (key->sk_flags & SK_SEARCHNULL)
+			{
+				if (column->bv_allnulls || column->bv_hasnulls)
+					continue;	/* this key is fine, continue */
+
+				PG_RETURN_BOOL(false);
+			}
+
+			/*
+			 * For IS NOT NULL, we can only skip ranges that are known to have
+			 * only nulls.
+			 */
+			if (key->sk_flags & SK_SEARCHNOTNULL)
+			{
+				if (column->bv_allnulls)
+					PG_RETURN_BOOL(false);
+
+				continue;
+			}
+
+			/*
+			 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+			 * operators are strict and return false.
+			 */
 			PG_RETURN_BOOL(false);
 		}
-
-		/*
-		 * For IS NOT NULL, we can only skip ranges that are known to have
-		 * only nulls.
-		 */
-		if (key->sk_flags & SK_SEARCHNOTNULL)
-			PG_RETURN_BOOL(!column->bv_allnulls);
-
-		/*
-		 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
-		 * operators are strict and return false.
-		 */
-		PG_RETURN_BOOL(false);
+		else
+			/* note we have regular (non-NULL) scan keys */
+			regular_keys = true;
 	}
 
-	/* if the range is all empty, it cannot possibly be consistent */
-	if (column->bv_allnulls)
+	/*
+	 * If the page range is all nulls, it cannot possibly be consistent if
+	 * there are some regular scan keys.
+	 */
+	if (column->bv_allnulls && regular_keys)
 		PG_RETURN_BOOL(false);
 
-	attno = key->sk_attno;
-	subtype = key->sk_subtype;
-	value = key->sk_argument;
-	switch (key->sk_strategy)
+	/* If there are no regular keys, the page range is considered consistent. */
+	if (!regular_keys)
+		PG_RETURN_BOOL(true);
+
+	matches = true;
+
+	for (keyno = 0; keyno < nkeys; keyno++)
 	{
-		case BTLessStrategyNumber:
-		case BTLessEqualStrategyNumber:
-			finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
-												 key->sk_strategy);
-			matches = FunctionCall2Coll(finfo, colloid, column->bv_values[0],
-										value);
-			break;
-		case BTEqualStrategyNumber:
+		ScanKey	key = keys[keyno];
 
-			/*
-			 * In the equality case (WHERE col = someval), we want to return
-			 * the current page range if the minimum value in the range <=
-			 * scan key, and the maximum value >= scan key.
-			 */
-			finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
-												 BTLessEqualStrategyNumber);
-			matches = FunctionCall2Coll(finfo, colloid, column->bv_values[0],
-										value);
-			if (!DatumGetBool(matches))
+		/* ignore IS NULL/IS NOT NULL tests handled above */
+		if (key->sk_flags & SK_ISNULL)
+			continue;
+
+		attno = key->sk_attno;
+		subtype = key->sk_subtype;
+		value = key->sk_argument;
+		switch (key->sk_strategy)
+		{
+			case BTLessStrategyNumber:
+			case BTLessEqualStrategyNumber:
+				finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
+													key->sk_strategy);
+				matches = FunctionCall2Coll(finfo, colloid, column->bv_values[0],
+											value);
 				break;
-			/* max() >= scankey */
-			finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
-												 BTGreaterEqualStrategyNumber);
-			matches = FunctionCall2Coll(finfo, colloid, column->bv_values[1],
-										value);
-			break;
-		case BTGreaterEqualStrategyNumber:
-		case BTGreaterStrategyNumber:
-			finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
-												 key->sk_strategy);
-			matches = FunctionCall2Coll(finfo, colloid, column->bv_values[1],
-										value);
-			break;
-		default:
-			/* shouldn't happen */
-			elog(ERROR, "invalid strategy number %d", key->sk_strategy);
-			matches = 0;
+			case BTEqualStrategyNumber:
+
+				/*
+				* In the equality case (WHERE col = someval), we want to return
+				* the current page range if the minimum value in the range <=
+				* scan key, and the maximum value >= scan key.
+				*/
+				finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
+													BTLessEqualStrategyNumber);
+				matches = FunctionCall2Coll(finfo, colloid, column->bv_values[0],
+											value);
+				if (!DatumGetBool(matches))
+					break;
+				/* max() >= scankey */
+				finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
+													BTGreaterEqualStrategyNumber);
+				matches = FunctionCall2Coll(finfo, colloid, column->bv_values[1],
+											value);
+				break;
+			case BTGreaterEqualStrategyNumber:
+			case BTGreaterStrategyNumber:
+				finfo = minmax_get_strategy_procinfo(bdesc, attno, subtype,
+													key->sk_strategy);
+				matches = FunctionCall2Coll(finfo, colloid, column->bv_values[1],
+											value);
+				break;
+			default:
+				/* shouldn't happen */
+				elog(ERROR, "invalid strategy number %d", key->sk_strategy);
+				matches = 0;
+				break;
+		}
+
+		/* found non-matching key */
+		if (!matches)
 			break;
 	}
 
-- 
2.9.5

