public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
Subject: [PATCH 3/8] Process all scan keys in existing BRIN opclasses
Date: Fri, 5 Mar 2021 00:45:33 +0100
This modifies the existing BRIN opclasses (minmax, inclusion) although
those don't really benefit from this change. The primary purpose of this
is to allow more advanced opclases in the future.
---
src/backend/access/brin/brin_inclusion.c | 140 ++++++++++++++++-------
src/backend/access/brin/brin_minmax.c | 92 ++++++++++++---
src/include/catalog/pg_proc.dat | 4 +-
3 files changed, 177 insertions(+), 59 deletions(-)
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c
index 12e5bddd1f..a260074c91 100644
--- a/src/backend/access/brin/brin_inclusion.c
+++ b/src/backend/access/brin/brin_inclusion.c
@@ -85,6 +85,8 @@ static FmgrInfo *inclusion_get_procinfo(BrinDesc *bdesc, uint16 attno,
uint16 procnum);
static FmgrInfo *inclusion_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno,
Oid subtype, uint16 strategynum);
+static bool inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column,
+ ScanKey key, Oid colloid);
/*
@@ -251,6 +253,10 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS)
/*
* BRIN inclusion consistent function
*
+ * We inspect the IS NULL scan keys first, which allows us to make a decision
+ * without looking at the contents of the page range. Only when the page range
+ * matches all those keys, we check the regular scan keys.
+ *
* All of the strategies are optional.
*/
Datum
@@ -258,24 +264,31 @@ 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);
- Oid colloid = PG_GET_COLLATION(),
- subtype;
- Datum unionval;
- AttrNumber attno;
- Datum query;
- FmgrInfo *finfo;
- Datum result;
-
- Assert(key->sk_attno == column->bv_attno);
+ ScanKey *keys = (ScanKey *) PG_GETARG_POINTER(2);
+ int nkeys = PG_GETARG_INT32(3);
+ Oid colloid = PG_GET_COLLATION();
+ int keyno;
+ bool has_regular_keys = false;
- /* Handle IS NULL/IS NOT NULL tests. */
- if (key->sk_flags & SK_ISNULL)
+ /* Handle IS NULL/IS NOT NULL tests */
+ for (keyno = 0; keyno < nkeys; keyno++)
{
+ ScanKey key = keys[keyno];
+
+ Assert(key->sk_attno == column->bv_attno);
+
+ /* Skip regular scan keys (and remember that we have some). */
+ if ((!key->sk_flags & SK_ISNULL))
+ {
+ has_regular_keys = true;
+ continue;
+ }
+
if (key->sk_flags & SK_SEARCHNULL)
{
if (column->bv_allnulls || column->bv_hasnulls)
- PG_RETURN_BOOL(true);
+ continue; /* this key is fine, continue */
+
PG_RETURN_BOOL(false);
}
@@ -284,7 +297,12 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
* only nulls.
*/
if (key->sk_flags & SK_SEARCHNOTNULL)
- PG_RETURN_BOOL(!column->bv_allnulls);
+ {
+ if (column->bv_allnulls)
+ PG_RETURN_BOOL(false);
+
+ continue;
+ }
/*
* Neither IS NULL nor IS NOT NULL was used; assume all indexable
@@ -293,7 +311,14 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(false);
}
- /* If it is all nulls, it cannot possibly be consistent. */
+ /* If there are no regular keys, the page range is considered consistent. */
+ if (!has_regular_keys)
+ PG_RETURN_BOOL(true);
+
+ /*
+ * If is all nulls, it cannot possibly be consistent (at this point we
+ * know there are at least some regular scan keys).
+ */
if (column->bv_allnulls)
PG_RETURN_BOOL(false);
@@ -301,10 +326,45 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE]))
PG_RETURN_BOOL(true);
- attno = key->sk_attno;
- subtype = key->sk_subtype;
- query = key->sk_argument;
- unionval = column->bv_values[INCLUSION_UNION];
+ /* Check that the range is consistent with all regular scan keys. */
+ for (keyno = 0; keyno < nkeys; keyno++)
+ {
+ ScanKey key = keys[keyno];
+
+ /* Skip IS NULL/IS NOT NULL keys (already handled above). */
+ if (key->sk_flags & SK_ISNULL)
+ continue;
+
+ /*
+ * When there are multiple scan keys, failure to meet the criteria for
+ * a single one of them is enough to discard the range as a whole, so
+ * break out of the loop as soon as a false return value is obtained.
+ */
+ if (!inclusion_consistent_key(bdesc, column, key, colloid))
+ PG_RETURN_BOOL(false);
+ }
+
+ PG_RETURN_BOOL(true);
+}
+
+/*
+ * inclusion_consistent_key
+ * Determine if the range is consistent with a single scan key.
+ */
+static bool
+inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
+ Oid colloid)
+{
+ FmgrInfo *finfo;
+ AttrNumber attno = key->sk_attno;
+ Oid subtype = key->sk_subtype;
+ Datum query = key->sk_argument;
+ Datum unionval = column->bv_values[INCLUSION_UNION];
+ Datum result;
+
+ /* This should be called only for regular keys, not for IS [NOT] NULL. */
+ Assert(!(key->sk_flags & SK_ISNULL));
+
switch (key->sk_strategy)
{
/*
@@ -324,49 +384,49 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTOverRightStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
case RTOverLeftStrategyNumber:
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTRightStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
case RTOverRightStrategyNumber:
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTLeftStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
case RTRightStrategyNumber:
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTOverLeftStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
case RTBelowStrategyNumber:
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTOverAboveStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
case RTOverBelowStrategyNumber:
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTAboveStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
case RTOverAboveStrategyNumber:
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTBelowStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
case RTAboveStrategyNumber:
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTOverBelowStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_BOOL(!DatumGetBool(result));
+ return !DatumGetBool(result);
/*
* Overlap and contains strategies
@@ -384,7 +444,7 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
key->sk_strategy);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_DATUM(result);
+ return DatumGetBool(result);
/*
* Contained by strategies
@@ -404,9 +464,9 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
RTOverlapStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
if (DatumGetBool(result))
- PG_RETURN_BOOL(true);
+ return true;
- PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+ return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
/*
* Adjacent strategy
@@ -423,12 +483,12 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
RTOverlapStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
if (DatumGetBool(result))
- PG_RETURN_BOOL(true);
+ return true;
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
RTAdjacentStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
- PG_RETURN_DATUM(result);
+ return DatumGetBool(result);
/*
* Basic comparison strategies
@@ -458,9 +518,9 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
RTRightStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
if (!DatumGetBool(result))
- PG_RETURN_BOOL(true);
+ return true;
- PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+ return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
case RTSameStrategyNumber:
case RTEqualStrategyNumber:
@@ -468,30 +528,30 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
RTContainsStrategyNumber);
result = FunctionCall2Coll(finfo, colloid, unionval, query);
if (DatumGetBool(result))
- PG_RETURN_BOOL(true);
+ return true;
- PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+ return DatumGetBool(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);
+ return true;
- PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+ return DatumGetBool(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));
+ return !DatumGetBool(result);
default:
/* shouldn't happen */
elog(ERROR, "invalid strategy number %d", key->sk_strategy);
- PG_RETURN_BOOL(false);
+ return false;
}
}
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 2ffbd9bf0d..e116084a02 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -30,6 +30,8 @@ typedef struct MinmaxOpaque
static FmgrInfo *minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno,
Oid subtype, uint16 strategynum);
+static bool minmax_consistent_key(BrinDesc *bdesc, BrinValues *column,
+ ScanKey key, Oid colloid);
Datum
@@ -140,29 +142,41 @@ brin_minmax_add_value(PG_FUNCTION_ARGS)
* Given an index tuple corresponding to a certain page range and a scan key,
* return whether the scan key is consistent with the index tuple's min/max
* values. Return true if so, false otherwise.
+ *
+ * We inspect the IS NULL scan keys first, which allows us to make a decision
+ * without looking at the contents of the page range. Only when the page range
+ * matches all those keys, we check the regular scan keys.
*/
Datum
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);
- Oid colloid = PG_GET_COLLATION(),
- subtype;
- AttrNumber attno;
- Datum value;
- Datum matches;
- FmgrInfo *finfo;
-
- Assert(key->sk_attno == column->bv_attno);
+ ScanKey *keys = (ScanKey *) PG_GETARG_POINTER(2);
+ int nkeys = PG_GETARG_INT32(3);
+ Oid colloid = PG_GET_COLLATION();
+ int keyno;
+ bool has_regular_keys = false;
/* handle IS NULL/IS NOT NULL tests */
- if (key->sk_flags & SK_ISNULL)
+ for (keyno = 0; keyno < nkeys; keyno++)
{
+ ScanKey key = keys[keyno];
+
+ Assert(key->sk_attno == column->bv_attno);
+
+ /* Skip regular scan keys (and remember that we have some). */
+ if ((!key->sk_flags & SK_ISNULL))
+ {
+ has_regular_keys = true;
+ continue;
+ }
+
if (key->sk_flags & SK_SEARCHNULL)
{
if (column->bv_allnulls || column->bv_hasnulls)
- PG_RETURN_BOOL(true);
+ continue; /* this key is fine, continue */
+
PG_RETURN_BOOL(false);
}
@@ -171,7 +185,12 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
* only nulls.
*/
if (key->sk_flags & SK_SEARCHNOTNULL)
- PG_RETURN_BOOL(!column->bv_allnulls);
+ {
+ if (column->bv_allnulls)
+ PG_RETURN_BOOL(false);
+
+ continue;
+ }
/*
* Neither IS NULL nor IS NOT NULL was used; assume all indexable
@@ -180,13 +199,52 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(false);
}
- /* if the range is all empty, it cannot possibly be consistent */
+ /* If there are no regular keys, the page range is considered consistent. */
+ if (!has_regular_keys)
+ PG_RETURN_BOOL(true);
+
+ /*
+ * If is all nulls, it cannot possibly be consistent (at this point we
+ * know there are at least some regular scan keys).
+ */
if (column->bv_allnulls)
PG_RETURN_BOOL(false);
- attno = key->sk_attno;
- subtype = key->sk_subtype;
- value = key->sk_argument;
+ /* Check that the range is consistent with all scan keys. */
+ for (keyno = 0; keyno < nkeys; keyno++)
+ {
+ ScanKey key = keys[keyno];
+
+ /* ignore IS NULL/IS NOT NULL tests handled above */
+ if (key->sk_flags & SK_ISNULL)
+ continue;
+
+ /*
+ * When there are multiple scan keys, failure to meet the criteria for
+ * a single one of them is enough to discard the range as a whole, so
+ * break out of the loop as soon as a false return value is obtained.
+ */
+ if (!minmax_consistent_key(bdesc, column, key, colloid))
+ PG_RETURN_DATUM(false);;
+ }
+
+ PG_RETURN_DATUM(true);
+}
+
+/*
+ * minmax_consistent_key
+ * Determine if the range is consistent with a single scan key.
+ */
+static bool
+minmax_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
+ Oid colloid)
+{
+ FmgrInfo *finfo;
+ AttrNumber attno = key->sk_attno;
+ Oid subtype = key->sk_subtype;
+ Datum value = key->sk_argument;
+ Datum matches;
+
switch (key->sk_strategy)
{
case BTLessStrategyNumber:
@@ -229,7 +287,7 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
break;
}
- PG_RETURN_DATUM(matches);
+ return DatumGetBool(matches);
}
/*
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 59d2b71ca9..93bdb5ec83 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8194,7 +8194,7 @@
prosrc => 'brin_minmax_add_value' },
{ oid => '3385', descr => 'BRIN minmax support',
proname => 'brin_minmax_consistent', prorettype => 'bool',
- proargtypes => 'internal internal internal',
+ proargtypes => 'internal internal internal int4',
prosrc => 'brin_minmax_consistent' },
{ oid => '3386', descr => 'BRIN minmax support',
proname => 'brin_minmax_union', prorettype => 'bool',
@@ -8210,7 +8210,7 @@
prosrc => 'brin_inclusion_add_value' },
{ oid => '4107', descr => 'BRIN inclusion support',
proname => 'brin_inclusion_consistent', prorettype => 'bool',
- proargtypes => 'internal internal internal',
+ proargtypes => 'internal internal internal int4',
prosrc => 'brin_inclusion_consistent' },
{ oid => '4108', descr => 'BRIN inclusion support',
proname => 'brin_inclusion_union', prorettype => 'bool',
--
2.26.2
--------------AC3FFB734F56C5F52D422EFC
Content-Type: text/x-patch; charset=UTF-8;
name="0004-Move-IS-NOT-NULL-handling-from-BRIN-support-20210305.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0004-Move-IS-NOT-NULL-handling-from-BRIN-support-20210305.pa";
filename*1="tch"
view thread (78+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 3/8] Process all scan keys in existing BRIN opclasses
In-Reply-To: <no-message-id-1872479@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox