From 9ff60273e35cad6e9d3a4adf59d5c2455afe9d9e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 19 Jan 2016 12:04:32 -0500 Subject: Fix assorted inconsistencies in GiST opclass support function declarations. The conventions specified by the GiST SGML documentation were widely ignored. For example, the strategy-number argument for "consistent" and "distance" functions is specified to be a smallint, but most of the built-in support functions declared it as an integer, and for that matter the core code passed it using Int32GetDatum not Int16GetDatum. None of that makes any real difference at runtime, but it's quite confusing for newcomers to the code, and it makes it very hard to write an amvalidate() function that checks support function signatures. So let's try to instill some consistency here. Another similar issue is that the "query" argument is not of a single well-defined type, but could have different types depending on the strategy (corresponding to search operators with different righthand-side argument types). Some of the functions threw up their hands and declared the query argument as being of "internal" type, which surely isn't right ("any" would have been more appropriate); but the majority position seemed to be to declare it as being of the indexed data type, corresponding to a search operator with both input types the same. So I've specified a convention that that's what to do always. Also, the result of the "union" support function actually must be of the index's storage type, but the documentation suggested declaring it to return "internal", and some of the functions followed that. Standardize on telling the truth, instead. Similarly, standardize on declaring the "same" function's inputs as being of the storage type, not "internal". Also, somebody had forgotten to add the "recheck" argument to both the documentation of the "distance" support function and all of their SQL declarations, even though the C code was happily using that argument. Clean that up too. Fix up some other omissions in the docs too, such as documenting that union's second input argument is vestigial. So far as the errors in core function declarations go, we can just fix pg_proc.h and bump catversion. Adjusting the erroneous declarations in contrib modules is more debatable: in principle any change in those scripts should involve an extension version bump, which is a pain. However, since these changes are purely cosmetic and make no functional difference, I think we can get away without doing that. --- doc/src/sgml/gist.sgml | 75 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 18 deletions(-) (limited to 'doc/src/sgml/gist.sgml') diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml index 2d1a5aa863f..b3cc347e5cc 100644 --- a/doc/src/sgml/gist.sgml +++ b/doc/src/sgml/gist.sgml @@ -359,9 +359,20 @@ my_consistent(PG_FUNCTION_ARGS) the value being looked up in the index. The StrategyNumber parameter indicates which operator of your operator class is being applied — it matches one of the operator numbers in the - CREATE OPERATOR CLASS command. Depending on what operators - you have included in the class, the data type of query could - vary with the operator, but the above skeleton assumes it doesn't. + CREATE OPERATOR CLASS command. + + + + Depending on which operators you have included in the class, the data + type of query could vary with the operator, since it will + be whatever type is on the righthand side of the operator, which might + be different from the indexed data type appearing on the lefthand side. + (The above code skeleton assumes that only one type is possible; if + not, fetching the query argument value would have to depend + on the operator.) It is recommended that the SQL declaration of + the consistent function use the opclass's indexed data + type for the query argument, even though the actual type + might be something else depending on the operator. @@ -381,7 +392,7 @@ my_consistent(PG_FUNCTION_ARGS) CREATE OR REPLACE FUNCTION my_union(internal, internal) -RETURNS internal +RETURNS storage_type AS 'MODULE_PATHNAME' LANGUAGE C STRICT; @@ -434,9 +445,21 @@ my_union(PG_FUNCTION_ARGS) - The union implementation function should return a - pointer to newly palloc()ed memory. You can't just - return whatever the input is. + The result of the union function must be a value of the + index's storage type, whatever that is (it might or might not be + different from the indexed column's type). The union + function should return a pointer to newly palloc()ed + memory. You can't just return the input value as-is, even if there is + no type change. + + + + As shown above, the union function's + first internal argument is actually + a GistEntryVector pointer. The second argument is a + pointer to an integer variable, which can be ignored. (It used to be + required that the union function store the size of its + result value into that variable, but this is no longer necessary.) @@ -576,6 +599,12 @@ my_penalty(PG_FUNCTION_ARGS) PG_RETURN_POINTER(penalty); } + + For historical reasons, the penalty function doesn't + just return a float result; instead it has to store the value + at the location indicated by the third argument. The return + value per se is ignored, though it's conventional to pass back the + address of that argument. @@ -615,9 +644,9 @@ Datum my_picksplit(PG_FUNCTION_ARGS) { GistEntryVector *entryvec = (GistEntryVector *) PG_GETARG_POINTER(0); + GIST_SPLITVEC *v = (GIST_SPLITVEC *) PG_GETARG_POINTER(1); OffsetNumber maxoff = entryvec->n - 1; GISTENTRY *ent = entryvec->vector; - GIST_SPLITVEC *v = (GIST_SPLITVEC *) PG_GETARG_POINTER(1); int i, nbytes; OffsetNumber *left, @@ -683,6 +712,11 @@ my_picksplit(PG_FUNCTION_ARGS) PG_RETURN_POINTER(v); } + + Notice that the picksplit function's result is delivered + by modifying the passed-in v structure. The return + value per se is ignored, though it's conventional to pass back the + address of v. @@ -700,13 +734,15 @@ my_picksplit(PG_FUNCTION_ARGS) Returns true if two index entries are identical, false otherwise. + (An index entry is a value of the index's storage type, + not necessarily the original indexed column's type.) The SQL declaration of the function must look like this: -CREATE OR REPLACE FUNCTION my_same(internal, internal, internal) +CREATE OR REPLACE FUNCTION my_same(storage_type, storage_type, internal) RETURNS internal AS 'MODULE_PATHNAME' LANGUAGE C STRICT; @@ -731,7 +767,9 @@ my_same(PG_FUNCTION_ARGS) For historical reasons, the same function doesn't just return a Boolean result; instead it has to store the flag - at the location indicated by the third argument. + at the location indicated by the third argument. The return + value per se is ignored, though it's conventional to pass back the + address of that argument. @@ -756,7 +794,7 @@ my_same(PG_FUNCTION_ARGS) The SQL declaration of the function must look like this: -CREATE OR REPLACE FUNCTION my_distance(internal, data_type, smallint, oid) +CREATE OR REPLACE FUNCTION my_distance(internal, data_type, smallint, oid, internal) RETURNS float8 AS 'MODULE_PATHNAME' LANGUAGE C STRICT; @@ -824,7 +862,7 @@ my_distance(PG_FUNCTION_ARGS) fetch - Converts the compressed index representation of the data item into the + Converts the compressed index representation of a data item into the original data type, for index-only scans. The returned data must be an exact, non-lossy copy of the originally indexed value. @@ -840,11 +878,12 @@ LANGUAGE C STRICT; The argument is a pointer to a GISTENTRY struct. On - entry, its 'key' field contains a non-NULL leaf datum in its + entry, its key field contains a non-NULL leaf datum in compressed form. The return value is another GISTENTRY - struct, whose 'key' field contains the same datum in the original, - uncompressed form. If the opclass' compress function does nothing for - leaf entries, the fetch method can return the argument as is. + struct, whose key field contains the same datum in its + original, uncompressed form. If the opclass's compress function does + nothing for leaf entries, the fetch method can return the + argument as-is. @@ -879,8 +918,8 @@ my_fetch(PG_FUNCTION_ARGS) If the compress method is lossy for leaf entries, the operator class - cannot support index-only scans, and must not define a 'fetch' - function. + cannot support index-only scans, and must not define + a fetch function. -- cgit v1.2.3