On Fri, Apr 22, 2016 at 1:30 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Andrew Dunstan <and...@dunslane.net> writes:
>> On 04/21/2016 05:15 PM, Tom Lane wrote:
>>> Do the other contrib modules all pass? I can't recall if seg was the
>>> only one we'd left like this.
>
>> Only seg fails.
>
> As a crosscheck, I put some code into fmgr_c_validator() to log a message
> when creating a V0 function with a pass-by-val return type. (Pass-by-ref
> is no problem, according to my hypothesis, since it necessarily means
> the C function returns a pointer.) I get these hits in core + contrib
> regression tests:
>
> [...]
>
> If we assume that oldstyle functions returning integer are still okay,
> which the success of the regression test case involving oldstyle_length()
> seems to prove, then indeed seg's bool-returning functions are the only
> hazard.
>
> Note though that this test fails to cover any contrib modules that
> lack regression tests, since they wouldn't have gotten loaded by
> "make installcheck".
Your assumption is right. With the patch attached for contrib/seg/
that converts all those functions to use the V1 declaration, I am able
to make the tests pass. As the internal shape of the functions is not
changed and that there are no functional changes, I guess that it
would be fine to backpatch down to where VS2015 is intended to be
supported. Is anybody here foreseeing any problems for back-branches
if there is such a change?
--
Michael
diff --git a/contrib/seg/seg.c b/contrib/seg/seg.c
index c6c082b..c3651d4 100644
--- a/contrib/seg/seg.c
+++ b/contrib/seg/seg.c
@@ -47,52 +47,60 @@ PG_FUNCTION_INFO_V1(seg_center);
/*
** GiST support methods
*/
-bool gseg_consistent(GISTENTRY *entry,
- SEG *query,
- StrategyNumber strategy,
- Oid subtype,
- bool *recheck);
-GISTENTRY *gseg_compress(GISTENTRY *entry);
-GISTENTRY *gseg_decompress(GISTENTRY *entry);
-float *gseg_penalty(GISTENTRY *origentry, GISTENTRY *newentry, float *result);
-GIST_SPLITVEC *gseg_picksplit(GistEntryVector *entryvec, GIST_SPLITVEC *v);
+PG_FUNCTION_INFO_V1(gseg_consistent);
+PG_FUNCTION_INFO_V1(gseg_compress);
+PG_FUNCTION_INFO_V1(gseg_decompress);
+PG_FUNCTION_INFO_V1(gseg_penalty);
+PG_FUNCTION_INFO_V1(gseg_picksplit);
+PG_FUNCTION_INFO_V1(gseg_same);
+PG_FUNCTION_INFO_V1(gseg_union);
+
bool gseg_leaf_consistent(SEG *key, SEG *query, StrategyNumber strategy);
bool gseg_internal_consistent(SEG *key, SEG *query, StrategyNumber strategy);
-SEG *gseg_union(GistEntryVector *entryvec, int *sizep);
SEG *gseg_binary_union(SEG *r1, SEG *r2, int *sizep);
-bool *gseg_same(SEG *b1, SEG *b2, bool *result);
/*
** R-tree support functions
*/
-bool seg_same(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_same);
+static inline bool seg_same_internal(SEG *a, SEG *b);
bool seg_contains_int(SEG *a, int *b);
bool seg_contains_float4(SEG *a, float4 *b);
bool seg_contains_float8(SEG *a, float8 *b);
-bool seg_contains(SEG *a, SEG *b);
-bool seg_contained(SEG *a, SEG *b);
-bool seg_overlap(SEG *a, SEG *b);
-bool seg_left(SEG *a, SEG *b);
-bool seg_over_left(SEG *a, SEG *b);
-bool seg_right(SEG *a, SEG *b);
-bool seg_over_right(SEG *a, SEG *b);
-SEG *seg_union(SEG *a, SEG *b);
-SEG *seg_inter(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_contains);
+static inline bool seg_contains_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_contained);
+static inline bool seg_contained_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_overlap);
+static inline bool seg_overlap_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_left);
+static inline bool seg_left_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_over_left);
+static inline bool seg_over_left_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_right);
+static inline bool seg_right_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_over_right);
+static inline bool seg_over_right_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_union);
+static inline SEG *seg_union_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_inter);
void rt_seg_size(SEG *a, float *sz);
/*
** Various operators
*/
-int32 seg_cmp(SEG *a, SEG *b);
-bool seg_lt(SEG *a, SEG *b);
-bool seg_le(SEG *a, SEG *b);
-bool seg_gt(SEG *a, SEG *b);
-bool seg_ge(SEG *a, SEG *b);
-bool seg_different(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_cmp);
+static inline int32 seg_cmp_internal(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_lt);
+PG_FUNCTION_INFO_V1(seg_le);
+PG_FUNCTION_INFO_V1(seg_gt);
+PG_FUNCTION_INFO_V1(seg_ge);
+PG_FUNCTION_INFO_V1(seg_different);
+
/*
-** Auxiliary funxtions
+** Auxiliary functions
*/
static int restore(char *s, float val, int n);
@@ -193,13 +201,15 @@ seg_upper(PG_FUNCTION_ARGS)
** the predicate x op query == FALSE, where op is the oper
** corresponding to strategy in the pg_amop table.
*/
-bool
-gseg_consistent(GISTENTRY *entry,
- SEG *query,
- StrategyNumber strategy,
- Oid subtype,
- bool *recheck)
+Datum
+gseg_consistent(PG_FUNCTION_ARGS)
{
+ GISTENTRY *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
+ SEG *query = (SEG *) PG_GETARG_POINTER(1);
+ StrategyNumber strategy = (StrategyNumber) PG_GETARG_UINT16(2);
+ /* Oid subtype = PG_GETARG_OID(3); */
+ bool *recheck = (bool *) PG_GETARG_POINTER(4);
+
/* All cases served by this function are exact */
*recheck = false;
@@ -217,9 +227,11 @@ gseg_consistent(GISTENTRY *entry,
** The GiST Union method for segments
** returns the minimal bounding seg that encloses all the entries in entryvec
*/
-SEG *
-gseg_union(GistEntryVector *entryvec, int *sizep)
+Datum
+gseg_union(PG_FUNCTION_ARGS)
{
+ GistEntryVector *entryvec = (GistEntryVector *) PG_GETARG_POINTER(0);
+ int *sizep = (int *) PG_GETARG_POINTER(1);
int numranges,
i;
SEG *out = (SEG *) NULL;
@@ -241,37 +253,42 @@ gseg_union(GistEntryVector *entryvec, int *sizep)
tmp = out;
}
- return (out);
+ PG_RETURN_POINTER(out);
}
/*
** GiST Compress and Decompress methods for segments
** do not do anything.
*/
-GISTENTRY *
-gseg_compress(GISTENTRY *entry)
+Datum
+gseg_compress(PG_FUNCTION_ARGS)
{
- return (entry);
+ GISTENTRY *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
+ PG_RETURN_POINTER(entry);
}
-GISTENTRY *
-gseg_decompress(GISTENTRY *entry)
+Datum
+gseg_decompress(PG_FUNCTION_ARGS)
{
- return (entry);
+ GISTENTRY *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
+ PG_RETURN_POINTER(entry);
}
/*
** The GiST Penalty method for segments
** As in the R-tree paper, we use change in area as our penalty metric
*/
-float *
-gseg_penalty(GISTENTRY *origentry, GISTENTRY *newentry, float *result)
+Datum
+gseg_penalty(PG_FUNCTION_ARGS)
{
+ GISTENTRY *origentry = (GISTENTRY *) PG_GETARG_POINTER(0);
+ GISTENTRY *newentry = (GISTENTRY *) PG_GETARG_POINTER(0);
+ float *result = (float *) PG_GETARG_POINTER(2);
SEG *ud;
float tmp1,
tmp2;
- ud = seg_union((SEG *) DatumGetPointer(origentry->key),
+ ud = seg_union_internal((SEG *) DatumGetPointer(origentry->key),
(SEG *) DatumGetPointer(newentry->key));
rt_seg_size(ud, &tmp1);
rt_seg_size((SEG *) DatumGetPointer(origentry->key), &tmp2);
@@ -282,7 +299,7 @@ gseg_penalty(GISTENTRY *origentry, GISTENTRY *newentry, float *result)
fprintf(stderr, "\t%g\n", *result);
#endif
- return (result);
+ PG_RETURN_POINTER(result);
}
/*
@@ -309,10 +326,11 @@ gseg_picksplit_item_cmp(const void *a, const void *b)
* it's easier and more robust to just sort the segments by center-point and
* split at the middle.
*/
-GIST_SPLITVEC *
-gseg_picksplit(GistEntryVector *entryvec,
- GIST_SPLITVEC *v)
+Datum
+gseg_picksplit(PG_FUNCTION_ARGS)
{
+ GistEntryVector *entryvec = (GistEntryVector *) PG_GETARG_POINTER(0);
+ GIST_SPLITVEC *v = (GIST_SPLITVEC *) PG_GETARG_POINTER(1);
int i;
SEG *datum_l,
*datum_r,
@@ -365,7 +383,7 @@ gseg_picksplit(GistEntryVector *entryvec,
v->spl_nleft++;
for (i = 1; i < firstright; i++)
{
- datum_l = seg_union(datum_l, sort_items[i].data);
+ datum_l = seg_union_internal(datum_l, sort_items[i].data);
*left++ = sort_items[i].index;
v->spl_nleft++;
}
@@ -379,7 +397,7 @@ gseg_picksplit(GistEntryVector *entryvec,
v->spl_nright++;
for (i = firstright + 1; i < maxoff; i++)
{
- datum_r = seg_union(datum_r, sort_items[i].data);
+ datum_r = seg_union_internal(datum_r, sort_items[i].data);
*right++ = sort_items[i].index;
v->spl_nright++;
}
@@ -387,16 +405,20 @@ gseg_picksplit(GistEntryVector *entryvec,
v->spl_ldatum = PointerGetDatum(datum_l);
v->spl_rdatum = PointerGetDatum(datum_r);
- return v;
+ PG_RETURN_POINTER(v);
}
/*
** Equality methods
*/
-bool *
-gseg_same(SEG *b1, SEG *b2, bool *result)
+Datum
+gseg_same(PG_FUNCTION_ARGS)
{
- if (seg_same(b1, b2))
+ SEG *b1 = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b2 = (SEG *) PG_GETARG_POINTER(1);
+ bool *result = (bool *) PG_GETARG_POINTER(2);
+
+ if (seg_same_internal(b1, b2))
*result = TRUE;
else
*result = FALSE;
@@ -405,7 +427,7 @@ gseg_same(SEG *b1, SEG *b2, bool *result)
fprintf(stderr, "same: %s\n", (*result ? "TRUE" : "FALSE"));
#endif
- return (result);
+ PG_RETURN_POINTER(result);
}
/*
@@ -425,30 +447,30 @@ gseg_leaf_consistent(SEG *key,
switch (strategy)
{
case RTLeftStrategyNumber:
- retval = (bool) seg_left(key, query);
+ retval = (bool) seg_left_internal(key, query);
break;
case RTOverLeftStrategyNumber:
- retval = (bool) seg_over_left(key, query);
+ retval = (bool) seg_over_left_internal(key, query);
break;
case RTOverlapStrategyNumber:
- retval = (bool) seg_overlap(key, query);
+ retval = (bool) seg_overlap_internal(key, query);
break;
case RTOverRightStrategyNumber:
- retval = (bool) seg_over_right(key, query);
+ retval = (bool) seg_over_right_internal(key, query);
break;
case RTRightStrategyNumber:
- retval = (bool) seg_right(key, query);
+ retval = (bool) seg_right_internal(key, query);
break;
case RTSameStrategyNumber:
- retval = (bool) seg_same(key, query);
+ retval = (bool) seg_same_internal(key, query);
break;
case RTContainsStrategyNumber:
case RTOldContainsStrategyNumber:
- retval = (bool) seg_contains(key, query);
+ retval = (bool) seg_contains_internal(key, query);
break;
case RTContainedByStrategyNumber:
case RTOldContainedByStrategyNumber:
- retval = (bool) seg_contained(key, query);
+ retval = (bool) seg_contained_internal(key, query);
break;
default:
retval = FALSE;
@@ -470,28 +492,28 @@ gseg_internal_consistent(SEG *key,
switch (strategy)
{
case RTLeftStrategyNumber:
- retval = (bool) !seg_over_right(key, query);
+ retval = (bool) !seg_over_right_internal(key, query);
break;
case RTOverLeftStrategyNumber:
- retval = (bool) !seg_right(key, query);
+ retval = (bool) !seg_right_internal(key, query);
break;
case RTOverlapStrategyNumber:
- retval = (bool) seg_overlap(key, query);
+ retval = (bool) seg_overlap_internal(key, query);
break;
case RTOverRightStrategyNumber:
- retval = (bool) !seg_left(key, query);
+ retval = (bool) !seg_left_internal(key, query);
break;
case RTRightStrategyNumber:
- retval = (bool) !seg_over_left(key, query);
+ retval = (bool) !seg_over_left_internal(key, query);
break;
case RTSameStrategyNumber:
case RTContainsStrategyNumber:
case RTOldContainsStrategyNumber:
- retval = (bool) seg_contains(key, query);
+ retval = (bool) seg_contains_internal(key, query);
break;
case RTContainedByStrategyNumber:
case RTOldContainedByStrategyNumber:
- retval = (bool) seg_overlap(key, query);
+ retval = (bool) seg_overlap_internal(key, query);
break;
default:
retval = FALSE;
@@ -504,39 +526,71 @@ gseg_binary_union(SEG *r1, SEG *r2, int *sizep)
{
SEG *retval;
- retval = seg_union(r1, r2);
+ retval = seg_union_internal(r1, r2);
*sizep = sizeof(SEG);
return (retval);
}
+Datum
+seg_contains(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_contains_internal(a, b));
+}
bool
-seg_contains(SEG *a, SEG *b)
+seg_contains_internal(SEG *a, SEG *b)
{
return ((a->lower <= b->lower) && (a->upper >= b->upper));
}
+Datum
+seg_contained(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_contained_internal(a, b));
+}
bool
-seg_contained(SEG *a, SEG *b)
+seg_contained_internal(SEG *a, SEG *b)
{
- return (seg_contains(b, a));
+ return (seg_contains_internal(b, a));
}
/*****************************************************************************
* Operator class for R-tree indexing
*****************************************************************************/
-bool
-seg_same(SEG *a, SEG *b)
+Datum
+seg_same(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_same_internal(a, b));
+}
+static inline bool
+seg_same_internal(SEG *a, SEG *b)
{
- return seg_cmp(a, b) == 0;
+ return seg_cmp_internal(a, b) == 0;
}
/* seg_overlap -- does a overlap b?
*/
-bool
-seg_overlap(SEG *a, SEG *b)
+Datum
+seg_overlap(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_overlap_internal(a, b));
+}
+static inline bool
+seg_overlap_internal(SEG *a, SEG *b)
{
return (
((a->upper >= b->upper) && (a->lower <= b->upper))
@@ -547,39 +601,79 @@ seg_overlap(SEG *a, SEG *b)
/* seg_overleft -- is the right edge of (a) located at or left of the right edge of (b)?
*/
-bool
-seg_over_left(SEG *a, SEG *b)
+Datum
+seg_over_left(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_over_left_internal(a, b));
+}
+static inline bool
+seg_over_left_internal(SEG *a, SEG *b)
{
return (a->upper <= b->upper);
}
/* seg_left -- is (a) entirely on the left of (b)?
*/
-bool
-seg_left(SEG *a, SEG *b)
+Datum
+seg_left(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_left_internal(a, b));
+}
+static inline bool
+seg_left_internal(SEG *a, SEG *b)
{
return (a->upper < b->lower);
}
/* seg_right -- is (a) entirely on the right of (b)?
*/
-bool
-seg_right(SEG *a, SEG *b)
+Datum
+seg_right(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_right_internal(a, b));
+}
+static inline bool
+seg_right_internal(SEG *a, SEG *b)
{
return (a->lower > b->upper);
}
/* seg_overright -- is the left edge of (a) located at or right of the left edge of (b)?
*/
-bool
-seg_over_right(SEG *a, SEG *b)
+Datum
+seg_over_right(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_over_right_internal(a, b));
+}
+static inline bool
+seg_over_right_internal(SEG *a, SEG *b)
{
return (a->lower >= b->lower);
}
-SEG *
-seg_union(SEG *a, SEG *b)
+Datum
+seg_union(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_POINTER(seg_union_internal(a, b));
+}
+static inline SEG *
+seg_union_internal(SEG *a, SEG *b)
{
SEG *n;
@@ -617,10 +711,12 @@ seg_union(SEG *a, SEG *b)
}
-SEG *
-seg_inter(SEG *a, SEG *b)
+Datum
+seg_inter(PG_FUNCTION_ARGS)
{
- SEG *n;
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+ SEG *n;
n = (SEG *) palloc(sizeof(*n));
@@ -652,7 +748,7 @@ seg_inter(SEG *a, SEG *b)
n->l_ext = b->l_ext;
}
- return (n);
+ PG_RETURN_POINTER(n);
}
void
@@ -678,8 +774,17 @@ seg_size(PG_FUNCTION_ARGS)
/*****************************************************************************
* Miscellaneous operators
*****************************************************************************/
-int32
-seg_cmp(SEG *a, SEG *b)
+Datum
+seg_cmp(PG_FUNCTION_ARGS)
+{
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_INT32(seg_cmp_internal(a, b));
+}
+
+static inline int32
+seg_cmp_internal(SEG *a, SEG *b)
{
/*
* First compare on lower boundary position
@@ -797,34 +902,49 @@ seg_cmp(SEG *a, SEG *b)
return 0;
}
-bool
-seg_lt(SEG *a, SEG *b)
+Datum
+seg_lt(PG_FUNCTION_ARGS)
{
- return seg_cmp(a, b) < 0;
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_cmp_internal(a, b) < 0);
}
-bool
-seg_le(SEG *a, SEG *b)
+Datum
+seg_le(PG_FUNCTION_ARGS)
{
- return seg_cmp(a, b) <= 0;
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_cmp_internal(a, b) <= 0);
}
-bool
-seg_gt(SEG *a, SEG *b)
+Datum
+seg_gt(PG_FUNCTION_ARGS)
{
- return seg_cmp(a, b) > 0;
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_cmp_internal(a, b) > 0);
}
-bool
-seg_ge(SEG *a, SEG *b)
+Datum
+seg_ge(PG_FUNCTION_ARGS)
{
- return seg_cmp(a, b) >= 0;
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_cmp_internal(a, b) >= 0);
}
-bool
-seg_different(SEG *a, SEG *b)
+Datum
+seg_different(PG_FUNCTION_ARGS)
{
- return seg_cmp(a, b) != 0;
+ SEG *a = (SEG *) PG_GETARG_POINTER(0);
+ SEG *b = (SEG *) PG_GETARG_POINTER(1);
+
+ PG_RETURN_BOOL(seg_cmp_internal(a, b) != 0);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
