Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit17ce344

Browse files
committed
BRIN: be more strict about required support procs
With improperly defined operator classes, it's possible to get aPostgres crash because we'd try to invoke a procedure that doesn'texist. This is because the code is being a bit too trusting that theopclass is correctly defined. Add some ereport(ERROR)s for cases wheremandatory support procedures are not defined, transforming the crashesinto errors.The particular case that was reported is an incomplete opclass inPostGIS.Backpatch all the way down to 13.Reported-by: Tobias Wendorff <tobias.wendorff@tu-dortmund.de>Diagnosed-by: David Rowley <dgrowleyml@gmail.com>Reviewed-by: Tomas Vondra <tomas@vondra.me>Discussion:https://postgr.es/m/fb6d9a35-6c8e-4869-af80-0a4944a793a4@tu-dortmund.de
1 parentd35d32d commit17ce344

File tree

3 files changed

+28
-41
lines changed

3 files changed

+28
-41
lines changed

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

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,6 @@ typedef struct BloomOpaque
438438
* consistency. We may need additional procs in the future.
439439
*/
440440
FmgrInfoextra_procinfos[BLOOM_MAX_PROCNUMS];
441-
boolextra_proc_missing[BLOOM_MAX_PROCNUMS];
442441
}BloomOpaque;
443442

444443
staticFmgrInfo*bloom_get_procinfo(BrinDesc*bdesc,uint16attno,
@@ -714,27 +713,19 @@ bloom_get_procinfo(BrinDesc *bdesc, uint16 attno, uint16 procnum)
714713
*/
715714
opaque= (BloomOpaque*)bdesc->bd_info[attno-1]->oi_opaque;
716715

717-
/*
718-
* If we already searched for this proc and didn't find it, don't bother
719-
* searching again.
720-
*/
721-
if (opaque->extra_proc_missing[basenum])
722-
returnNULL;
723-
724716
if (opaque->extra_procinfos[basenum].fn_oid==InvalidOid)
725717
{
726718
if (RegProcedureIsValid(index_getprocid(bdesc->bd_index,attno,
727719
procnum)))
728-
{
729720
fmgr_info_copy(&opaque->extra_procinfos[basenum],
730721
index_getprocinfo(bdesc->bd_index,attno,procnum),
731722
bdesc->bd_context);
732-
}
733723
else
734-
{
735-
opaque->extra_proc_missing[basenum]= true;
736-
returnNULL;
737-
}
724+
ereport(ERROR,
725+
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
726+
errmsg_internal("invalid opclass definition"),
727+
errdetail_internal("The operator class is missing support function %d for column %d.",
728+
procnum,attno));
738729
}
739730

740731
return&opaque->extra_procinfos[basenum];

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

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ typedef struct InclusionOpaque
8282
}InclusionOpaque;
8383

8484
staticFmgrInfo*inclusion_get_procinfo(BrinDesc*bdesc,uint16attno,
85-
uint16procnum);
85+
uint16procnum,boolmissing_ok);
8686
staticFmgrInfo*inclusion_get_strategy_procinfo(BrinDesc*bdesc,uint16attno,
8787
Oidsubtype,uint16strategynum);
8888

@@ -179,7 +179,7 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS)
179179
* new value for emptiness; if it returns true, we need to set the
180180
* "contains empty" flag in the element (unless already set).
181181
*/
182-
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_EMPTY);
182+
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_EMPTY, true);
183183
if (finfo!=NULL&&DatumGetBool(FunctionCall1Coll(finfo,colloid,newval)))
184184
{
185185
if (!DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]))
@@ -195,7 +195,7 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS)
195195
PG_RETURN_BOOL(true);
196196

197197
/* Check if the new value is already contained. */
198-
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_CONTAINS);
198+
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_CONTAINS, true);
199199
if (finfo!=NULL&&
200200
DatumGetBool(FunctionCall2Coll(finfo,colloid,
201201
column->bv_values[INCLUSION_UNION],
@@ -210,7 +210,7 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS)
210210
* it's not going to be used any longer. However, the BRIN framework
211211
* doesn't allow for the value not being present. Improve someday.
212212
*/
213-
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGEABLE);
213+
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGEABLE, true);
214214
if (finfo!=NULL&&
215215
!DatumGetBool(FunctionCall2Coll(finfo,colloid,
216216
column->bv_values[INCLUSION_UNION],
@@ -221,8 +221,7 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS)
221221
}
222222

223223
/* Finally, merge the new value to the existing union. */
224-
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGE);
225-
Assert(finfo!=NULL);
224+
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGE, false);
226225
result=FunctionCall2Coll(finfo,colloid,
227226
column->bv_values[INCLUSION_UNION],newval);
228227
if (!attr->attbyval&&
@@ -506,7 +505,7 @@ brin_inclusion_union(PG_FUNCTION_ARGS)
506505
}
507506

