agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH 1/6] Pass all scan keys to BRIN consistent function at once 4+ messages / 3 participants [nested] [flat]
* [PATCH 1/6] Pass all scan keys to BRIN consistent function at once @ 2020-09-12 13:07 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw) Passing all scan keys to the BRIN consistent function at once may allow elimination of additional ranges, which would be impossible when only passing individual scan keys. The code continues to support both the original (one scan key at a time) and new (all scan keys at once) approaches, depending on whether the consistent function accepts three or four arguments. 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. Author: Tomas Vondra <[email protected]> Reviewed-by: Alvaro Herrera <[email protected]> Reviewed-by: Mark Dilger <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/brin/brin.c | 150 +++++++++++++++----- src/backend/access/brin/brin_inclusion.c | 170 +++++++++++++++-------- src/backend/access/brin/brin_minmax.c | 121 +++++++++++----- src/backend/access/brin/brin_validate.c | 4 +- src/include/catalog/pg_proc.dat | 4 +- 5 files changed, 324 insertions(+), 125 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 27ba596c6e..dc187153aa 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -390,6 +390,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; @@ -411,6 +414,61 @@ 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; + + /* + * This is a bit of an overkill - we don't know how many + * scan keys are there for this attribute, so we simply + * allocate the largest number possible. This may waste + * a bit of memory, but we only expect small number of + * scan keys in general, so this should be negligible, + * and it's cheaper than having to repalloc repeatedly. + */ + 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); @@ -471,7 +529,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) } else { - int keyno; + int attno; /* * Compare scan keys with summary values stored for the range. @@ -481,51 +539,75 @@ 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; - /* - * 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)); + /* skip attributes without any san keys */ + if (nkeys[attno - 1] == 0) + continue; - /* First time this column? look up consistent function */ - if (consistentFn[keyattno - 1].fn_oid == InvalidOid) - { - FmgrInfo *tmp; + bval = &dtup->bt_columns[attno - 1]; - tmp = index_getprocinfo(idxRel, keyattno, - BRIN_PROCNUM_CONSISTENT); - fmgr_info_copy(&consistentFn[keyattno - 1], tmp, - CurrentMemoryContext); - } + Assert((nkeys[attno - 1] > 0) && + (nkeys[attno - 1] <= scan->numberOfKeys)); /* * Check whether the scan key is consistent with the page * range values; if so, have the pages in the range added * to the output bitmap. * - * 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. + * The opclass may or may not support processing of multiple + * scan keys. We can determine that based on the number of + * arguments - functions with extra parameter (number of scan + * keys) do support this, otherwise we have to simply pass the + * scan keys one by one, */ - add = FunctionCall3Coll(&consistentFn[keyattno - 1], - key->sk_collation, - PointerGetDatum(bdesc), - PointerGetDatum(bval), - PointerGetDatum(key)); - addrange = DatumGetBool(add); + if (consistentFn[attno - 1].fn_nargs >= 4) + { + Oid collation; + + /* + * Collation from the first key (has to be the same for + * all keys for the same attribue). + */ + collation = keys[attno - 1][0]->sk_collation; + + /* Check all keys at once */ + add = FunctionCall4Coll(&consistentFn[attno - 1], + collation, + PointerGetDatum(bdesc), + PointerGetDatum(bval), + PointerGetDatum(keys[attno - 1]), + Int32GetDatum(nkeys[attno - 1])); + addrange = DatumGetBool(add); + } + else + { + /* + * Check keys one by one + * + * 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. + */ + int keyno; + + for (keyno = 0; keyno < nkeys[attno - 1]; keyno++) + { + add = FunctionCall3Coll(&consistentFn[attno - 1], + keys[attno - 1][keyno]->sk_collation, + PointerGetDatum(bdesc), + PointerGetDatum(bval), + PointerGetDatum(keys[attno - 1][keyno])); + addrange = DatumGetBool(add); + if (!addrange) + break; + } + } + if (!addrange) break; } diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c index 12e5bddd1f..215bc794d3 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); /* @@ -258,53 +260,109 @@ 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 regular_keys = false; - /* 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); - PG_RETURN_BOOL(false); - } + if (key->sk_flags & SK_SEARCHNULL) + { + if (column->bv_allnulls || column->bv_hasnulls) + continue; /* this key is fine, continue */ - /* - * 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); + PG_RETURN_BOOL(false); + } - /* - * 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) + { + 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); + } + else + /* note we have regular (non-NULL) scan keys */ + regular_keys = true; } - /* If it is all nulls, 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); + /* If there are no regular keys, the page range is considered consistent. */ + if (!regular_keys) + PG_RETURN_BOOL(true); + /* It has to be checked, if it contains elements that are not mergeable. */ 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 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 (!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; + switch (key->sk_strategy) { /* @@ -324,49 +382,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 +442,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 +462,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 +481,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); + RTAdjacentStrategyNumber); result = FunctionCall2Coll(finfo, colloid, unionval, query); - PG_RETURN_DATUM(result); + return DatumGetBool(result); /* * Basic comparison strategies @@ -458,9 +516,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 +526,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..12878ff3a0 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 @@ -146,47 +148,104 @@ 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 regular_keys = false; - /* 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); } + else + /* note we have regular (non-NULL) scan keys */ + regular_keys = true; + } - /* - * 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); + /* + * 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); + + /* If there are no regular keys, the page range is considered consistent. */ + if (!regular_keys) + PG_RETURN_BOOL(true); - /* - * Neither IS NULL nor IS NOT NULL was used; assume all indexable - * operators are strict and return false. + /* 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. */ - PG_RETURN_BOOL(false); + if (!minmax_consistent_key(bdesc, column, key, colloid)) + PG_RETURN_DATUM(false);; } - /* if the range is all empty, it cannot possibly be consistent */ - if (column->bv_allnulls) - PG_RETURN_BOOL(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; - attno = key->sk_attno; - subtype = key->sk_subtype; - value = key->sk_argument; switch (key->sk_strategy) { case BTLessStrategyNumber: @@ -229,7 +288,7 @@ brin_minmax_consistent(PG_FUNCTION_ARGS) break; } - PG_RETURN_DATUM(matches); + return DatumGetBool(matches); } /* diff --git a/src/backend/access/brin/brin_validate.c b/src/backend/access/brin/brin_validate.c index 6d4253c05e..11835d85cd 100644 --- a/src/backend/access/brin/brin_validate.c +++ b/src/backend/access/brin/brin_validate.c @@ -97,8 +97,8 @@ brinvalidate(Oid opclassoid) break; case BRIN_PROCNUM_CONSISTENT: ok = check_amproc_signature(procform->amproc, BOOLOID, true, - 3, 3, INTERNALOID, INTERNALOID, - INTERNALOID); + 3, 4, INTERNALOID, INTERNALOID, + INTERNALOID, INT4OID); break; case BRIN_PROCNUM_UNION: ok = check_amproc_signature(procform->amproc, BOOLOID, true, diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index d27336adcd..f3ead7d53d 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -8145,7 +8145,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', @@ -8161,7 +8161,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 --------------310A2AE1CC4C2E2E77559E3D Content-Type: text/x-patch; charset=UTF-8; name="0002-Move-IS-NOT-NULL-handling-from-BRIN-suppo-20210114-2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Move-IS-NOT-NULL-handling-from-BRIN-suppo-20210114-2.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* Extend pgbench partitioning to pgbench_history @ 2023-11-30 10:29 Gabriele Bartolini <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Gabriele Bartolini @ 2023-11-30 10:29 UTC (permalink / raw) To: pgsql-hackers Hi there, While benchmarking a new feature involving tablespace support in CloudNativePG (Kubernetes operator), I wanted to try out the partitioning feature of pgbench. I saw it supporting both range and hash partitioning, but limited to pgbench_accounts. With the attached patch, I extend the partitioning capability to the pgbench_history table too. I have been thinking of adding an option to control this, but I preferred to ask in this list whether it really makes sense or not (I struggle indeed to see use cases where accounts is partitioned and history is not). Please let me know what you think. Thanks, Gabriele -- Gabriele Bartolini Vice President, Cloud Native at EDB enterprisedb.com Attachments: [application/octet-stream] 0001-Include-pgbench_history-in-partitioning-method-for-p.patch (7.3K, ../../CA+VUV5q9=N64fduq660bvFGQeFQcfqU2xh+XR_1OaPj87io3sg@mail.gmail.com/3-0001-Include-pgbench_history-in-partitioning-method-for-p.patch) download | inline diff: From ba8f507b126a9c5bd22dd40bb8ce0c1f0c43ac59 Mon Sep 17 00:00:00 2001 From: Gabriele Bartolini <[email protected]> Date: Thu, 30 Nov 2023 11:02:39 +0100 Subject: [PATCH] Include pgbench_history in partitioning method for pgbench In case partitioning, make sure that pgbench_history is also partitioned with the same criteria. Signed-off-by: Gabriele Bartolini <[email protected]> --- doc/src/sgml/ref/pgbench.sgml | 10 +++---- src/bin/pgbench/pgbench.c | 53 +++++++++++++++++++++-------------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 05d3f81619..4c02d2a61d 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -365,8 +365,8 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <term><option>--partition-method=<replaceable>NAME</replaceable></option></term> <listitem> <para> - Create a partitioned <literal>pgbench_accounts</literal> table with - <replaceable>NAME</replaceable> method. + Create partitioned <literal>pgbench_accounts</literal> and <literal>pgbench_history</literal> + tables with <replaceable>NAME</replaceable> method. Expected values are <literal>range</literal> or <literal>hash</literal>. This option requires that <option>--partitions</option> is set to non-zero. If unspecified, default is <literal>range</literal>. @@ -378,9 +378,9 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <term><option>--partitions=<replaceable>NUM</replaceable></option></term> <listitem> <para> - Create a partitioned <literal>pgbench_accounts</literal> table with - <replaceable>NUM</replaceable> partitions of nearly equal size for - the scaled number of accounts. + Create partitioned <literal>pgbench_accounts</literal> and <literal>pgbench_history</literal> + tables with <replaceable>NUM</replaceable> partitions of nearly equal size for + the scaled number of accounts - and future history records. Default is <literal>0</literal>, meaning no partitioning. </para> </listitem> diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 2e1650d0ad..87adaf4d8f 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -217,8 +217,8 @@ char *tablespace = NULL; char *index_tablespace = NULL; /* - * Number of "pgbench_accounts" partitions. 0 is the default and means no - * partitioning. + * Number of "pgbench_accounts" and "pgbench_history" partitions. + * 0 is the default and means no partitioning. */ static int partitions = 0; @@ -889,8 +889,10 @@ usage(void) " --index-tablespace=TABLESPACE\n" " create indexes in the specified tablespace\n" " --partition-method=(range|hash)\n" - " partition pgbench_accounts with this method (default: range)\n" - " --partitions=NUM partition pgbench_accounts into NUM parts (default: 0)\n" + " partition pgbench_accounts and pgbench_history with this method" + " (default: range)." + " --partitions=NUM partition pgbench_accounts and pgbench_history into NUM parts" + " (default: 0)\n" " --tablespace=TABLESPACE create tables in the specified tablespace\n" " --unlogged-tables create tables as unlogged tables\n" "\nOptions to select what to run:\n" @@ -4697,20 +4699,22 @@ initDropTables(PGconn *con) } /* - * Create "pgbench_accounts" partitions if needed. + * Create "pgbench_accounts" and/or "pgbench_history" partitions if needed. * * This is the larger table of pgbench default tpc-b like schema - * with a known size, so we choose to partition it. + * with a known size, so we choose to partition it. In case of hash + * method, we also partition the history table, which is instead + * dynamically populated. */ static void -createPartitions(PGconn *con) +createPartitions(PGconn *con, const char *table) { PQExpBufferData query; /* we must have to create some partitions */ Assert(partitions > 0); - fprintf(stderr, "creating %d partitions...\n", partitions); + fprintf(stderr, "creating %d partitions for table %s ...\n", partitions, table); initPQExpBuffer(&query); @@ -4721,10 +4725,10 @@ createPartitions(PGconn *con) int64 part_size = (naccounts * (int64) scale + partitions - 1) / partitions; printfPQExpBuffer(&query, - "create%s table pgbench_accounts_%d\n" - " partition of pgbench_accounts\n" + "create%s table %s_%d\n" + " partition of %s\n" " for values from (", - unlogged_tables ? " unlogged" : "", p); + unlogged_tables ? " unlogged" : "", table, p, table); /* * For RANGE, we use open-ended partitions at the beginning and @@ -4748,11 +4752,11 @@ createPartitions(PGconn *con) } else if (partition_method == PART_HASH) printfPQExpBuffer(&query, - "create%s table pgbench_accounts_%d\n" - " partition of pgbench_accounts\n" + "create%s table %s_%d\n" + " partition of %s\n" " for values with (modulus %d, remainder %d)", - unlogged_tables ? " unlogged" : "", p, - partitions, p - 1); + unlogged_tables ? " unlogged" : "", table, p, + table, partitions, p - 1); else /* cannot get there */ Assert(0); @@ -4760,7 +4764,8 @@ createPartitions(PGconn *con) * Per ddlinfo in initCreateTables, fillfactor is needed on table * pgbench_accounts. */ - appendPQExpBuffer(&query, " with (fillfactor=%d)", fillfactor); + if (strcmp(table, "pgbench_accounts") == 0) + appendPQExpBuffer(&query, " with (fillfactor=%d)", fillfactor); executeStatement(con, query.data); } @@ -4836,9 +4841,9 @@ initCreateTables(PGconn *con) (scale >= SCALE_32BIT_THRESHOLD) ? ddl->bigcols : ddl->smcols); /* Partition pgbench_accounts table */ - if (partition_method != PART_NONE && strcmp(ddl->table, "pgbench_accounts") == 0) - appendPQExpBuffer(&query, - " partition by %s (aid)", PARTITION_METHOD[partition_method]); + if (partition_method != PART_NONE && ( + strcmp(ddl->table, "pgbench_accounts") == 0 || strcmp(ddl->table, "pgbench_history") == 0)) + appendPQExpBuffer(&query, " partition by %s (aid)", PARTITION_METHOD[partition_method]); else if (ddl->declare_fillfactor) { /* fillfactor is only expected on actual tables */ @@ -4860,7 +4865,10 @@ initCreateTables(PGconn *con) termPQExpBuffer(&query); if (partition_method != PART_NONE) - createPartitions(con); + { + createPartitions(con, "pgbench_accounts"); + createPartitions(con, "pgbench_history"); + } } /* @@ -7225,7 +7233,10 @@ main(int argc, char **argv) fprintf(stderr, "starting vacuum..."); tryExecuteStatement(con, "vacuum pgbench_branches"); tryExecuteStatement(con, "vacuum pgbench_tellers"); - tryExecuteStatement(con, "truncate pgbench_history"); + if (partitions > 0) + tryExecuteStatement(con, "truncate pgbench_history CASCADE"); + else + tryExecuteStatement(con, "truncate pgbench_history"); fprintf(stderr, "end.\n"); if (do_vacuum_accounts) -- 2.43.0 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Extend pgbench partitioning to pgbench_history @ 2023-11-30 11:01 Gabriele Bartolini <[email protected]> parent: Gabriele Bartolini <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Gabriele Bartolini @ 2023-11-30 11:01 UTC (permalink / raw) To: pgsql-hackers Please discard the previous patch and use this one (it had a leftover comment from an initial attempt to limit this to hash case). Thanks, Gabriele On Thu, 30 Nov 2023 at 11:29, Gabriele Bartolini < [email protected]> wrote: > Hi there, > > While benchmarking a new feature involving tablespace support in > CloudNativePG (Kubernetes operator), I wanted to try out the partitioning > feature of pgbench. I saw it supporting both range and hash partitioning, > but limited to pgbench_accounts. > > With the attached patch, I extend the partitioning capability to the > pgbench_history table too. > > I have been thinking of adding an option to control this, but I preferred > to ask in this list whether it really makes sense or not (I struggle indeed > to see use cases where accounts is partitioned and history is not). > > Please let me know what you think. > > Thanks, > Gabriele > -- > Gabriele Bartolini > Vice President, Cloud Native at EDB > enterprisedb.com > -- Gabriele Bartolini Vice President, Cloud Native at EDB enterprisedb.com Attachments: [application/octet-stream] 0001-Include-pgbench_history-in-partitioning-method-for-p.patch (7.4K, ../../CA+VUV5qZP3emvE0MkhDEo3jk3YVXkZ5y1hegFTGL3E1t67w3Uw@mail.gmail.com/3-0001-Include-pgbench_history-in-partitioning-method-for-p.patch) download | inline diff: From 4c8121d88aacb1f09236cb1a32a82dbf11c421be Mon Sep 17 00:00:00 2001 From: Gabriele Bartolini <[email protected]> Date: Thu, 30 Nov 2023 11:02:39 +0100 Subject: [PATCH] Include pgbench_history in partitioning method for pgbench In case partitioning, make sure that pgbench_history is also partitioned with the same criteria. Signed-off-by: Gabriele Bartolini <[email protected]> --- doc/src/sgml/ref/pgbench.sgml | 10 +++---- src/bin/pgbench/pgbench.c | 55 +++++++++++++++++++++-------------- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 05d3f81619..4c02d2a61d 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -365,8 +365,8 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <term><option>--partition-method=<replaceable>NAME</replaceable></option></term> <listitem> <para> - Create a partitioned <literal>pgbench_accounts</literal> table with - <replaceable>NAME</replaceable> method. + Create partitioned <literal>pgbench_accounts</literal> and <literal>pgbench_history</literal> + tables with <replaceable>NAME</replaceable> method. Expected values are <literal>range</literal> or <literal>hash</literal>. This option requires that <option>--partitions</option> is set to non-zero. If unspecified, default is <literal>range</literal>. @@ -378,9 +378,9 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <term><option>--partitions=<replaceable>NUM</replaceable></option></term> <listitem> <para> - Create a partitioned <literal>pgbench_accounts</literal> table with - <replaceable>NUM</replaceable> partitions of nearly equal size for - the scaled number of accounts. + Create partitioned <literal>pgbench_accounts</literal> and <literal>pgbench_history</literal> + tables with <replaceable>NUM</replaceable> partitions of nearly equal size for + the scaled number of accounts - and future history records. Default is <literal>0</literal>, meaning no partitioning. </para> </listitem> diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 2e1650d0ad..677cbee5ae 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -217,8 +217,8 @@ char *tablespace = NULL; char *index_tablespace = NULL; /* - * Number of "pgbench_accounts" partitions. 0 is the default and means no - * partitioning. + * Number of "pgbench_accounts" and "pgbench_history" partitions. + * 0 is the default and means no partitioning. */ static int partitions = 0; @@ -889,8 +889,10 @@ usage(void) " --index-tablespace=TABLESPACE\n" " create indexes in the specified tablespace\n" " --partition-method=(range|hash)\n" - " partition pgbench_accounts with this method (default: range)\n" - " --partitions=NUM partition pgbench_accounts into NUM parts (default: 0)\n" + " partition pgbench_accounts and pgbench_history with this method" + " (default: range)." + " --partitions=NUM partition pgbench_accounts and pgbench_history into NUM parts" + " (default: 0)\n" " --tablespace=TABLESPACE create tables in the specified tablespace\n" " --unlogged-tables create tables as unlogged tables\n" "\nOptions to select what to run:\n" @@ -4697,20 +4699,22 @@ initDropTables(PGconn *con) } /* - * Create "pgbench_accounts" partitions if needed. + * Create "pgbench_accounts" and/or "pgbench_history" partitions if needed. * - * This is the larger table of pgbench default tpc-b like schema - * with a known size, so we choose to partition it. + * "pgbench_accounts" is the larger table of pgbench default tpc-b like schema + * with a known size, so we choose to partition it. "pgbench_history" on the + * contrary is dynamically populated when pgbench runs, and we partition it + * based on the same criteria used in "pgbench_accounts". */ static void -createPartitions(PGconn *con) +createPartitions(PGconn *con, const char *table) { PQExpBufferData query; /* we must have to create some partitions */ Assert(partitions > 0); - fprintf(stderr, "creating %d partitions...\n", partitions); + fprintf(stderr, "creating %d partitions for table %s ...\n", partitions, table); initPQExpBuffer(&query); @@ -4721,10 +4725,10 @@ createPartitions(PGconn *con) int64 part_size = (naccounts * (int64) scale + partitions - 1) / partitions; printfPQExpBuffer(&query, - "create%s table pgbench_accounts_%d\n" - " partition of pgbench_accounts\n" + "create%s table %s_%d\n" + " partition of %s\n" " for values from (", - unlogged_tables ? " unlogged" : "", p); + unlogged_tables ? " unlogged" : "", table, p, table); /* * For RANGE, we use open-ended partitions at the beginning and @@ -4748,11 +4752,11 @@ createPartitions(PGconn *con) } else if (partition_method == PART_HASH) printfPQExpBuffer(&query, - "create%s table pgbench_accounts_%d\n" - " partition of pgbench_accounts\n" + "create%s table %s_%d\n" + " partition of %s\n" " for values with (modulus %d, remainder %d)", - unlogged_tables ? " unlogged" : "", p, - partitions, p - 1); + unlogged_tables ? " unlogged" : "", table, p, + table, partitions, p - 1); else /* cannot get there */ Assert(0); @@ -4760,7 +4764,8 @@ createPartitions(PGconn *con) * Per ddlinfo in initCreateTables, fillfactor is needed on table * pgbench_accounts. */ - appendPQExpBuffer(&query, " with (fillfactor=%d)", fillfactor); + if (strcmp(table, "pgbench_accounts") == 0) + appendPQExpBuffer(&query, " with (fillfactor=%d)", fillfactor); executeStatement(con, query.data); } @@ -4836,9 +4841,9 @@ initCreateTables(PGconn *con) (scale >= SCALE_32BIT_THRESHOLD) ? ddl->bigcols : ddl->smcols); /* Partition pgbench_accounts table */ - if (partition_method != PART_NONE && strcmp(ddl->table, "pgbench_accounts") == 0) - appendPQExpBuffer(&query, - " partition by %s (aid)", PARTITION_METHOD[partition_method]); + if (partition_method != PART_NONE && ( + strcmp(ddl->table, "pgbench_accounts") == 0 || strcmp(ddl->table, "pgbench_history") == 0)) + appendPQExpBuffer(&query, " partition by %s (aid)", PARTITION_METHOD[partition_method]); else if (ddl->declare_fillfactor) { /* fillfactor is only expected on actual tables */ @@ -4860,7 +4865,10 @@ initCreateTables(PGconn *con) termPQExpBuffer(&query); if (partition_method != PART_NONE) - createPartitions(con); + { + createPartitions(con, "pgbench_accounts"); + createPartitions(con, "pgbench_history"); + } } /* @@ -7225,7 +7233,10 @@ main(int argc, char **argv) fprintf(stderr, "starting vacuum..."); tryExecuteStatement(con, "vacuum pgbench_branches"); tryExecuteStatement(con, "vacuum pgbench_tellers"); - tryExecuteStatement(con, "truncate pgbench_history"); + if (partitions > 0) + tryExecuteStatement(con, "truncate pgbench_history CASCADE"); + else + tryExecuteStatement(con, "truncate pgbench_history"); fprintf(stderr, "end.\n"); if (do_vacuum_accounts) -- 2.43.0 ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v3 2/2] Remove unnecessary 32-bit optimizations and alignment checks. @ 2026-01-22 17:33 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Nathan Bossart @ 2026-01-22 17:33 UTC (permalink / raw) --- src/port/pg_popcount_x86.c | 63 ++++++++------------------------------ 1 file changed, 13 insertions(+), 50 deletions(-) diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c index 245f0167d00..0e98f532552 100644 --- a/src/port/pg_popcount_x86.c +++ b/src/port/pg_popcount_x86.c @@ -382,33 +382,16 @@ pg_popcount_sse42(const char *buf, int bytes) uint64 popcnt = 0; #if SIZEOF_VOID_P >= 8 - /* Process in 64-bit chunks if the buffer is aligned. */ - if (buf == (const char *) TYPEALIGN(8, buf)) - { - const uint64 *words = (const uint64 *) buf; - - while (bytes >= 8) - { - popcnt += pg_popcount64_sse42(*words++); - bytes -= 8; - } + /* Process in 64-bit chunks. */ + const uint64 *words = (const uint64 *) buf; - buf = (const char *) words; - } -#else - /* Process in 32-bit chunks if the buffer is aligned. */ - if (buf == (const char *) TYPEALIGN(4, buf)) + while (bytes >= 8) { - const uint32 *words = (const uint32 *) buf; - - while (bytes >= 4) - { - popcnt += pg_popcount32_sse42(*words++); - bytes -= 4; - } - - buf = (const char *) words; + popcnt += pg_popcount64_sse42(*words++); + bytes -= 8; } + + buf = (const char *) words; #endif /* Process any remaining bytes */ @@ -428,37 +411,17 @@ pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask) uint64 popcnt = 0; #if SIZEOF_VOID_P >= 8 - /* Process in 64-bit chunks if the buffer is aligned */ + /* Process in 64-bit chunks. */ uint64 maskv = ~UINT64CONST(0) / 0xFF * mask; + const uint64 *words = (const uint64 *) buf; - if (buf == (const char *) TYPEALIGN(8, buf)) + while (bytes >= 8) { - const uint64 *words = (const uint64 *) buf; - - while (bytes >= 8) - { - popcnt += pg_popcount64_sse42(*words++ & maskv); - bytes -= 8; - } - - buf = (const char *) words; + popcnt += pg_popcount64_sse42(*words++ & maskv); + bytes -= 8; } -#else - /* Process in 32-bit chunks if the buffer is aligned. */ - uint32 maskv = ~((uint32) 0) / 0xFF * mask; - if (buf == (const char *) TYPEALIGN(4, buf)) - { - const uint32 *words = (const uint32 *) buf; - - while (bytes >= 4) - { - popcnt += pg_popcount32_sse42(*words++ & maskv); - bytes -= 4; - } - - buf = (const char *) words; - } + buf = (const char *) words; #endif /* Process any remaining bytes */ -- 2.50.1 (Apple Git-155) --1Cwfhs/B30EIRjWN-- ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-01-22 17:33 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-12 13:07 [PATCH 1/6] Pass all scan keys to BRIN consistent function at once Tomas Vondra <[email protected]> 2023-11-30 10:29 Extend pgbench partitioning to pgbench_history Gabriele Bartolini <[email protected]> 2023-11-30 11:01 ` Re: Extend pgbench partitioning to pgbench_history Gabriele Bartolini <[email protected]> 2026-01-22 17:33 [PATCH v3 2/2] Remove unnecessary 32-bit optimizations and alignment checks. Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox