8000 Support the old signature of BRIN consistent function · postgres/postgres@a681e3c · GitHub
[go: up one dir, main page]
Skip to content

Commit a681e3c

Browse files
tvondratvondra
committed
Support the old signature of BRIN consistent function
Commit a1c649d changed the signature of the BRIN consistent function by adding a new required parameter. Treating the parameter as optional, which would make the change backwards incompatibile, was rejected with the justification that there are few out-of-core extensions, so it's not worth adding making the code more complex, and it's better to deal with that in the extension. But after further thought, that would be rather problematic, because pg_upgrade simply dumps catalog contents and the same version of an extension needs to work on both PostgreSQL versions. Supporting both variants of the consistent function (with 3 or 4 arguments) makes that possible. The signature is not the only thing that changed, as commit 72ccf55 moved handling of IS [NOT] NULL keys from the support procedures. But this change is backward compatible - handling the keys in exension is unnecessary, but harmless. The consistent function will do a bit of unnecessary work, but it should be very cheap. This also undoes most of the changes to the existing opclasses (minmax and inclusion), making them use the old signature again. This should make backpatching simpler. Catversion bump, because of changes in pg_amproc. Author: Tomas Vondra <tomas.vondra@postgresql.org> Author: Nikita Glukhov <n.gluhov@postgrespro.ru> Reviewed-by: Mark Dilger <hornschnorter@gmail.com> Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com> Reviewed-by: Masahiko Sawada <masahiko.sawada@enterprisedb.com> Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com
1 parent a68dfa2 commit a681e3c

File tree

7 files changed

+121
-146
lines changed

7 files changed

+121
-146
lines changed

‎doc/src/sgml/brin.sgml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,19 @@ typedef struct BrinOpcInfo
476476
</listitem>
477477
</varlistentry>
478478

479+
<varlistentry>
480+
<term><function>bool consistent(BrinDesc *bdesc, BrinValues *column,
481+
ScanKey key)</function></term>
482+
<listitem>
483+
<para>
484+
Returns whether the ScanKey is consistent with the given indexed
485+
values for a range.
486+
The attribute number to use is passed as part of the scan key.
487+
This is an older backward-compatible variant of the consistent function.
488+
</para>
489+
</listitem>
490+
</varlistentry>
491+
479492
<varlistentry>
480493
<term><function>bool addValue(BrinDesc *bdesc, BrinValues *column,
481494
Datum newval, bool isnull)</function></term>

‎src/backend/access/brin/brin.c‎

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -634,26 +634,58 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
634634
break;
635635
}
636636

637+
/*
638+
* Collation from the first key (has to be the same for
639+
* all keys for the same attribue).
640+
*/
641+
collation = keys[attno - 1][0]->sk_collation;
642+
637643
/*
638644
* Check whether the scan key is consistent with the page
639645
* range values; if so, have the pages in the range added
640646
* to the output bitmap.
641647
*
642-
* XXX We simply use the collation from the first key (it
643-
* has to be the same for all keys for the same attribue).
648+
* The opclass may or may not support processing of multiple
649+
* scan keys. We can determine that based on the number of
650+
* arguments - functions with extra parameter (number of scan
651+
* keys) do support this, otherwise we have to simply pass the
652+
* scan keys one by one.
644653
*/
645-
collation = keys[attno - 1][0]->sk_collation;
646-
647-
/* Check all keys at once */
648-
add = FunctionCall4Coll(&consistentFn[attno - 1],
649-
collation,
650-
PointerGetDatum(bdesc),
651-
PointerGetDatum(bval),
652-
PointerGetDatum(keys[attno - 1]),
653-
Int32GetDatum(nkeys[attno - 1]));
654-
addrange = DatumGetBool(add);
655-
if (!addrange)
656-
break;
654+
if (consistentFn[attno - 1].fn_nargs >= 4)
655+
{
656+
/* Check all keys at once */
657+
add = FunctionCall4Coll(&consistentFn[attno - 1],
658+
collation,
659+
PointerGetDatum(bdesc),
660+
PointerGetDatum(bval),
661+
PointerGetDatum(keys[attno - 1]),
662+
Int32GetDatum(nkeys[attno - 1]));
663+
addrange = DatumGetBool(add);
664+
}
665+
else
666+
{
667+
/*
668+
* Check keys one by one
669+
*
670+
* When there are multiple scan keys, failure to meet the
671+
* criteria for a single one of them is enough to discard
672+
* the range as a whole, so break out of the loop as soon
673+
* as a false return value is obtained.
674+
*/
675+
int keyno;
676+
677+
for (keyno = 0; keyno < nkeys[attno - 1]; keyno++)
678+
{
679+
add = FunctionCall3Coll(&consistentFn[attno - 1],
680+
keys[attno - 1][keyno]->sk_collation,
681+
PointerGetDatum(bdesc),
682+
PointerGetDatum(bval),
683+
PointerGetDatum(keys[attno - 1][keyno]));
684+
addrange = DatumGetBool(add);
685+
if (!addrange)
686+
break;
687+
}
688+
}
657689
}
658690
}
659691
}

‎src/backend/access/brin/brin_inclusion.c‎

Lines changed: 40 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ static FmgrInfo *inclusion_get_procinfo(BrinDesc *bdesc, uint16 attno,
8585
uint16 procnum);
8686
static FmgrInfo *inclusion_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno,
8787
Oid subtype, uint16 strategynum);
88-
static bool inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column,
89-
ScanKey key, Oid colloid);
9088

9189

9290
/*
@@ -243,9 +241,9 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS)
243241
/*
244242
* BRIN inclusion consistent function
245243
*
246-
* We inspect the IS NULL scan keys first, which allows us to make a decision
247-
* without looking at the contents of the page range. Only when the page range
248-
* matches the IS NULL keys, we check the regular scan keys.
244+
* We're no longer dealing with NULL keys in the consistent function, that is
245+
* now handled by the AM code. That means we should not get any all-NULL ranges
246+
* either, because those can't be consistent with regular (not [IS] NULL) keys.
249247
*
250248
* All of the strategies are optional.
251249
*/
@@ -254,63 +252,29 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
254252
{
255253
BrinDesc *bdesc = (BrinDesc *) PG_GETARG_POINTER(0);
256254
BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1);
257-
ScanKey *keys = (ScanKey *) PG_GETARG_POINTER(2);
258-
int nkeys = PG_GETARG_INT32(3);
259-
Oid colloid = PG_GET_COLLATION();
260-
int keyno;
255+
ScanKey key = (ScanKey) PG_GETARG_POINTER(2);
256+
Oid colloid = PG_GET_COLLATION(),
257+
subtype;
258+
Datum unionval;
259+
AttrNumber attno;
260+
Datum query;
261+
FmgrInfo *finfo;
262+
Datum result;
261263

262-
/* make sure we got some scan keys */
263-
Assert((nkeys > 0) && (keys != NULL));
264+
/* This opclass uses the old signature with only three arguments. */
265+
Assert(PG_NARGS() == 3);
264266

265-
/*
266-
* If is all nulls, it cannot possibly be consistent (at this point we
267-
* know there are at least some regular scan keys).
268-
*/
269-
if (column->bv_allnulls)
270-
PG_RETURN_BOOL(false);
267+
/* Should not be dealing with all-NULL ranges. */
268+
Assert(!column->bv_allnulls);
271269

272270
/* It has to be checked, if it contains elements that are not mergeable. */
273271
if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE]))
274272
PG_RETURN_BOOL(true);
275273