508507
/* Check if A and B are mergeable; if not, mark A unmergeable. */
509-
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGEABLE);
508+
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGEABLE, true);
510509
if (finfo!=NULL&&
511510
!DatumGetBool(FunctionCall2Coll(finfo,colloid,
512511
col_a->bv_values[INCLUSION_UNION],
@@ -517,8 +516,7 @@ brin_inclusion_union(PG_FUNCTION_ARGS)
517516
}
518517

519518
/* Finally, merge B to A. */
520-
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGE);
521-
Assert(finfo!=NULL);
519+
finfo=inclusion_get_procinfo(bdesc,attno,PROCNUM_MERGE, false);
522520
result=FunctionCall2Coll(finfo,colloid,
523521
col_a->bv_values[INCLUSION_UNION],
524522
col_b->bv_values[INCLUSION_UNION]);
@@ -539,10 +537,12 @@ brin_inclusion_union(PG_FUNCTION_ARGS)
539537
* Cache and return inclusion opclass support procedure
540538
*
541539
* Return the procedure corresponding to the given function support number
542-
* or null if it is not exists.
540+
* or null if it is not exists. If missing_ok is true and the procedure
541+
* isn't set up for this opclass, return NULL instead of raising an error.
543542
*/
544543
staticFmgrInfo*
545-
inclusion_get_procinfo(BrinDesc*bdesc,uint16attno,uint16procnum)
544+
inclusion_get_procinfo(BrinDesc*bdesc,uint16attno,uint16procnum,
545+
boolmissing_ok)
546546
{
547547
InclusionOpaque*opaque;
548548
uint16basenum=procnum-PROCNUM_BASE;
@@ -564,13 +564,18 @@ inclusion_get_procinfo(BrinDesc *bdesc, uint16 attno, uint16 procnum)
564564
{
565565
if (RegProcedureIsValid(index_getprocid(bdesc->bd_index,attno,
566566
procnum)))
567-
{
568567
fmgr_info_copy(&opaque->extra_procinfos[basenum],
569568
index_getprocinfo(bdesc->bd_index,attno,procnum),
570569
bdesc->bd_context);
571-
}
572570
else
573571
{
572+
if (!missing_ok)
573+
ereport(ERROR,
574+
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
575+
errmsg_internal("invalid opclass definition"),
576+
errdetail_internal("The operator class is missing support function %d for column %d.",
577+
procnum,attno));
578+
574579
opaque->extra_proc_missing[basenum]= true;
575580
returnNULL;
576581
}

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

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@
111111
typedefstructMinmaxMultiOpaque
112112
{
113113
FmgrInfoextra_procinfos[MINMAX_MAX_PROCNUMS];
114-
boolextra_proc_missing[MINMAX_MAX_PROCNUMS];
115114
Oidcached_subtype;
116115
FmgrInfostrategy_procinfos[BTMaxStrategyNumber];
117116
}MinmaxMultiOpaque;
@@ -2872,27 +2871,19 @@ minmax_multi_get_procinfo(BrinDesc *bdesc, uint16 attno, uint16 procnum)
28722871
*/
28732872
opaque= (MinmaxMultiOpaque*)bdesc->bd_info[attno-1]->oi_opaque;
28742873

2875-
/*
2876-
* If we already searched for this proc and didn't find it, don't bother
2877-
* searching again.
2878-
*/
2879-
if (opaque->extra_proc_missing[basenum])
2880-
returnNULL;
2881-
28822874
if (opaque->extra_procinfos[basenum].fn_oid==InvalidOid)
28832875
{
28842876
if (RegProcedureIsValid(index_getprocid(bdesc->bd_index,attno,
28852877
procnum)))
2886-
{
28872878
fmgr_info_copy(&opaque->extra_procinfos[basenum],
28882879
index_getprocinfo(bdesc->bd_index,attno,procnum),
28892880
bdesc->bd_context);
2890-
}
28912881
else
2892-
{
2893-
opaque->extra_proc_missing[basenum]= true;
2894-
returnNULL;
2895-
}
2882+
ereport(ERROR,
2883+
errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2884+
errmsg_internal("invalid opclass definition"),
2885+
errdetail_internal("The operator class is missing support function %d for column %d.",
2886+
procnum,attno));
28962887
}
28972888

28982889
return&opaque->extra_procinfos[basenum];

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp