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

Commit9f96827

Browse files
committed
Invent "amadjustmembers" AM method for validating opclass members.
This allows AM-specific knowledge to be applied during creation ofpg_amop and pg_amproc entries. Specifically, the AM knows better thancore code which entries to consider as required or optional. Givingthe latter entries the appropriate sort of dependency allows them tobe dropped without taking out the whole opclass or opfamily; whichis something we'd like to have to correct obsolescent entries inextensions.This callback also opens the door to performing AM-specific validitychecks during opclass creation, rather than hoping than an opclassdeveloper will remember to test with "amvalidate". For the most partI've not actually added any such checks yet; that can happen in afollow-on patch. (Note that we shouldn't remove any tests from"amvalidate", as those are still needed to cross-check manuallyconstructed entries in the initdb data. So adding tests to"amadjustmembers" will be somewhat duplicative, but it seems likea good idea anyway.)Patch by me, reviewed by Alexander Korotkov, Hamid Akhtar, andAnastasia Lubennikova.Discussion:https://postgr.es/m/4578.1565195302@sss.pgh.pa.us
1 parente2b37d9 commit9f96827

File tree

24 files changed

+646
-112
lines changed

24 files changed

+646
-112
lines changed

‎contrib/bloom/blutils.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ blhandler(PG_FUNCTION_ARGS)
139139
amroutine->amproperty=NULL;
140140
amroutine->ambuildphasename=NULL;
141141
amroutine->amvalidate=blvalidate;
142+
amroutine->amadjustmembers=NULL;
142143
amroutine->ambeginscan=blbeginscan;
143144
amroutine->amrescan=blrescan;
144145
amroutine->amgettuple=NULL;

‎doc/src/sgml/indexam.sgml‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ typedef struct IndexAmRoutine
143143
amproperty_function amproperty; /* can be NULL */
144144
ambuildphasename_function ambuildphasename; /* can be NULL */
145145
amvalidate_function amvalidate;
146+
amadjustmembers_function amadjustmembers; /* can be NULL */
146147
ambeginscan_function ambeginscan;
147148
amrescan_function amrescan;
148149
amgettuple_function amgettuple; /* can be NULL */
@@ -502,7 +503,48 @@ amvalidate (Oid opclassoid);
502503
the access method can reasonably do that. For example, this might include
503504
testing that all required support functions are provided.
504505
The <function>amvalidate</function> function must return false if the opclass is
505-
invalid. Problems should be reported with <function>ereport</function> messages.
506+
invalid. Problems should be reported with <function>ereport</function>
507+
messages, typically at <literal>INFO</literal> level.
508+
</para>
509+
510+
<para>
511+
<programlisting>
512+
void
513+
amadjustmembers (Oid opfamilyoid,
514+
Oid opclassoid,
515+
List *operators,
516+
List *functions);
517+
</programlisting>
518+
Validate proposed new operator and function members of an operator family,
519+
so far as the access method can reasonably do that, and set their
520+
dependency types if the default is not satisfactory. This is called
521+
during <command>CREATE OPERATOR CLASS</command> and during
522+
<command>ALTER OPERATOR FAMILY ADD</command>; in the latter
523+
case <parameter>opclassoid</parameter> is <literal>InvalidOid</literal>.
524+
The <type>List</type> arguments are lists
525+
of <structname>OpFamilyMember</structname> structs, as defined
526+
in <filename>amapi.h</filename>.
527+
528+
Tests done by this function will typically be a subset of those
529+
performed by <function>amvalidate</function>,
530+
since <function>amadjustmembers</function> cannot assume that it is
531+
seeing a complete set of members. For example, it would be reasonable
532+
to check the signature of a support function, but not to check whether
533+
all required support functions are provided. Any problems can be
534+
reported by throwing an error.
535+
536+
The dependency-related fields of
537+
the <structname>OpFamilyMember</structname> structs are initialized by
538+
the core code to create hard dependencies on the opclass if this
539+
is <command>CREATE OPERATOR CLASS</command>, or soft dependencies on the
540+
opfamily if this is <command>ALTER OPERATOR FAMILY ADD</command>.
541+
<function>amadjustmembers</function> can adjust these fields if some other
542+
behavior is more appropriate. For example, GIN, GiST, and SP-GiST
543+
always set operator members to have soft dependencies on the opfamily,
544+
since the connection between an operator and an opclass is relatively
545+
weak in these index types; so it is reasonable to allow operator members
546+
to be added and removed freely. Optional support functions are typically
547+
also given soft dependencies, so that they can be removed if necessary.
506548
</para>
507549

508550

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ brinhandler(PG_FUNCTION_ARGS)
120120
amroutine->amproperty=NULL;
121121
amroutine->ambuildphasename=NULL;
122122
amroutine->amvalidate=brinvalidate;
123+
amroutine->amadjustmembers=NULL;
123124
amroutine->ambeginscan=brinbeginscan;
124125
amroutine->amrescan=brinrescan;
125126
amroutine->amgettuple=NULL;

‎src/backend/access/gin/ginutil.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ ginhandler(PG_FUNCTION_ARGS)
7171
amroutine->amproperty=NULL;
7272
amroutine->ambuildphasename=NULL;
7373
amroutine->amvalidate=ginvalidate;
74+
amroutine->amadjustmembers=ginadjustmembers;
7475
amroutine->ambeginscan=ginbeginscan;
7576
amroutine->amrescan=ginrescan;
7677
amroutine->amgettuple=NULL;

‎src/backend/access/gin/ginvalidate.c‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,3 +271,68 @@ ginvalidate(Oid opclassoid)
271271

272272
returnresult;
273273
}
274+
275+
/*
276+
* Prechecking function for adding operators/functions to a GIN opfamily.
277+
*/
278+
void
279+
ginadjustmembers(Oidopfamilyoid,
280+
Oidopclassoid,
281+
List*operators,
282+
List*functions)
283+
{
284+
ListCell*lc;
285+
286+
/*
287+
* Operator members of a GIN opfamily should never have hard dependencies,
288+
* since their connection to the opfamily depends only on what the support
289+
* functions think, and that can be altered. For consistency, we make all
290+
* soft dependencies point to the opfamily, though a soft dependency on
291+
* the opclass would work as well in the CREATE OPERATOR CLASS case.
292+
*/
293+
foreach(lc,operators)
294+
{
295+
OpFamilyMember*op= (OpFamilyMember*)lfirst(lc);
296+
297+
op->ref_is_hard= false;
298+
op->ref_is_family= true;
299+
op->refobjid=opfamilyoid;
300+
}
301+
302+
/*
303+
* Required support functions should have hard dependencies. Preferably
304+
* those are just dependencies on the opclass, but if we're in ALTER
305+
* OPERATOR FAMILY, we leave the dependency pointing at the whole
306+
* opfamily. (Given that GIN opclasses generally don't share opfamilies,
307+
* it seems unlikely to be worth working harder.)
308+
*/
309+
foreach(lc,functions)
310+
{
311+
OpFamilyMember*op= (OpFamilyMember*)lfirst(lc);
312+
313+
switch (op->number)
314+
{
315+
caseGIN_EXTRACTVALUE_PROC:
316+
caseGIN_EXTRACTQUERY_PROC:
317+
/* Required support function */
318+
op->ref_is_hard= true;
319+
break;
320+
caseGIN_COMPARE_PROC:
321+
caseGIN_CONSISTENT_PROC:
322+
caseGIN_COMPARE_PARTIAL_PROC:
323+
caseGIN_TRICONSISTENT_PROC:
324+
caseGIN_OPTIONS_PROC:
325+
/* Optional, so force it to be a soft family dependency */
326+
op->ref_is_hard= false;
327+
op->ref_is_family= true;
328+
op->refobjid=opfamilyoid;
329+
break;
330+
default:
331+
ereport(ERROR,
332+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
333+
errmsg("support function number %d is invalid for access method %s",
334+
op->number,"gin")));
335+
break;
336+
}
337+
}
338+
}

‎src/backend/access/gist/gist.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ gisthandler(PG_FUNCTION_ARGS)
9292
amroutine->amproperty=gistproperty;
9393
amroutine->ambuildphasename=NULL;
9494
amroutine->amvalidate=gistvalidate;
95+
amroutine->amadjustmembers=gistadjustmembers;
9596
amroutine->ambeginscan=gistbeginscan;
9697
amroutine->amrescan=gistrescan;
9798
amroutine->amgettuple=gistgettuple;

‎src/backend/access/gist/gistvalidate.c‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,3 +279,72 @@ gistvalidate(Oid opclassoid)
279279

280280
returnresult;
281281
}
282+
283+
/*
284+
* Prechecking function for adding operators/functions to a GiST opfamily.
285+
*/
286+
void
287+
gistadjustmembers(Oidopfamilyoid,
288+
Oidopclassoid,
289+
List*operators,
290+
List*functions)
291+
{
292+
ListCell*lc;
293+
294+
/*
295+
* Operator members of a GiST opfamily should never have hard
296+
* dependencies, since their connection to the opfamily depends only on
297+
* what the support functions think, and that can be altered. For
298+
* consistency, we make all soft dependencies point to the opfamily,
299+
* though a soft dependency on the opclass would work as well in the
300+
* CREATE OPERATOR CLASS case.
301+
*/
302+
foreach(lc,operators)
303+
{
304+
OpFamilyMember*op= (OpFamilyMember*)lfirst(lc);
305+
306+
op->ref_is_hard= false;
307+
op->ref_is_family= true;
308+
op->refobjid=opfamilyoid;
309+
}
310+
311+
/*
312+
* Required support functions should have hard dependencies. Preferably
313+
* those are just dependencies on the opclass, but if we're in ALTER
314+
* OPERATOR FAMILY, we leave the dependency pointing at the whole
315+
* opfamily. (Given that GiST opclasses generally don't share opfamilies,
316+
* it seems unlikely to be worth working harder.)
317+
*/
318+
foreach(lc,functions)
319+
{
320+
OpFamilyMember*op= (OpFamilyMember*)lfirst(lc);
321+
322+
switch (op->number)
323+
{
324+
caseGIST_CONSISTENT_PROC:
325+
caseGIST_UNION_PROC:
326+
caseGIST_PENALTY_PROC:
327+
caseGIST_PICKSPLIT_PROC:
328+
caseGIST_EQUAL_PROC:
329+
/* Required support function */
330+
op->ref_is_hard= true;
331+
break;
332+
caseGIST_COMPRESS_PROC:
333+
caseGIST_DECOMPRESS_PROC:
334+
caseGIST_DISTANCE_PROC:
335+
caseGIST_FETCH_PROC:
336+
caseGIST_OPTIONS_PROC:
337+
/* Optional, so force it to be a soft family dependency */
338+
op->ref_is_hard= false;
339+
op->ref_is_family= true;
340+
op->refobjid=opfamilyoid;
341+
break;
342+
default:
343+
ereport(ERROR,
344+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
345+
errmsg("support function number %d is invalid for access method %s",
346+
op->number,"gist")));
347+
break;
348+
}
349+
}
350+
}