276-
/* Check that the range is consistent with all regular scan keys. */
277-
for (keyno = 0; keyno < nkeys; keyno++)
278-
{
279-
ScanKey key = keys[keyno];
280-
281-
/* NULL keys are handled and filtered-out in bringetbitmap */
282-
Assert(!(key->sk_flags & SK_ISNULL));
283-
284-
/*
285-
* When there are multiple scan keys, failure to meet the criteria for
286-
* a single one of them is enough to discard the range as a whole, so
287-
* break out of the loop as soon as a false return value is obtained.
288-
*/
289-
if (!inclusion_consistent_key(bdesc, column, key, colloid))
290-
PG_RETURN_BOOL(false);
291-
}
292-
293-
PG_RETURN_BOOL(true);
294-
}
295-
296-
/*
297-
* inclusion_consistent_key
298-
* Determine if the range is consistent with a single scan key.
299-
*/
300-
static bool
301-
inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
302-
Oid colloid)
303-
{
304-
FmgrInfo *finfo;
305-
AttrNumber attno = key->sk_attno;
306-
Oid subtype = key->sk_subtype;
307-
Datum query = key->sk_argument;
308-
Datum unionval = column->bv_values[INCLUSION_UNION];
309-
Datum result;
310-
311-
/* This should be called only for regular keys, not for IS [NOT] NULL. */
312-
Assert(!(key->sk_flags & SK_ISNULL));
313-
274+
attno = key->sk_attno;
275+
subtype = key->sk_subtype;
276+
query = key->sk_argument;
277+
unionval = column->bv_values[INCLUSION_UNION];
314278
switch (key->sk_strategy)
315279
{
316280
/*
@@ -330,49 +294,49 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
330294
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
331295
RTOverRightStrategyNumber);
332296
result = FunctionCall2Coll(finfo, colloid, unionval, query);
333-
return !DatumGetBool(result);
297+
PG_RETURN_BOOL(!DatumGetBool(result));
334298

335299
case RTOverLeftStrategyNumber:
336300
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
337301
RTRightStrategyNumber);
338302
result = FunctionCall2Coll(finfo, colloid, unionval, query);
339-
return !DatumGetBool(result);
303+
PG_RETURN_BOOL(!DatumGetBool(result));
340304

341305
case RTOverRightStrategyNumber:
342306
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
343307
RTLeftStrategyNumber);
344308
result = FunctionCall2Coll(finfo, colloid, unionval, query);
345-
return !DatumGetBool(result);
309+
PG_RETURN_BOOL(!DatumGetBool(result));
346310

347311
case RTRightStrategyNumber:
348312
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
349313
RTOverLeftStrategyNumber);
350314
result = FunctionCall2Coll(finfo, colloid, unionval, query);
351-
return !DatumGetBool(result);
315+
PG_RETURN_BOOL(!DatumGetBool(result));
352316

353317
case RTBelowStrategyNumber:
354318
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
355319
RTOverAboveStrategyNumber);
356320
result = FunctionCall2Coll(finfo, colloid, unionval, query);
357-
return !DatumGetBool(result);
321+
PG_RETURN_BOOL(!DatumGetBool(result));
358322

359323
case RTOverBelowStrategyNumber:
360324
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
361325
RTAboveStrategyNumber);
362326
result = FunctionCall2Coll(finfo, colloid, unionval, query);
363-
return !DatumGetBool(result);
327+
PG_RETURN_BOOL(!DatumGetBool(result));
364328

365329
case RTOverAboveStrategyNumber:
366330
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
367331
RTBelowStrategyNumber);
368332
result = FunctionCall2Coll(finfo, colloid, unionval, query);
369-
return !DatumGetBool(result);
333+
PG_RETURN_BOOL(!DatumGetBool(result));
370334

371335
case RTAboveStrategyNumber:
372336
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
373337
RTOverBelowStrategyNumber);
374338
result = FunctionCall2Coll(finfo, colloid, unionval, query);
375-
return !DatumGetBool(result);
339+
PG_RETURN_BOOL(!DatumGetBool(result));
376340

377341
/*
378342
* Overlap and contains strategies
@@ -390,7 +354,7 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
390354
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
391355
key->sk_strategy);
392356
result = FunctionCall2Coll(finfo, colloid, unionval, query);
393-
return DatumGetBool(result);
357+
PG_RETURN_DATUM(result);
394358

395359
/*
396360
* Contained by strategies
@@ -410,9 +374,9 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
410374
RTOverlapStrategyNumber);
411375
result = FunctionCall2Coll(finfo, colloid, unionval, query);
412376
if (DatumGetBool(result))
413-
return true;
377+
PG_RETURN_BOOL(true);
414378

415-
return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
379+
PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
416380

417381
/*
418382
* Adjacent strategy
@@ -429,12 +393,12 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
429393
RTOverlapStrategyNumber);
430394
result = FunctionCall2Coll(finfo, colloid, unionval, query);
431395
if (DatumGetBool(result))
432-
return true;
396+
PG_RETURN_BOOL(true);
433397

434398
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
435399
RTAdjacentStrategyNumber);
436400
result = FunctionCall2Coll(finfo, colloid, unionval, query);
437-
return DatumGetBool(result);
401+
PG_RETURN_DATUM(result);
438402

439403
/*
440404
* Basic comparison strategies
@@ -464,40 +428,40 @@ inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
464428
RTRightStrategyNumber);
465429
result = FunctionCall2Coll(finfo, colloid, unionval, query);
466430
if (!DatumGetBool(result))
467-
return true;
431+
PG_RETURN_BOOL(true);
468432

469-
return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
433+
PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
470434

471435
case RTSameStrategyNumber:
472436
case RTEqualStrategyNumber:
473437
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
474438
RTContainsStrategyNumber);
475439
result = FunctionCall2Coll(finfo, colloid, unionval, query);
476440
if (DatumGetBool(result))
477-
return true;
441+
PG_RETURN_BOOL(true);
478442

479-
return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
443+
PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
480444

481445
case RTGreaterEqualStrategyNumber:
482446
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
483447
RTLeftStrategyNumber);
484448
result = FunctionCall2Coll(finfo, colloid, unionval, query);
485449
if (!DatumGetBool(result))
486-
return true;
450+
PG_RETURN_BOOL(true);
487451

488-
return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
452+
PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
489453

490454
case RTGreaterStrategyNumber:
491455
/* no need to check for empty elements */
492456
finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
493457
RTLeftStrategyNumber);
494458
result = FunctionCall2Coll(finfo, colloid, unionval, query);
495-
return !DatumGetBool(result);
459+
PG_RETURN_BOOL(!DatumGetBool(result));
496460

497461
default:
498462
/* shouldn't happen */
499463
elog(ERROR, "invalid strategy number %d", key->sk_strategy);
500-
return false;
464+
PG_RETURN_BOOL(false);
501465
}
502466
}
503467

0 commit comments

Comments
 (0)
0