On 2016-12-08 13:34:41 -0800, Andres Freund wrote: > Hi, > > I'm wondering if it's not time for $subject: > - V0 causes confusion / weird crashes when PG_FUNCTION_INFO_V1 was > forgotten > - They have us keep weird hacks around just for the sake of testing V0 > - they actually cost performance, because we have to zero initialize Datums, > even if > the corresponding isnull marker is set. > - they allow to call arbitrary functions pretty easily > > I don't see any reason to keep them around. If seriously doubt anybody > is using them seriously in anything but error.
Patches attached.
>From dc6de0b47e1013c165c1026df65bda62cba8d8b7 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 28 Feb 2017 23:14:58 -0800 Subject: [PATCH 1/2] Move contrib/seg to only use V1 calling conventions. A later commit will remove V0 support. Author: Andres Freund Discussion: https://postgr.es/m/20161208213441.k3mbno4twhg2q...@alap3.anarazel.de --- contrib/seg/seg.c | 453 +++++++++++++++++++++++++++++------------------------- 1 file changed, 244 insertions(+), 209 deletions(-) diff --git a/contrib/seg/seg.c b/contrib/seg/seg.c index 895d879498..ab6b017282 100644 --- a/contrib/seg/seg.c +++ b/contrib/seg/seg.c @@ -47,60 +47,53 @@ 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); -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); +PG_FUNCTION_INFO_V1(gseg_consistent); +PG_FUNCTION_INFO_V1(gseg_compress); +PG_FUNCTION_INFO_V1(gseg_decompress); +PG_FUNCTION_INFO_V1(gseg_picksplit); +PG_FUNCTION_INFO_V1(gseg_penalty); +PG_FUNCTION_INFO_V1(gseg_union); +PG_FUNCTION_INFO_V1(gseg_same); +static Datum gseg_leaf_consistent(Datum key, Datum query, StrategyNumber strategy); +static Datum gseg_internal_consistent(Datum key, Datum query, StrategyNumber strategy); +static Datum gseg_binary_union(Datum r1, Datum r2, int *sizep); /* ** R-tree support functions */ -bool seg_same(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); -void rt_seg_size(SEG *a, float *sz); +PG_FUNCTION_INFO_V1(seg_same); +PG_FUNCTION_INFO_V1(seg_contains); +PG_FUNCTION_INFO_V1(seg_contained); +PG_FUNCTION_INFO_V1(seg_overlap); +PG_FUNCTION_INFO_V1(seg_left); +PG_FUNCTION_INFO_V1(seg_over_left); +PG_FUNCTION_INFO_V1(seg_right); +PG_FUNCTION_INFO_V1(seg_over_right); +PG_FUNCTION_INFO_V1(seg_union); +PG_FUNCTION_INFO_V1(seg_inter); +static void rt_seg_size(SEG *a, float *size); /* ** 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); +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 */ static int restore(char *s, float val, int n); - /***************************************************************************** * Input/Output functions *****************************************************************************/ + Datum seg_in(PG_FUNCTION_ARGS) { @@ -193,13 +186,17 @@ 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); + Datum query = PG_GETARG_DATUM(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; @@ -208,71 +205,74 @@ gseg_consistent(GISTENTRY *entry, * gseg_leaf_consistent */ if (GIST_LEAF(entry)) - return (gseg_leaf_consistent((SEG *) DatumGetPointer(entry->key), query, strategy)); + return gseg_leaf_consistent(entry->key, query, strategy); else - return (gseg_internal_consistent((SEG *) DatumGetPointer(entry->key), query, strategy)); + return gseg_internal_consistent(entry->key, query, strategy); } /* ** 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(0); int numranges, i; - SEG *out = (SEG *) NULL; - SEG *tmp; + Datum out = 0; + Datum tmp; #ifdef GIST_DEBUG fprintf(stderr, "union\n"); #endif numranges = entryvec->n; - tmp = (SEG *) DatumGetPointer(entryvec->vector[0].key); + tmp = entryvec->vector[0].key; *sizep = sizeof(SEG); for (i = 1; i < numranges; i++) { - out = gseg_binary_union(tmp, (SEG *) - DatumGetPointer(entryvec->vector[i].key), - sizep); + out = gseg_binary_union(tmp, entryvec->vector[i].key, sizep); tmp = out; } - return (out); + PG_RETURN_DATUM(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); + PG_RETURN_POINTER(PG_GETARG_POINTER(0)); } -GISTENTRY * -gseg_decompress(GISTENTRY *entry) +Datum +gseg_decompress(PG_FUNCTION_ARGS) { - return (entry); + PG_RETURN_POINTER(PG_GETARG_POINTER(0)); } /* ** 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(1); + float *result = (float *) PG_GETARG_POINTER(2); SEG *ud; float tmp1, tmp2; - ud = seg_union((SEG *) DatumGetPointer(origentry->key), - (SEG *) DatumGetPointer(newentry->key)); + ud = (SEG *) DatumGetPointer( + DirectFunctionCall2(seg_union, origentry->key, newentry->key)); rt_seg_size(ud, &tmp1); rt_seg_size((SEG *) DatumGetPointer(origentry->key), &tmp2); *result = tmp1 - tmp2; @@ -282,7 +282,7 @@ gseg_penalty(GISTENTRY *origentry, GISTENTRY *newentry, float *result) fprintf(stderr, "\t%g\n", *result); #endif - return (result); + PG_RETURN_POINTER(result); } /* @@ -309,14 +309,15 @@ 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, - *seg; + Datum datum_l, + datum_r; + SEG *seg; gseg_picksplit_item *sort_items; OffsetNumber *left, *right; @@ -359,13 +360,14 @@ gseg_picksplit(GistEntryVector *entryvec, /* * Emit segments to the left output page, and compute its bounding box. */ - datum_l = (SEG *) palloc(sizeof(SEG)); - memcpy(datum_l, sort_items[0].data, sizeof(SEG)); + datum_l = PointerGetDatum(palloc(sizeof(SEG))); + memcpy(DatumGetPointer(datum_l), sort_items[0].data, sizeof(SEG)); *left++ = sort_items[0].index; v->spl_nleft++; for (i = 1; i < firstright; i++) { - datum_l = seg_union(datum_l, sort_items[i].data); + datum_l = DirectFunctionCall2(seg_union, datum_l, + PointerGetDatum(sort_items[i].data)); *left++ = sort_items[i].index; v->spl_nleft++; } @@ -373,13 +375,14 @@ gseg_picksplit(GistEntryVector *entryvec, /* * Likewise for the right page. */ - datum_r = (SEG *) palloc(sizeof(SEG)); - memcpy(datum_r, sort_items[firstright].data, sizeof(SEG)); + datum_r = PointerGetDatum(palloc(sizeof(SEG))); + memcpy(DatumGetPointer(datum_r), sort_items[firstright].data, sizeof(SEG)); *right++ = sort_items[firstright].index; v->spl_nright++; for (i = firstright + 1; i < maxoff; i++) { - datum_r = seg_union(datum_r, sort_items[i].data); + datum_r = DirectFunctionCall2(seg_union, datum_r, + PointerGetDatum(sort_items[i].data)); *right++ = sort_items[i].index; v->spl_nright++; } @@ -387,16 +390,18 @@ 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)) + bool *result = (bool *) PG_GETARG_POINTER(2); + + if (DirectFunctionCall2(seg_same, PG_GETARG_DATUM(0), PG_GETARG_DATUM(2))) *result = TRUE; else *result = FALSE; @@ -405,18 +410,16 @@ gseg_same(SEG *b1, SEG *b2, bool *result) fprintf(stderr, "same: %s\n", (*result ? "TRUE" : "FALSE")); #endif - return (result); + PG_RETURN_POINTER(result); } /* ** SUPPORT ROUTINES */ -bool -gseg_leaf_consistent(SEG *key, - SEG *query, - StrategyNumber strategy) +Datum +gseg_leaf_consistent(Datum key, Datum query, StrategyNumber strategy) { - bool retval; + Datum retval; #ifdef GIST_QUERY_DEBUG fprintf(stderr, "leaf_consistent, %d\n", strategy); @@ -425,41 +428,41 @@ gseg_leaf_consistent(SEG *key, switch (strategy) { case RTLeftStrategyNumber: - retval = (bool) seg_left(key, query); + retval = DirectFunctionCall2(seg_left, key, query); break; case RTOverLeftStrategyNumber: - retval = (bool) seg_over_left(key, query); + retval = DirectFunctionCall2(seg_over_left, key, query); break; case RTOverlapStrategyNumber: - retval = (bool) seg_overlap(key, query); + retval = DirectFunctionCall2(seg_overlap, key, query); break; case RTOverRightStrategyNumber: - retval = (bool) seg_over_right(key, query); + retval = DirectFunctionCall2(seg_over_right, key, query); break; case RTRightStrategyNumber: - retval = (bool) seg_right(key, query); + retval = DirectFunctionCall2(seg_right, key, query); break; case RTSameStrategyNumber: - retval = (bool) seg_same(key, query); + retval = DirectFunctionCall2(seg_same, key, query); break; case RTContainsStrategyNumber: case RTOldContainsStrategyNumber: - retval = (bool) seg_contains(key, query); + retval = DirectFunctionCall2(seg_contains, key, query); + break; case RTContainedByStrategyNumber: case RTOldContainedByStrategyNumber: - retval = (bool) seg_contained(key, query); + retval = DirectFunctionCall2(seg_contained, key, query); break; default: retval = FALSE; } - return (retval); + + PG_RETURN_DATUM(retval); } -bool -gseg_internal_consistent(SEG *key, - SEG *query, - StrategyNumber strategy) +static Datum +gseg_internal_consistent(Datum key, Datum query, StrategyNumber strategy) { bool retval; @@ -470,117 +473,147 @@ gseg_internal_consistent(SEG *key, switch (strategy) { case RTLeftStrategyNumber: - retval = (bool) !seg_over_right(key, query); + retval = + !DatumGetBool(DirectFunctionCall2(seg_over_right, key, query)); break; case RTOverLeftStrategyNumber: - retval = (bool) !seg_right(key, query); + retval = + !DatumGetBool(DirectFunctionCall2(seg_right, key, query)); break; case RTOverlapStrategyNumber: - retval = (bool) seg_overlap(key, query); + retval = + !DatumGetBool(DirectFunctionCall2(seg_overlap, key, query)); break; case RTOverRightStrategyNumber: - retval = (bool) !seg_left(key, query); + retval = + !DatumGetBool(DirectFunctionCall2(seg_left, key, query)); break; case RTRightStrategyNumber: - retval = (bool) !seg_over_left(key, query); + retval = + !DatumGetBool(DirectFunctionCall2(seg_over_left, key, query)); break; case RTSameStrategyNumber: case RTContainsStrategyNumber: case RTOldContainsStrategyNumber: - retval = (bool) seg_contains(key, query); + retval = + DatumGetBool(DirectFunctionCall2(seg_contains, key, query)); break; case RTContainedByStrategyNumber: case RTOldContainedByStrategyNumber: - retval = (bool) seg_overlap(key, query); + retval = + DatumGetBool(DirectFunctionCall2(seg_overlap, key, query)); break; default: retval = FALSE; } - return (retval); + + PG_RETURN_BOOL(retval); } -SEG * -gseg_binary_union(SEG *r1, SEG *r2, int *sizep) +static Datum +gseg_binary_union(Datum r1, Datum r2, int *sizep) { - SEG *retval; + Datum retval; - retval = seg_union(r1, r2); + retval = DirectFunctionCall2(seg_union, r1, r2); *sizep = sizeof(SEG); return (retval); } -bool -seg_contains(SEG *a, SEG *b) +Datum +seg_contains(PG_FUNCTION_ARGS) { - return ((a->lower <= b->lower) && (a->upper >= b->upper)); + SEG *a = (SEG *) PG_GETARG_POINTER(0); + SEG *b = (SEG *) PG_GETARG_POINTER(1); + + PG_RETURN_BOOL((a->lower <= b->lower) && (a->upper >= b->upper)); } -bool -seg_contained(SEG *a, SEG *b) +Datum +seg_contained(PG_FUNCTION_ARGS) { - return (seg_contains(b, a)); + PG_RETURN_DATUM( + DirectFunctionCall2(seg_contains, + PG_GETARG_DATUM(1), + PG_GETARG_DATUM(0))); } /***************************************************************************** * Operator class for R-tree indexing *****************************************************************************/ -bool -seg_same(SEG *a, SEG *b) +Datum +seg_same(PG_FUNCTION_ARGS) { - return seg_cmp(a, b) == 0; + Datum cmp = + DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1)); + + PG_RETURN_BOOL(DatumGetInt32(cmp) == 0); } /* seg_overlap -- does a overlap b? */ -bool -seg_overlap(SEG *a, SEG *b) +Datum +seg_overlap(PG_FUNCTION_ARGS) { - return ( - ((a->upper >= b->upper) && (a->lower <= b->upper)) - || - ((b->upper >= a->upper) && (b->lower <= a->upper)) - ); + SEG *a = (SEG *) PG_GETARG_POINTER(0); + SEG *b = (SEG *) PG_GETARG_POINTER(1); + + PG_RETURN_BOOL(((a->upper >= b->upper) && (a->lower <= b->upper)) || + ((b->upper >= a->upper) && (b->lower <= a->upper))); } -/* seg_overleft -- is the right edge of (a) located at or left of the right edge of (b)? +/* seg_over_left -- 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) { - return (a->upper <= b->upper); + SEG *a = (SEG *) PG_GETARG_POINTER(0); + SEG *b = (SEG *) PG_GETARG_POINTER(1); + + PG_RETURN_BOOL(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) { - return (a->upper < b->lower); + SEG *a = (SEG *) PG_GETARG_POINTER(0); + SEG *b = (SEG *) PG_GETARG_POINTER(1); + + PG_RETURN_BOOL(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) { - return (a->lower > b->upper); + SEG *a = (SEG *) PG_GETARG_POINTER(0); + SEG *b = (SEG *) PG_GETARG_POINTER(1); + + PG_RETURN_BOOL(a->lower > b->upper); } -/* seg_overright -- is the left edge of (a) located at or right of the left edge of (b)? +/* seg_over_right -- 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) { - return (a->lower >= b->lower); + SEG *a = (SEG *) PG_GETARG_POINTER(0); + SEG *b = (SEG *) PG_GETARG_POINTER(1); + + PG_RETURN_BOOL(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); SEG *n; n = (SEG *) palloc(sizeof(*n)); @@ -613,13 +646,14 @@ seg_union(SEG *a, SEG *b) n->l_ext = b->l_ext; } - return (n); + PG_RETURN_POINTER(n); } - -SEG * -seg_inter(SEG *a, SEG *b) +Datum +seg_inter(PG_FUNCTION_ARGS) { + SEG *a = (SEG *) PG_GETARG_POINTER(0); + SEG *b = (SEG *) PG_GETARG_POINTER(1); SEG *n; n = (SEG *) palloc(sizeof(*n)); @@ -652,10 +686,10 @@ seg_inter(SEG *a, SEG *b) n->l_ext = b->l_ext; } - return (n); + PG_RETURN_POINTER(n); } -void +static void rt_seg_size(SEG *a, float *size) { if (a == (SEG *) NULL || a->upper <= a->lower) @@ -678,16 +712,19 @@ 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); + /* * First compare on lower boundary position */ if (a->lower < b->lower) - return -1; + PG_RETURN_INT32(-1); if (a->lower > b->lower) - return 1; + PG_RETURN_INT32(1); /* * a->lower == b->lower, so consider type of boundary. @@ -699,27 +736,27 @@ seg_cmp(SEG *a, SEG *b) if (a->l_ext != b->l_ext) { if (a->l_ext == '-') - return -1; + PG_RETURN_INT32(-1); if (b->l_ext == '-') - return 1; + PG_RETURN_INT32(1); if (a->l_ext == '<') - return -1; + PG_RETURN_INT32(-1); if (b->l_ext == '<') - return 1; + PG_RETURN_INT32(1); if (a->l_ext == '>') - return 1; + PG_RETURN_INT32(1); if (b->l_ext == '>') - return -1; + PG_RETURN_INT32(-1); } /* * For other boundary types, consider # of significant digits first. */ if (a->l_sigd < b->l_sigd) /* (a) is blurred and is likely to include (b) */ - return -1; + PG_RETURN_INT32(-1); if (a->l_sigd > b->l_sigd) /* (a) is less blurred and is likely to be * included in (b) */ - return 1; + PG_RETURN_INT32(1); /* * For same # of digits, an approximate boundary is more blurred than @@ -728,9 +765,9 @@ seg_cmp(SEG *a, SEG *b) if (a->l_ext != b->l_ext) { if (a->l_ext == '~') /* (a) is approximate, while (b) is exact */ - return -1; + PG_RETURN_INT32(-1); if (b->l_ext == '~') - return 1; + PG_RETURN_INT32(1); /* can't get here unless data is corrupt */ elog(ERROR, "bogus lower boundary types %d %d", (int) a->l_ext, (int) b->l_ext); @@ -742,9 +779,9 @@ seg_cmp(SEG *a, SEG *b) * First compare on upper boundary position */ if (a->upper < b->upper) - return -1; + PG_RETURN_INT32(-1); if (a->upper > b->upper) - return 1; + PG_RETURN_INT32(1); /* * a->upper == b->upper, so consider type of boundary. @@ -756,17 +793,17 @@ seg_cmp(SEG *a, SEG *b) if (a->u_ext != b->u_ext) { if (a->u_ext == '-') - return 1; + PG_RETURN_INT32(1); if (b->u_ext == '-') - return -1; + PG_RETURN_INT32(-1); if (a->u_ext == '<') - return -1; + PG_RETURN_INT32(-1); if (b->u_ext == '<') - return 1; + PG_RETURN_INT32(1); if (a->u_ext == '>') - return 1; + PG_RETURN_INT32(1); if (b->u_ext == '>') - return -1; + PG_RETURN_INT32(-1); } /* @@ -774,10 +811,10 @@ seg_cmp(SEG *a, SEG *b) * result here is converse of the lower-boundary case. */ if (a->u_sigd < b->u_sigd) /* (a) is blurred and is likely to include (b) */ - return 1; + PG_RETURN_INT32(1); if (a->u_sigd > b->u_sigd) /* (a) is less blurred and is likely to be * included in (b) */ - return -1; + PG_RETURN_INT32(-1); /* * For same # of digits, an approximate boundary is more blurred than @@ -786,45 +823,61 @@ seg_cmp(SEG *a, SEG *b) if (a->u_ext != b->u_ext) { if (a->u_ext == '~') /* (a) is approximate, while (b) is exact */ - return 1; + PG_RETURN_INT32(1); if (b->u_ext == '~') - return -1; + PG_RETURN_INT32(-1); /* can't get here unless data is corrupt */ elog(ERROR, "bogus upper boundary types %d %d", (int) a->u_ext, (int) b->u_ext); } - return 0; + PG_RETURN_INT32(0); } -bool -seg_lt(SEG *a, SEG *b) +Datum +seg_lt(PG_FUNCTION_ARGS) { - return seg_cmp(a, b) < 0; + int cmp = DatumGetInt32( + DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1))); + + PG_RETURN_BOOL(cmp < 0); } -bool -seg_le(SEG *a, SEG *b) +Datum +seg_le(PG_FUNCTION_ARGS) { - return seg_cmp(a, b) <= 0; + int cmp = DatumGetInt32( + DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1))); + + PG_RETURN_BOOL(cmp <= 0); } -bool -seg_gt(SEG *a, SEG *b) +Datum +seg_gt(PG_FUNCTION_ARGS) { - return seg_cmp(a, b) > 0; + int cmp = DatumGetInt32( + DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1))); + + PG_RETURN_BOOL(cmp > 0); } -bool -seg_ge(SEG *a, SEG *b) +Datum +seg_ge(PG_FUNCTION_ARGS) { - return seg_cmp(a, b) >= 0; + int cmp = DatumGetInt32( + DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1))); + + PG_RETURN_BOOL(cmp >= 0); } -bool -seg_different(SEG *a, SEG *b) + +Datum +seg_different(PG_FUNCTION_ARGS) { - return seg_cmp(a, b) != 0; + int cmp = DatumGetInt32( + DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1))); + + PG_RETURN_BOOL(cmp != 0); } @@ -985,24 +1038,6 @@ restore(char *result, float val, int n) ** Miscellany */ -bool -seg_contains_int(SEG *a, int *b) -{ - return ((a->lower <= *b) && (a->upper >= *b)); -} - -bool -seg_contains_float4(SEG *a, float4 *b) -{ - return ((a->lower <= *b) && (a->upper >= *b)); -} - -bool -seg_contains_float8(SEG *a, float8 *b) -{ - return ((a->lower <= *b) && (a->upper >= *b)); -} - /* find out the number of significant digits in a string representing * a floating point number */ -- 2.11.0.22.g8d7a455.dirty
>From 6a61db32b4d52fee02157d6a93593a8cceab9fff Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 28 Feb 2017 23:14:58 -0800 Subject: [PATCH 2/2] Remove support for version-0 calling conventions. The V0 convention is failure prone because we've so far assumed that a function is V0 if PG_FUNCTION_INFO_V1 is missing, leading to crashes if a function was coded against the V1 interface. V0 doesn't allow proper NULL, SRF and toast handling. V0 doesn't offer features that V1 doesn't. Thus remove V0 support. Author: Andres Freund Discussion: https://postgr.es/m/20161208213441.k3mbno4twhg2q...@alap3.anarazel.de --- contrib/earthdistance/earthdistance.c | 22 -- doc/src/sgml/xfunc.sgml | 288 +++++------------ src/backend/utils/fmgr/fmgr.c | 377 +---------------------- src/include/fmgr.h | 8 +- src/test/regress/input/create_function_2.source | 5 - src/test/regress/input/misc.source | 13 - src/test/regress/output/create_function_2.source | 4 - src/test/regress/output/misc.source | 18 -- src/test/regress/regress.c | 47 ++- 9 files changed, 103 insertions(+), 679 deletions(-) diff --git a/contrib/earthdistance/earthdistance.c b/contrib/earthdistance/earthdistance.c index 861b166373..6ad6d87ce8 100644 --- a/contrib/earthdistance/earthdistance.c +++ b/contrib/earthdistance/earthdistance.c @@ -88,16 +88,8 @@ geo_distance_internal(Point *pt1, Point *pt2) * * returns: float8 * distance between the points in miles on earth's surface - * - * If float8 is passed-by-value, the oldstyle version-0 calling convention - * is unportable, so we use version-1. However, if it's passed-by-reference, - * continue to use oldstyle. This is just because we'd like earthdistance - * to serve as a canary for any unintentional breakage of version-0 functions - * with float8 results. ******************************************************/ -#ifdef USE_FLOAT8_BYVAL - PG_FUNCTION_INFO_V1(geo_distance); Datum @@ -110,17 +102,3 @@ geo_distance(PG_FUNCTION_ARGS) result = geo_distance_internal(pt1, pt2); PG_RETURN_FLOAT8(result); } -#else /* !USE_FLOAT8_BYVAL */ - -double *geo_distance(Point *pt1, Point *pt2); - -double * -geo_distance(Point *pt1, Point *pt2) -{ - double *resultp = palloc(sizeof(double)); - - *resultp = geo_distance_internal(pt1, pt2); - return resultp; -} - -#endif /* USE_FLOAT8_BYVAL */ diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 255bfddad7..cd41b89136 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -675,7 +675,7 @@ CREATE FUNCTION mleast(VARIADIC arr numeric[]) RETURNS numeric AS $$ $$ LANGUAGE SQL; SELECT mleast(10, -1, 5, 4.4); - mleast + mleast -------- -1 (1 row) @@ -775,19 +775,19 @@ AS $$ $$; SELECT foo(10, 20, 30); - foo + foo ----- 60 (1 row) SELECT foo(10, 20); - foo + foo ----- 33 (1 row) SELECT foo(10); - foo + foo ----- 15 (1 row) @@ -1209,13 +1209,13 @@ CREATE FUNCTION anyleast (VARIADIC anyarray) RETURNS anyelement AS $$ $$ LANGUAGE SQL; SELECT anyleast(10, -1, 5, 4); - anyleast + anyleast ---------- -1 (1 row) SELECT anyleast('abc'::text, 'def'); - anyleast + anyleast ---------- abc (1 row) @@ -1225,7 +1225,7 @@ CREATE FUNCTION concat_values(text, VARIADIC anyarray) RETURNS text AS $$ $$ LANGUAGE SQL; SELECT concat_values('|', 1, 4, 2); - concat_values + concat_values --------------- 1|4|2 (1 row) @@ -1610,14 +1610,11 @@ CREATE FUNCTION square_root(double precision) RETURNS double precision </para> <para> - Two different calling conventions are currently used for C functions. - The newer <quote>version 1</quote> calling convention is indicated by writing - a <literal>PG_FUNCTION_INFO_V1()</literal> macro call for the function, - as illustrated below. Lack of such a macro indicates an old-style - (<quote>version 0</quote>) function. The language name specified in <command>CREATE FUNCTION</command> - is <literal>C</literal> in either case. Old-style functions are now deprecated - because of portability problems and lack of functionality, but they - are still supported for compatibility reasons. + + Currently only one calling convention is used for C functions + (<quote>version 1</quote>). Support for that calling convention is + indicated by writing a <literal>PG_FUNCTION_INFO_V1()</literal> macro + call for the function, as illustrated below. </para> <sect2 id="xfunc-c-dynload"> @@ -1655,8 +1652,8 @@ CREATE FUNCTION square_root(double precision) RETURNS double precision <para> If the name starts with the string <literal>$libdir</literal>, that part is replaced by the <productname>PostgreSQL</> package - library directory - name, which is determined at build time.<indexterm><primary>$libdir</></> + library directory name, which is determined at build time. + <indexterm><primary>$libdir</></> </para> </listitem> @@ -2138,160 +2135,6 @@ memcpy(destination->data, buffer, 40); </sect2> <sect2> - <title>Version 0 Calling Conventions</title> - - <para> - We present the <quote>old style</quote> calling convention first — although - this approach is now deprecated, it's easier to get a handle on - initially. In the version-0 method, the arguments and result - of the C function are just declared in normal C style, but being - careful to use the C representation of each SQL data type as shown - above. - </para> - - <para> - Here are some examples: - -<programlisting><![CDATA[ -#include "postgres.h" -#include <string.h> -#include "utils/geo_decls.h" - -#ifdef PG_MODULE_MAGIC -PG_MODULE_MAGIC; -#endif - -/* by value */ - -int -add_one(int arg) -{ - return arg + 1; -} - -/* by reference, fixed length */ - -float8 * -add_one_float8(float8 *arg) -{ - float8 *result = (float8 *) palloc(sizeof(float8)); - - *result = *arg + 1.0; - - return result; -} - -Point * -makepoint(Point *pointx, Point *pointy) -{ - Point *new_point = (Point *) palloc(sizeof(Point)); - - new_point->x = pointx->x; - new_point->y = pointy->y; - - return new_point; -} - -/* by reference, variable length */ - -text * -copytext(text *t) -{ - /* - * VARSIZE is the total size of the struct in bytes. - */ - text *new_t = (text *) palloc(VARSIZE(t)); - SET_VARSIZE(new_t, VARSIZE(t)); - /* - * VARDATA is a pointer to the data region of the struct. - */ - memcpy((void *) VARDATA(new_t), /* destination */ - (void *) VARDATA(t), /* source */ - VARSIZE(t) - VARHDRSZ); /* how many bytes */ - return new_t; -} - -text * -concat_text(text *arg1, text *arg2) -{ - int32 new_text_size = VARSIZE(arg1) + VARSIZE(arg2) - VARHDRSZ; - text *new_text = (text *) palloc(new_text_size); - - SET_VARSIZE(new_text, new_text_size); - memcpy(VARDATA(new_text), VARDATA(arg1), VARSIZE(arg1) - VARHDRSZ); - memcpy(VARDATA(new_text) + (VARSIZE(arg1) - VARHDRSZ), - VARDATA(arg2), VARSIZE(arg2) - VARHDRSZ); - return new_text; -} -]]> -</programlisting> - </para> - - <para> - Supposing that the above code has been prepared in file - <filename>funcs.c</filename> and compiled into a shared object, - we could define the functions to <productname>PostgreSQL</productname> - with commands like this: - -<programlisting> -CREATE FUNCTION add_one(integer) RETURNS integer - AS '<replaceable>DIRECTORY</replaceable>/funcs', 'add_one' - LANGUAGE C STRICT; - --- note overloading of SQL function name "add_one" -CREATE FUNCTION add_one(double precision) RETURNS double precision - AS '<replaceable>DIRECTORY</replaceable>/funcs', 'add_one_float8' - LANGUAGE C STRICT; - -CREATE FUNCTION makepoint(point, point) RETURNS point - AS '<replaceable>DIRECTORY</replaceable>/funcs', 'makepoint' - LANGUAGE C STRICT; - -CREATE FUNCTION copytext(text) RETURNS text - AS '<replaceable>DIRECTORY</replaceable>/funcs', 'copytext' - LANGUAGE C STRICT; - -CREATE FUNCTION concat_text(text, text) RETURNS text - AS '<replaceable>DIRECTORY</replaceable>/funcs', 'concat_text' - LANGUAGE C STRICT; -</programlisting> - </para> - - <para> - Here, <replaceable>DIRECTORY</replaceable> stands for the - directory of the shared library file (for instance the - <productname>PostgreSQL</productname> tutorial directory, which - contains the code for the examples used in this section). - (Better style would be to use just <literal>'funcs'</> in the - <literal>AS</> clause, after having added - <replaceable>DIRECTORY</replaceable> to the search path. In any - case, we can omit the system-specific extension for a shared - library, commonly <literal>.so</literal> or - <literal>.sl</literal>.) - </para> - - <para> - Notice that we have specified the functions as <quote>strict</quote>, - meaning that - the system should automatically assume a null result if any input - value is null. By doing this, we avoid having to check for null inputs - in the function code. Without this, we'd have to check for null values - explicitly, by checking for a null pointer for each - pass-by-reference argument. (For pass-by-value arguments, we don't - even have a way to check!) - </para> - - <para> - Although this calling convention is simple to use, - it is not very portable; on some architectures there are problems - with passing data types that are smaller than <type>int</type> this way. Also, there is - no simple way to return a null result, nor to cope with null arguments - in any way other than making the function strict. The version-1 - convention, presented next, overcomes these objections. - </para> - </sect2> - - <sect2> <title>Version 1 Calling Conventions</title> <para> @@ -2316,8 +2159,10 @@ PG_FUNCTION_INFO_V1(funcname); <para> In a version-1 function, each actual argument is fetched using a <function>PG_GETARG_<replaceable>xxx</replaceable>()</function> - macro that corresponds to the argument's data type, and the - result is returned using a + macro that corresponds to the argument's data type. In non-strict + functions there needs to be a previous check about argument null-ness + using <function>PG_ARGNULL_<replaceable>xxx</replaceable>()</function>. + The result is returned using a <function>PG_RETURN_<replaceable>xxx</replaceable>()</function> macro for the return type. <function>PG_GETARG_<replaceable>xxx</replaceable>()</function> @@ -2328,7 +2173,7 @@ PG_FUNCTION_INFO_V1(funcname); </para> <para> - Here we show the same functions as above, coded in version-1 style: + Here are some examples using the version-1 calling convention: <programlisting><![CDATA[ #include "postgres.h" @@ -2421,27 +2266,67 @@ concat_text(PG_FUNCTION_ARGS) } ]]> </programlisting> + + <para> + Supposing that the above code has been prepared in file + <filename>funcs.c</filename> and compiled into a shared object, + we could define the functions to <productname>PostgreSQL</productname> + with commands like this: + +<programlisting> +CREATE FUNCTION add_one(integer) RETURNS integer + AS '<replaceable>DIRECTORY</replaceable>/funcs', 'add_one' + LANGUAGE C STRICT; + +-- note overloading of SQL function name "add_one" +CREATE FUNCTION add_one(double precision) RETURNS double precision + AS '<replaceable>DIRECTORY</replaceable>/funcs', 'add_one_float8' + LANGUAGE C STRICT; + +CREATE FUNCTION makepoint(point, point) RETURNS point + AS '<replaceable>DIRECTORY</replaceable>/funcs', 'makepoint' + LANGUAGE C STRICT; + +CREATE FUNCTION copytext(text) RETURNS text + AS '<replaceable>DIRECTORY</replaceable>/funcs', 'copytext' + LANGUAGE C STRICT; + +CREATE FUNCTION concat_text(text, text) RETURNS text + AS '<replaceable>DIRECTORY</replaceable>/funcs', 'concat_text' + LANGUAGE C STRICT; +</programlisting> + + <para> + Here, <replaceable>DIRECTORY</replaceable> stands for the + directory of the shared library file (for instance the + <productname>PostgreSQL</productname> tutorial directory, which + contains the code for the examples used in this section). + (Better style would be to use just <literal>'funcs'</> in the + <literal>AS</> clause, after having added + <replaceable>DIRECTORY</replaceable> to the search path. In any + case, we can omit the system-specific extension for a shared + library, commonly <literal>.so</literal>.) </para> <para> - The <command>CREATE FUNCTION</command> commands are the same as - for the version-0 equivalents. + Notice that we have specified the functions as <quote>strict</quote>, + meaning that + the system should automatically assume a null result if any input + value is null. By doing this, we avoid having to check for null inputs + in the function code. Without this, we'd have to check for null values + explicitly, using PG_ARGISNULL(). </para> <para> - At first glance, the version-1 coding conventions might appear to - be just pointless obscurantism. They do, however, offer a number - of improvements, because the macros can hide unnecessary detail. - An example is that in coding <function>add_one_float8</>, we no longer need to - be aware that <type>float8</type> is a pass-by-reference type. Another - example is that the <literal>GETARG</> macros for variable-length types allow - for more efficient fetching of <quote>toasted</quote> (compressed or + At first glance, the version-1 coding conventions might appear to be just + pointless obscurantism, over using plain <literal>C</> calling + conventions. They do however allow to deal with <literal>NULL</>able + arguments/return values, and <quote>toasted</quote> (compressed or out-of-line) values. </para> <para> - One big improvement in version-1 functions is better handling of null - inputs and results. The macro <function>PG_ARGISNULL(<replaceable>n</>)</function> + The macro <function>PG_ARGISNULL(<replaceable>n</>)</function> allows a function to test whether each input is null. (Of course, doing this is only necessary in functions not declared <quote>strict</>.) As with the @@ -2455,7 +2340,7 @@ concat_text(PG_FUNCTION_ARGS) </para> <para> - Other options provided in the new-style interface are two + Other options provided by the version-1 interface are two variants of the <function>PG_GETARG_<replaceable>xxx</replaceable>()</function> macros. The first of these, @@ -2487,9 +2372,7 @@ concat_text(PG_FUNCTION_ARGS) to return set results (<xref linkend="xfunc-c-return-set">) and implement trigger functions (<xref linkend="triggers">) and procedural-language call handlers (<xref - linkend="plhandler">). Version-1 code is also more - portable than version-0, because it does not break restrictions - on function call protocol in the C standard. For more details + linkend="plhandler">). For more details see <filename>src/backend/utils/fmgr/README</filename> in the source distribution. </para> @@ -2624,7 +2507,7 @@ SELECT name, c_overpaid(emp, 1500) AS overpaid WHERE name = 'Bill' OR name = 'Sam'; </programlisting> - Using call conventions version 0, we can define + Using the version-1 calling conventions, we can define <function>c_overpaid</> as: <programlisting><![CDATA[ @@ -2635,31 +2518,6 @@ SELECT name, c_overpaid(emp, 1500) AS overpaid PG_MODULE_MAGIC; #endif -bool -c_overpaid(HeapTupleHeader t, /* the current row of emp */ - int32 limit) -{ - bool isnull; - int32 salary; - - salary = DatumGetInt32(GetAttributeByName(t, "salary", &isnull)); - if (isnull) - return false; - return salary > limit; -} -]]> -</programlisting> - - In version-1 coding, the above would look like this: - -<programlisting><![CDATA[ -#include "postgres.h" -#include "executor/executor.h" /* for GetAttributeByName() */ - -#ifdef PG_MODULE_MAGIC -PG_MODULE_MAGIC; -#endif - PG_FUNCTION_INFO_V1(c_overpaid); Datum diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index 3976496aef..7d95fb7cdb 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -37,37 +37,6 @@ PGDLLIMPORT needs_fmgr_hook_type needs_fmgr_hook = NULL; PGDLLIMPORT fmgr_hook_type fmgr_hook = NULL; /* - * Declaration for old-style function pointer type. This is now used only - * in fmgr_oldstyle() and is no longer exported. - * - * The m68k SVR4 ABI defines that pointers are returned in %a0 instead of - * %d0. So if a function pointer is declared to return a pointer, the - * compiler may look only into %a0, but if the called function was declared - * to return an integer type, it puts its value only into %d0. So the - * caller doesn't pick up the correct return value. The solution is to - * declare the function pointer to return int, so the compiler picks up the - * return value from %d0. (Functions returning pointers put their value - * *additionally* into %d0 for compatibility.) The price is that there are - * some warnings about int->pointer conversions ... which we can suppress - * with suitably ugly casts in fmgr_oldstyle(). - */ -#if (defined(__mc68000__) || (defined(__m68k__))) && defined(__ELF__) -typedef int32 (*func_ptr) (); -#else -typedef char *(*func_ptr) (); -#endif - -/* - * For an oldstyle function, fn_extra points to a record like this: - */ -typedef struct -{ - func_ptr func; /* Address of the oldstyle function */ - bool arg_toastable[FUNC_MAX_ARGS]; /* is n'th arg of a toastable - * datatype? */ -} Oldstyle_fnextra; - -/* * Hashtable for fast lookup of external C functions */ typedef struct @@ -90,7 +59,6 @@ static void fmgr_info_other_lang(Oid functionId, FmgrInfo *finfo, HeapTuple proc static CFuncHashTabEntry *lookup_C_func(HeapTuple procedureTuple); static void record_C_func(HeapTuple procedureTuple, PGFunction user_fn, const Pg_finfo_record *inforec); -static Datum fmgr_oldstyle(PG_FUNCTION_ARGS); static Datum fmgr_security_definer(PG_FUNCTION_ARGS); @@ -304,13 +272,10 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt, static void fmgr_info_C_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedureTuple) { - Form_pg_proc procedureStruct = (Form_pg_proc) GETSTRUCT(procedureTuple); CFuncHashTabEntry *hashentry; PGFunction user_fn; const Pg_finfo_record *inforec; - Oldstyle_fnextra *fnextra; bool isnull; - int i; /* * See if we have the function address cached already @@ -362,20 +327,6 @@ fmgr_info_C_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedureTuple) switch (inforec->api_version) { - case 0: - /* Old style: need to use a handler */ - finfo->fn_addr = fmgr_oldstyle; - fnextra = (Oldstyle_fnextra *) - MemoryContextAllocZero(finfo->fn_mcxt, - sizeof(Oldstyle_fnextra)); - finfo->fn_extra = (void *) fnextra; - fnextra->func = (func_ptr) user_fn; - for (i = 0; i < procedureStruct->pronargs; i++) - { - fnextra->arg_toastable[i] = - TypeIsToastable(procedureStruct->proargtypes.values[i]); - } - break; case 1: /* New style: call directly */ finfo->fn_addr = user_fn; @@ -415,14 +366,6 @@ fmgr_info_other_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedureTuple) CurrentMemoryContext, true); finfo->fn_addr = plfinfo.fn_addr; - /* - * If lookup of the PL handler function produced nonnull fn_extra, - * complain --- it must be an oldstyle function! We no longer support - * oldstyle PL handlers. - */ - if (plfinfo.fn_extra != NULL) - elog(ERROR, "language %u has old-style handler", language); - ReleaseSysCache(languageTuple); } @@ -431,10 +374,7 @@ fmgr_info_other_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedureTuple) * The function is specified by a handle for the containing library * (obtained from load_external_function) as well as the function name. * - * If no info function exists for the given name, it is not an error. - * Instead we return a default info record for a version-0 function. - * We want to raise an error here only if the info function returns - * something bogus. + * If no info function exists for the given name an error is raised. * * This function is broken out of fmgr_info_C_lang so that fmgr_c_validator * can validate the information record for a function not yet entered into @@ -446,7 +386,6 @@ fetch_finfo_record(void *filehandle, char *funcname) char *infofuncname; PGFInfoFunction infofunc; const Pg_finfo_record *inforec; - static Pg_finfo_record default_inforec = {0}; infofuncname = psprintf("pg_finfo_%s", funcname); @@ -455,9 +394,12 @@ fetch_finfo_record(void *filehandle, char *funcname) infofuncname); if (infofunc == NULL) { - /* Not found --- assume version 0 */ - pfree(infofuncname); - return &default_inforec; + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("could not find function information for function \"%s\"", + funcname), + errhint("SQL-callable functions need an accompanying PG_FUNCTION_INFO_V1(funcname)."))); + return NULL; /* silence compiler */ } /* Found, so call it */ @@ -468,7 +410,6 @@ fetch_finfo_record(void *filehandle, char *funcname) elog(ERROR, "null result from info function \"%s\"", infofuncname); switch (inforec->api_version) { - case 0: case 1: /* OK, no additional fields to validate */ break; @@ -585,18 +526,7 @@ fmgr_info_copy(FmgrInfo *dstinfo, FmgrInfo *srcinfo, { memcpy(dstinfo, srcinfo, sizeof(FmgrInfo)); dstinfo->fn_mcxt = destcxt; - if (dstinfo->fn_addr == fmgr_oldstyle) - { - /* For oldstyle functions we must copy fn_extra */ - Oldstyle_fnextra *fnextra; - - fnextra = (Oldstyle_fnextra *) - MemoryContextAlloc(destcxt, sizeof(Oldstyle_fnextra)); - memcpy(fnextra, srcinfo->fn_extra, sizeof(Oldstyle_fnextra)); - dstinfo->fn_extra = (void *) fnextra; - } - else - dstinfo->fn_extra = NULL; + dstinfo->fn_extra = NULL; } @@ -617,245 +547,6 @@ fmgr_internal_function(const char *proname) /* - * Handler for old-style "C" language functions - */ -static Datum -fmgr_oldstyle(PG_FUNCTION_ARGS) -{ - Oldstyle_fnextra *fnextra; - int n_arguments = fcinfo->nargs; - int i; - bool isnull; - func_ptr user_fn; - char *returnValue; - - if (fcinfo->flinfo == NULL || fcinfo->flinfo->fn_extra == NULL) - elog(ERROR, "fmgr_oldstyle received NULL pointer"); - fnextra = (Oldstyle_fnextra *) fcinfo->flinfo->fn_extra; - - /* - * Result is NULL if any argument is NULL, but we still call the function - * (peculiar, but that's the way it worked before, and after all this is a - * backwards-compatibility wrapper). Note, however, that we'll never get - * here with NULL arguments if the function is marked strict. - * - * We also need to detoast any TOAST-ed inputs, since it's unlikely that - * an old-style function knows about TOASTing. - */ - isnull = false; - for (i = 0; i < n_arguments; i++) - { - if (PG_ARGISNULL(i)) - isnull = true; - else if (fnextra->arg_toastable[i]) - fcinfo->arg[i] = PointerGetDatum(PG_DETOAST_DATUM(fcinfo->arg[i])); - } - fcinfo->isnull = isnull; - - user_fn = fnextra->func; - - switch (n_arguments) - { - case 0: - returnValue = (char *) (*user_fn) (); - break; - case 1: - - /* - * nullvalue() used to use isNull to check if arg is NULL; perhaps - * there are other functions still out there that also rely on - * this undocumented hack? - */ - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - &fcinfo->isnull); - break; - case 2: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1]); - break; - case 3: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2]); - break; - case 4: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3]); - break; - case 5: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4]); - break; - case 6: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5]); - break; - case 7: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6]); - break; - case 8: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7]); - break; - case 9: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8]); - break; - case 10: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8], - fcinfo->arg[9]); - break; - case 11: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8], - fcinfo->arg[9], - fcinfo->arg[10]); - break; - case 12: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8], - fcinfo->arg[9], - fcinfo->arg[10], - fcinfo->arg[11]); - break; - case 13: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8], - fcinfo->arg[9], - fcinfo->arg[10], - fcinfo->arg[11], - fcinfo->arg[12]); - break; - case 14: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8], - fcinfo->arg[9], - fcinfo->arg[10], - fcinfo->arg[11], - fcinfo->arg[12], - fcinfo->arg[13]); - break; - case 15: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8], - fcinfo->arg[9], - fcinfo->arg[10], - fcinfo->arg[11], - fcinfo->arg[12], - fcinfo->arg[13], - fcinfo->arg[14]); - break; - case 16: - returnValue = (char *) (*user_fn) (fcinfo->arg[0], - fcinfo->arg[1], - fcinfo->arg[2], - fcinfo->arg[3], - fcinfo->arg[4], - fcinfo->arg[5], - fcinfo->arg[6], - fcinfo->arg[7], - fcinfo->arg[8], - fcinfo->arg[9], - fcinfo->arg[10], - fcinfo->arg[11], - fcinfo->arg[12], - fcinfo->arg[13], - fcinfo->arg[14], - fcinfo->arg[15]); - break; - default: - - /* - * Increasing FUNC_MAX_ARGS doesn't automatically add cases to the - * above code, so mention the actual value in this error not - * FUNC_MAX_ARGS. You could add cases to the above if you needed - * to support old-style functions with many arguments, but making - * 'em be new-style is probably a better idea. - */ - ereport(ERROR, - (errcode(ERRCODE_TOO_MANY_ARGUMENTS), - errmsg("function %u has too many arguments (%d, maximum is %d)", - fcinfo->flinfo->fn_oid, n_arguments, 16))); - returnValue = NULL; /* keep compiler quiet */ - break; - } - - return PointerGetDatum(returnValue); -} - - -/* * Support for security-definer and proconfig-using functions. We support * both of these features using the same call handler, because they are * often used together and it would be inefficient (as well as notationally @@ -2031,58 +1722,6 @@ OidSendFunctionCall(Oid functionId, Datum val) } -/* - * !!! OLD INTERFACE !!! - * - * fmgr() is the only remaining vestige of the old-style caller support - * functions. It's no longer used anywhere in the Postgres distribution, - * but we should leave it around for a release or two to ease the transition - * for user-supplied C functions. OidFunctionCallN() replaces it for new - * code. - * - * DEPRECATED, DO NOT USE IN NEW CODE - */ -char * -fmgr(Oid procedureId,...) -{ - FmgrInfo flinfo; - FunctionCallInfoData fcinfo; - int n_arguments; - Datum result; - - fmgr_info(procedureId, &flinfo); - - MemSet(&fcinfo, 0, sizeof(fcinfo)); - fcinfo.flinfo = &flinfo; - fcinfo.nargs = flinfo.fn_nargs; - n_arguments = fcinfo.nargs; - - if (n_arguments > 0) - { - va_list pvar; - int i; - - if (n_arguments > FUNC_MAX_ARGS) - ereport(ERROR, - (errcode(ERRCODE_TOO_MANY_ARGUMENTS), - errmsg("function %u has too many arguments (%d, maximum is %d)", - flinfo.fn_oid, n_arguments, FUNC_MAX_ARGS))); - va_start(pvar, procedureId); - for (i = 0; i < n_arguments; i++) - fcinfo.arg[i] = PointerGetDatum(va_arg(pvar, char *)); - va_end(pvar); - } - - result = FunctionCallInvoke(&fcinfo); - - /* Check for null result, since caller is clearly not expecting one */ - if (fcinfo.isnull) - elog(ERROR, "function %u returned NULL", flinfo.fn_oid); - - return DatumGetPointer(result); -} - - /*------------------------------------------------------------------------- * Support routines for standard maybe-pass-by-reference datatypes * diff --git a/src/include/fmgr.h b/src/include/fmgr.h index a671480004..87b23f9731 100644 --- a/src/include/fmgr.h +++ b/src/include/fmgr.h @@ -320,10 +320,10 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena * datum); /*------------------------------------------------------------------------- * Support for detecting call convention of dynamically-loaded functions * - * Dynamically loaded functions may use either the version-1 ("new style") - * or version-0 ("old style") calling convention. Version 1 is the call - * convention defined in this header file; version 0 is the old "plain C" - * convention. A version-1 function must be accompanied by the macro call + * Dynamically loaded functions currently can only use the version-1 ("new + * style") calling convention. Version-0 ("old style") is not supported + * anymore. Version 1 is the call convention defined in this header file, and + * must be accompanied by the macro call * * PG_FUNCTION_INFO_V1(function_name); * diff --git a/src/test/regress/input/create_function_2.source b/src/test/regress/input/create_function_2.source index 3c26b2fec6..b167c8ac6d 100644 --- a/src/test/regress/input/create_function_2.source +++ b/src/test/regress/input/create_function_2.source @@ -87,11 +87,6 @@ CREATE FUNCTION reverse_name(name) AS '@libdir@/regress@DLSUFFIX@' LANGUAGE C STRICT; -CREATE FUNCTION oldstyle_length(int4, text) - RETURNS int4 - AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C; -- intentionally not strict - -- -- Function dynamic loading -- diff --git a/src/test/regress/input/misc.source b/src/test/regress/input/misc.source index dd2d1b2033..b1dbc573c9 100644 --- a/src/test/regress/input/misc.source +++ b/src/test/regress/input/misc.source @@ -250,19 +250,6 @@ SELECT *, name(equipment(h.*)) FROM hobbies_r h; SELECT *, (equipment(CAST((h.*) AS hobbies_r))).name FROM hobbies_r h; -- --- check that old-style C functions work properly with TOASTed values --- -create table oldstyle_test(i int4, t text); -insert into oldstyle_test values(null,null); -insert into oldstyle_test values(0,'12'); -insert into oldstyle_test values(1000,'12'); -insert into oldstyle_test values(0, repeat('x', 50000)); - -select i, length(t), octet_length(t), oldstyle_length(i,t) from oldstyle_test; - -drop table oldstyle_test; - --- -- functional joins -- diff --git a/src/test/regress/output/create_function_2.source b/src/test/regress/output/create_function_2.source index bdd1b1bec5..8f28bff298 100644 --- a/src/test/regress/output/create_function_2.source +++ b/src/test/regress/output/create_function_2.source @@ -67,10 +67,6 @@ CREATE FUNCTION reverse_name(name) RETURNS name AS '@libdir@/regress@DLSUFFIX@' LANGUAGE C STRICT; -CREATE FUNCTION oldstyle_length(int4, text) - RETURNS int4 - AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C; -- intentionally not strict -- -- Function dynamic loading -- diff --git a/src/test/regress/output/misc.source b/src/test/regress/output/misc.source index 574ef0d2e3..b9595cc239 100644 --- a/src/test/regress/output/misc.source +++ b/src/test/regress/output/misc.source @@ -682,24 +682,6 @@ SELECT *, (equipment(CAST((h.*) AS hobbies_r))).name FROM hobbies_r h; (7 rows) -- --- check that old-style C functions work properly with TOASTed values --- -create table oldstyle_test(i int4, t text); -insert into oldstyle_test values(null,null); -insert into oldstyle_test values(0,'12'); -insert into oldstyle_test values(1000,'12'); -insert into oldstyle_test values(0, repeat('x', 50000)); -select i, length(t), octet_length(t), oldstyle_length(i,t) from oldstyle_test; - i | length | octet_length | oldstyle_length -------+--------+--------------+----------------- - | | | - 0 | 2 | 2 | 2 - 1000 | 2 | 2 | 1002 - 0 | 50000 | 50000 | 50000 -(4 rows) - -drop table oldstyle_test; --- -- functional joins -- -- diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c index 986d54ce2f..da02696b93 100644 --- a/src/test/regress/regress.c +++ b/src/test/regress/regress.c @@ -45,8 +45,6 @@ extern PATH *poly2path(POLYGON *poly); extern void regress_lseg_construct(LSEG *lseg, Point *pt1, Point *pt2); -extern char *reverse_name(char *string); -extern int oldstyle_length(int n, text *t); #ifdef PG_MODULE_MAGIC PG_MODULE_MAGIC; @@ -240,14 +238,15 @@ typedef struct double radius; } WIDGET; -WIDGET *widget_in(char *str); -char *widget_out(WIDGET *widget); +PG_FUNCTION_INFO_V1(widget_in); +PG_FUNCTION_INFO_V1(widget_out); #define NARGS 3 -WIDGET * -widget_in(char *str) +Datum +widget_in(PG_FUNCTION_ARGS) { + char *str = PG_GETARG_CSTRING(0); char *p, *coord[NARGS]; int i; @@ -270,14 +269,16 @@ widget_in(char *str) result->center.y = atof(coord[1]); result->radius = atof(coord[2]); - return result; + PG_RETURN_DATUM(PointerGetDatum(result)); } -char * -widget_out(WIDGET *widget) +Datum +widget_out(PG_FUNCTION_ARGS) { - return psprintf("(%g,%g,%g)", - widget->center.x, widget->center.y, widget->radius); + WIDGET *widget = (WIDGET *) PG_GETARG_POINTER(0); + char *str = psprintf("(%g,%g,%g)", + widget->center.x, widget->center.y, widget->radius); + PG_RETURN_CSTRING(str); } PG_FUNCTION_INFO_V1(pt_in_widget); @@ -305,9 +306,12 @@ boxarea(PG_FUNCTION_ARGS) PG_RETURN_FLOAT8(width * height); } -char * -reverse_name(char *string) +PG_FUNCTION_INFO_V1(reverse_name); + +Datum +reverse_name(PG_FUNCTION_ARGS) { + char *string = PG_GETARG_CSTRING(0); int i; int len; char *new_string; @@ -320,22 +324,7 @@ reverse_name(char *string) len = i; for (; i >= 0; --i) new_string[len - i] = string[i]; - return new_string; -} - -/* - * This rather silly function is just to test that oldstyle functions - * work correctly on toast-able inputs. - */ -int -oldstyle_length(int n, text *t) -{ - int len = 0; - - if (t) - len = VARSIZE(t) - VARHDRSZ; - - return n + len; + PG_RETURN_CSTRING(new_string); } -- 2.11.0.22.g8d7a455.dirty
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