‎src/backend/access/hash/hash.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ hashhandler(PG_FUNCTION_ARGS)
8989
amroutine->amproperty=NULL;
9090
amroutine->ambuildphasename=NULL;
9191
amroutine->amvalidate=hashvalidate;
92+
amroutine->amadjustmembers=hashadjustmembers;
9293
amroutine->ambeginscan=hashbeginscan;
9394
amroutine->amrescan=hashrescan;
9495
amroutine->amgettuple=hashgettuple;

‎src/backend/access/hash/hashvalidate.c‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
#include"access/amvalidate.h"
1717
#include"access/hash.h"
1818
#include"access/htup_details.h"
19+
#include"access/xact.h"
20+
#include"catalog/pg_am.h"
1921
#include"catalog/pg_amop.h"
2022
#include"catalog/pg_amproc.h"
2123
#include"catalog/pg_opclass.h"
@@ -25,6 +27,7 @@
2527
#include"parser/parse_coerce.h"
2628
#include"utils/builtins.h"
2729
#include"utils/fmgroids.h"
30+
#include"utils/lsyscache.h"
2831
#include"utils/regproc.h"
2932
#include"utils/syscache.h"
3033

@@ -341,3 +344,96 @@ check_hash_func_signature(Oid funcid, int16 amprocnum, Oid argtype)
341344
ReleaseSysCache(tp);
342345
returnresult;
343346
}
347+
348+
/*
349+
* Prechecking function for adding operators/functions to a hash opfamily.
350+
*/
351+
void
352+
hashadjustmembers(Oidopfamilyoid,
353+
Oidopclassoid,
354+
List*operators,
355+
List*functions)
356+
{
357+
Oidopcintype;
358+
ListCell*lc;
359+
360+
/*
361+
* Hash operators and required support functions are always "loose"
362+
* members of the opfamily if they are cross-type. If they are not
363+
* cross-type, we prefer to tie them to the appropriate opclass ... but if
364+
* the user hasn't created one, we can't do that, and must fall back to
365+
* using the opfamily dependency. (We mustn't force creation of an
366+
* opclass in such a case, as leaving an incomplete opclass laying about
367+
* would be bad. Throwing an error is another undesirable alternative.)
368+
*
369+
* This behavior results in a bit of a dump/reload hazard, in that the
370+
* order of restoring objects could affect what dependencies we end up
371+
* with. pg_dump's existing behavior will preserve the dependency choices
372+
* in most cases, but not if a cross-type operator has been bound tightly
373+
* into an opclass. That's a mistake anyway, so silently "fixing" it
374+
* isn't awful.
375+
*
376+
* Optional support functions are always "loose" family members.
377+
*
378+
* To avoid repeated lookups, we remember the most recently used opclass's
379+
* input type.
380+
*/
381+
if (OidIsValid(opclassoid))
382+
{
383+
/* During CREATE OPERATOR CLASS, need CCI to see the pg_opclass row */
384+
CommandCounterIncrement();
385+
opcintype=get_opclass_input_type(opclassoid);
386+
}
387+
else
388+
opcintype=InvalidOid;
389+
390+
/*
391+
* We handle operators and support functions almost identically, so rather
392+
* than duplicate this code block, just join the lists.
393+
*/
394+
foreach(lc,list_concat_copy(operators,functions))
395+
{
396+
OpFamilyMember*op= (OpFamilyMember*)lfirst(lc);
397+
398+
if (op->is_func&&op->number!=HASHSTANDARD_PROC)
399+
{
400+
/* Optional support proc, so always a soft family dependency */
401+
op->ref_is_hard= false;
402+
op->ref_is_family= true;
403+
op->refobjid=opfamilyoid;
404+
}
405+
elseif (op->lefttype!=op->righttype)
406+
{
407+
/* Cross-type, so always a soft family dependency */
408+
op->ref_is_hard= false;
409+
op->ref_is_family= true;
410+
op->refobjid=opfamilyoid;
411+
}
412+
else
413+
{
414+
/* Not cross-type; is there a suitable opclass? */
415+
if (op->lefttype!=opcintype)
416+
{
417+
/* Avoid repeating this expensive lookup, even if it fails */
418+
opcintype=op->lefttype;
419+
opclassoid=opclass_for_family_datatype(HASH_AM_OID,
420+
opfamilyoid,
421+
opcintype);
422+
}
423+
if (OidIsValid(opclassoid))
424+
{
425+
/* Hard dependency on opclass */
426+
op->ref_is_hard= true;
427+
op->ref_is_family= false;
428+
op->refobjid=opclassoid;
429+
}
430+
else
431+
{
432+
/* We're stuck, so make a soft dependency on the opfamily */
433+
op->ref_is_hard= false;
434+
op->ref_is_family= true;
435+
op->refobjid=opfamilyoid;
436+
}
437+
}
438+
}
439+
}

0 commit comments

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