Time to drop old-style (V0) functions?
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.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres, all,
* Andres Freund (andres@anarazel.de) wrote:
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 easilyI don't see any reason to keep them around. If seriously doubt anybody
is using them seriously in anything but error.
+100
Thanks!
Stephen
Andres Freund <andres@anarazel.de> writes:
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
If by the first point you mean "assume V1 when no info function is found",
I object to that. If you mean you want to require an info function, that
might be OK.
The habit of zero-initializing Datums has got exactly nothing to do with
V0 functions; it's about ensuring consistent results and avoiding
heisenbugs from use of uninitialized memory. I do not think we should
drop it.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-08 17:38:38 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
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 easilyIf by the first point you mean "assume V1 when no info function is found",
I object to that. If you mean you want to require an info function, that
might be OK.
I mean throwing an error. Silently assuming V1 seems like a horrible
idea to me. It doesn't seem unlikely that we want to introduce a new
call interface at some point given the runtime cost of the current one,
and that'd just bring back the current problem.
The habit of zero-initializing Datums has got exactly nothing to do with
V0 functions; it's about ensuring consistent results and avoiding
heisenbugs from use of uninitialized memory. I do not think we should
drop it.
Well, V0 functions don't have a real way to get information about NULL,
and we allow non-strict V0 functions, so?
Regards,
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-08 14:53:58 -0800, Andres Freund wrote:
On 2016-12-08 17:38:38 -0500, Tom Lane wrote:
The habit of zero-initializing Datums has got exactly nothing to do with
V0 functions; it's about ensuring consistent results and avoiding
heisenbugs from use of uninitialized memory. I do not think we should
drop it.Well, V0 functions don't have a real way to get information about NULL,
and we allow non-strict V0 functions, so?
Oh, and the regression tests fail in V0 functions if you drop that.
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
On 2016-12-08 17:38:38 -0500, Tom Lane wrote:
The habit of zero-initializing Datums has got exactly nothing to do with
V0 functions; it's about ensuring consistent results and avoiding
heisenbugs from use of uninitialized memory. I do not think we should
drop it.
Well, V0 functions don't have a real way to get information about NULL,
and we allow non-strict V0 functions, so?
Non-strict V0 functions are pretty fundamentally broken, although IIRC
there was some hack whereby they could see the isnull marker for their
first argument, which is why we didn't just disallow the case. There was
never any expectation that checking for value == 0 was an appropriate
coding method for detecting nulls, because it couldn't work for
pass-by-value data types.
Again, the point of initializing those values is not to support broken
tests for nullness. It's to ensure consistent behavior in case of
buggy attempts to use null values. It's much like the fact that makeNode
zero-fills new node structs: that's mostly wasted work, if you want to
look at it in a certain way, but it's good for reproducibility.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-08 18:03:04 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2016-12-08 17:38:38 -0500, Tom Lane wrote:
The habit of zero-initializing Datums has got exactly nothing to do with
V0 functions; it's about ensuring consistent results and avoiding
heisenbugs from use of uninitialized memory. I do not think we should
drop it.Well, V0 functions don't have a real way to get information about NULL,
and we allow non-strict V0 functions, so?Non-strict V0 functions are pretty fundamentally broken, although IIRC
there was some hack whereby they could see the isnull marker for their
first argument, which is why we didn't just disallow the case. There was
never any expectation that checking for value == 0 was an appropriate
coding method for detecting nulls, because it couldn't work for
pass-by-value data types.
Well, we have a bunch in our regression tests ;). And I'm not saying
it's *good* that they rely on that, I think it's a reason to drop the
whole V0 interface.
(I also suspect there's a bunch in brin related to this)
Again, the point of initializing those values is not to support broken
tests for nullness. It's to ensure consistent behavior in case of
buggy attempts to use null values.
Well, it also makes such attempts undetectable. I'm not really convinced
that that's such an improvement.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Dec 8, 2016 at 5:53 PM, Andres Freund <andres@anarazel.de> wrote:
I mean throwing an error. Silently assuming V1 seems like a horrible
idea to me. It doesn't seem unlikely that we want to introduce a new
call interface at some point given the runtime cost of the current one,
and that'd just bring back the current problem.
I don't have a problem with getting rid of V0; I think it's an
annoyance. But I think a much more interesting question is how to
redesign this interface. I think V0 actually had the right idea in
passing arguments as C arguments rather than marshaling them in some
other data structure. If the function doesn't need flinfo or context
or collation and takes a fixed number of argument each of which is
either strict or a pass-by-reference data type (where you can use
DatumGetPointer(NULL) to represent a NULL!) and either never returns
NULL or returns a pass-by-reference data type, then you don't need any
of this complicated fcinfo stuff. You can just pass the arguments and
get back the result. That's less work for both the caller (who
doesn't have to stick the arguments into an fcinfo) and the callee
(who doesn't have to fish them back out). In the sortsupport
machinery, we've gone to some lengths to make it possible - at least
in the context of sorts - to get back to something closer to that kind
of interface. But that interface goes to considerable trouble to
achieve that result, making it practical only for bulk operations.
It's kind of ironic, at least IMHO, that the DirectionFunctionCall
macros are anything but direct. Each one is a non-inlined function
call that does a minimum of 8 variable assignments before actually
calling the function.
There obviously has to be some provision for functions that actually
need all the stuff we pass in the V1 calling convention, but there are
a huge number of functions that don't need any of it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 12/9/16 7:52 AM, Robert Haas wrote:
It's kind of ironic, at least IMHO, that the DirectionFunctionCall
macros are anything but direct. Each one is a non-inlined function
call that does a minimum of 8 variable assignments before actually
calling the function.
If this is a problem (it might be), then we can just make those calls,
er, direct C function calls to different function. For example,
result = DatumGetObjectId(DirectFunctionCall1(oidin,
CStringGetDatum(pro_name_or_oid)));
could just be
result = oidin_internal(pro_name_or_oid);
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Dec 19, 2016 at 3:13 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 12/9/16 7:52 AM, Robert Haas wrote:
It's kind of ironic, at least IMHO, that the DirectionFunctionCall
macros are anything but direct. Each one is a non-inlined function
call that does a minimum of 8 variable assignments before actually
calling the function.If this is a problem (it might be), then we can just make those calls,
er, direct C function calls to different function. For example,result = DatumGetObjectId(DirectFunctionCall1(oidin,
CStringGetDatum(pro_name_or_oid)));could just be
result = oidin_internal(pro_name_or_oid);
Yeah, true -- that works for simple cases, and might be beneficial
where you're doing lots and lots of calls in a tight loop.
For the more general problem, I wonder if we should try to figure out
something where we have one calling convention for "simple" functions
(those that little or none of the information in fcinfo and can pretty
much just take their SQL args as C args) and another for "complex"
functions (where we do the full push-ups).
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-19 15:25:42 -0500, Robert Haas wrote:
On Mon, Dec 19, 2016 at 3:13 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 12/9/16 7:52 AM, Robert Haas wrote:
It's kind of ironic, at least IMHO, that the DirectionFunctionCall
macros are anything but direct. Each one is a non-inlined function
call that does a minimum of 8 variable assignments before actually
calling the function.If this is a problem (it might be), then we can just make those calls,
er, direct C function calls to different function. For example,result = DatumGetObjectId(DirectFunctionCall1(oidin,
CStringGetDatum(pro_name_or_oid)));could just be
result = oidin_internal(pro_name_or_oid);
Yeah, true -- that works for simple cases, and might be beneficial
where you're doing lots and lots of calls in a tight loop.For the more general problem, I wonder if we should try to figure out
something where we have one calling convention for "simple" functions
(those that little or none of the information in fcinfo and can pretty
much just take their SQL args as C args) and another for "complex"
functions (where we do the full push-ups).
Unfortunately I think so would likely also increase overhead in a bunch
of places, because suddenly we need branches determining which callling
convention is in use. There's a fair amount of places, some of them
performance sensitive, that deal with functions that can get different
number of arguments. Prominently nodeAgg.c's transition functions for
example.
When JITing expressions that doesn't matter, because we avoid doing so
repetitively. But that's not really sufficient imo, non JITed
performance matters a lot too.
There's imo primarily two parts that make our calling convention
expensive:
a) Additional instructions required to push to/from fcinfo->arg[null],
including stack spilling required to have space for the arguments.
b) Increased number of cachelines touched, even for even trivial
functions. We need to read/write to at least five:
1) fcinfo->isnull to reset it
2) fcinfo->arg[0...] to set the argument
3) fcinfo->argnull[0...] to set argnull (separate cacheline)
4) fcinfo->flinfo->fn_addr to get the actual address to call
5) instruction cache miss for the function's contents
We can doctor around this some. A retively easy way to reduce the
overhead of a similar function call interface would be to restructure
things so we have something like:
typedef struct FunctionArg2
{
Datum arg;
bool argnull;
} FunctionArg2;
typedef struct FunctionCallInfoData2
{
/* cached function address from ->flinfo */
PGFunction fn_addr;
/* ptr to lookup info used for this call */
FmgrInfo *flinfo;
/* function must set true if result is NULL */
bool isnull;
/* # arguments actually passed */
short nargs;
/* collation for function to use */
Oid fncollation; /* collation for function to use */
/* pass or return extra info about result */
fmNodePtr resultinfo;
/* array big enough to fit flinfo->fn_nargs */
FunctionArg2 args[FLEXIBLE_ARRAY_MEMBER];
} FunctionCallInfoData2;
That'd mean some code changes because accessing arguments can't be done
quite as now, but it'd be fairly minor. We'd end up with a lot fewer
cache misses for common cases, because there's no need to fetch fn_addr,
and because the first two arg/argnull pairs still fit within the first
cacheline (which they never can in the current interface!).
But we'd still be passing arguments via memory, instead of using
registers.
I think a more efficient variant would make the function signature look
something like:
Datum /* directly returned argument */
pgfunc(
/* extra information about function call */
FunctionCallInfo *fcinfo,
/* bitmap of NULL arguments */
uint64_t nulls,
/* first argument */
Datum arg0,
/* second argument */
Datum arg1,
/* returned NULL */
bool *isnull
);
with additional arguments passed via fcinfo as currently done. Since
most of the performance critical function calls only have two arguments
that should allow to keep usage of fcinfo-> to the cases where we need
the extra information. On 64bit linuxes the above will be entirely in
registers, on 64bit windows isnull would be placed on the stack.
I don't quite see how we could avoid stack usage at all for windows, any
of the above arguments seems important.
There'd be a need for some trickery to make PG_GETARG_DATUM() work
efficiently - afaics the argument numbers passed to PG_GETARG_*() are
always constants, so that shouldn't be too bad.
Regards,
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-12-20 9:11 GMT+01:00 Andres Freund <andres@anarazel.de>:
On 2016-12-19 15:25:42 -0500, Robert Haas wrote:
On Mon, Dec 19, 2016 at 3:13 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 12/9/16 7:52 AM, Robert Haas wrote:
It's kind of ironic, at least IMHO, that the DirectionFunctionCall
macros are anything but direct. Each one is a non-inlined function
call that does a minimum of 8 variable assignments before actually
calling the function.If this is a problem (it might be), then we can just make those calls,
er, direct C function calls to different function. For example,result = DatumGetObjectId(DirectFunctionCall1(oidin,
CStringGetDatum(pro_name_or_oid)));could just be
result = oidin_internal(pro_name_or_oid);
Yeah, true -- that works for simple cases, and might be beneficial
where you're doing lots and lots of calls in a tight loop.For the more general problem, I wonder if we should try to figure out
something where we have one calling convention for "simple" functions
(those that little or none of the information in fcinfo and can pretty
much just take their SQL args as C args) and another for "complex"
functions (where we do the full push-ups).Unfortunately I think so would likely also increase overhead in a bunch
of places, because suddenly we need branches determining which callling
convention is in use. There's a fair amount of places, some of them
performance sensitive, that deal with functions that can get different
number of arguments. Prominently nodeAgg.c's transition functions for
example.When JITing expressions that doesn't matter, because we avoid doing so
repetitively. But that's not really sufficient imo, non JITed
performance matters a lot too.There's imo primarily two parts that make our calling convention
expensive:
a) Additional instructions required to push to/from fcinfo->arg[null],
including stack spilling required to have space for the arguments.
b) Increased number of cachelines touched, even for even trivial
functions. We need to read/write to at least five:
1) fcinfo->isnull to reset it
2) fcinfo->arg[0...] to set the argument
3) fcinfo->argnull[0...] to set argnull (separate cacheline)
4) fcinfo->flinfo->fn_addr to get the actual address to call
5) instruction cache miss for the function's contentsWe can doctor around this some. A retively easy way to reduce the
overhead of a similar function call interface would be to restructure
things so we have something like:typedef struct FunctionArg2
{
Datum arg;
bool argnull;
} FunctionArg2;typedef struct FunctionCallInfoData2
{
/* cached function address from ->flinfo */
PGFunction fn_addr;
/* ptr to lookup info used for this call */
FmgrInfo *flinfo;
/* function must set true if result is NULL */
bool isnull;
/* # arguments actually passed */
short nargs;
/* collation for function to use */
Oid fncollation; /* collation for function to use */
/* pass or return extra info about result */
fmNodePtr resultinfo;
/* array big enough to fit flinfo->fn_nargs */
FunctionArg2 args[FLEXIBLE_ARRAY_MEMBER];
} FunctionCallInfoData2;That'd mean some code changes because accessing arguments can't be done
quite as now, but it'd be fairly minor. We'd end up with a lot fewer
cache misses for common cases, because there's no need to fetch fn_addr,
and because the first two arg/argnull pairs still fit within the first
cacheline (which they never can in the current interface!).But we'd still be passing arguments via memory, instead of using
registers.I think a more efficient variant would make the function signature look
something like:Datum /* directly returned argument */
pgfunc(
/* extra information about function call */
FunctionCallInfo *fcinfo,
/* bitmap of NULL arguments */
uint64_t nulls,
/* first argument */
Datum arg0,
/* second argument */
Datum arg1,
/* returned NULL */
bool *isnull
);with additional arguments passed via fcinfo as currently done. Since
most of the performance critical function calls only have two arguments
that should allow to keep usage of fcinfo-> to the cases where we need
the extra information. On 64bit linuxes the above will be entirely in
registers, on 64bit windows isnull would be placed on the stack.I don't quite see how we could avoid stack usage at all for windows, any
of the above arguments seems important.There'd be a need for some trickery to make PG_GETARG_DATUM() work
efficiently - afaics the argument numbers passed to PG_GETARG_*() are
always constants, so that shouldn't be too bad.
In this case some benchmark can be very important (and interesting). I am
not sure if faster function execution has significant benefit on Vulcano
like executor.
Regards
Pavel
Show quoted text
Regards,
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-20 09:59:43 +0100, Pavel Stehule wrote:
In this case some benchmark can be very important (and interesting). I am
not sure if faster function execution has significant benefit on Vulcano
like executor.
It's fairly to see function calls as significant overhead. In fact, I
moved things *away* from a pure Vulcano style executor, and the benefits
weren't huge, primarily due to expression evaluation overhead (of which
function call overhead is one part). After JITing of expressions, it
becomes even more noticeable, because the overhead of the expression
evaluation is reduced.
I don't quite see why a Vulcano style executor should make function call
overhead meaningless?
Regards,
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-20 01:14:10 -0800, Andres Freund wrote:
On 2016-12-20 09:59:43 +0100, Pavel Stehule wrote:
In this case some benchmark can be very important (and interesting). I am
not sure if faster function execution has significant benefit on Vulcano
like executor.It's fairly to see function calls as significant overhead. In fact, I
moved things *away* from a pure Vulcano style executor, and the benefits
weren't huge, primarily due to expression evaluation overhead (of which
function call overhead is one part). After JITing of expressions, it
becomes even more noticeable, because the overhead of the expression
evaluation is reduced.
As an example, here's a JITed TPCH Q1 profile:
+ 15.48% postgres postgres [.] slot_deform_tuple
+ 8.42% postgres perf-27760.map [.] evalexpr90
+ 5.98% postgres postgres [.] float8_accum
+ 4.63% postgres postgres [.] slot_getattr
+ 3.69% postgres postgres [.] bpchareq
+ 3.39% postgres postgres [.] heap_getnext
+ 3.22% postgres postgres [.] float8pl
+ 2.86% postgres postgres [.] TupleHashTableMatch.isra.7
+ 2.77% postgres postgres [.] hashbpchar
+ 2.77% postgres postgres [.] float8mul
+ 2.73% postgres postgres [.] ExecAgg
+ 2.40% postgres postgres [.] hash_any
+ 2.34% postgres postgres [.] MemoryContextReset
+ 1.98% postgres postgres [.] pg_detoast_datum_packed
evalexpr90 is the expression that does the aggregate transition
function. float8_accum, bpchareq, float8pl , float8mul, ... are all
function calls, and a good percentage of the overhead in evalexpr90 is
pushing arguments onto fcinfo->arg[nulls].
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-12-20 10:28 GMT+01:00 Andres Freund <andres@anarazel.de>:
On 2016-12-20 01:14:10 -0800, Andres Freund wrote:
On 2016-12-20 09:59:43 +0100, Pavel Stehule wrote:
In this case some benchmark can be very important (and interesting). I
am
not sure if faster function execution has significant benefit on
Vulcano
like executor.
It's fairly to see function calls as significant overhead. In fact, I
moved things *away* from a pure Vulcano style executor, and the benefits
weren't huge, primarily due to expression evaluation overhead (of which
function call overhead is one part). After JITing of expressions, it
becomes even more noticeable, because the overhead of the expression
evaluation is reduced.
For me a Vulcano style is row by row processing. Using JIT or not using has
not significant impact.
Interesting change can be block level processing.
As an example, here's a JITed TPCH Q1 profile: + 15.48% postgres postgres [.] slot_deform_tuple + 8.42% postgres perf-27760.map [.] evalexpr90 + 5.98% postgres postgres [.] float8_accum + 4.63% postgres postgres [.] slot_getattr + 3.69% postgres postgres [.] bpchareq + 3.39% postgres postgres [.] heap_getnext + 3.22% postgres postgres [.] float8pl + 2.86% postgres postgres [.] TupleHashTableMatch.isra.7 + 2.77% postgres postgres [.] hashbpchar + 2.77% postgres postgres [.] float8mul + 2.73% postgres postgres [.] ExecAgg + 2.40% postgres postgres [.] hash_any + 2.34% postgres postgres [.] MemoryContextReset + 1.98% postgres postgres [.] pg_detoast_datum_packed
Our bottle neck is row format and row related processing.
Show quoted text
evalexpr90 is the expression that does the aggregate transition
function. float8_accum, bpchareq, float8pl , float8mul, ... are all
function calls, and a good percentage of the overhead in evalexpr90 is
pushing arguments onto fcinfo->arg[nulls].Greetings,
Andres Freund
On 2016-12-20 10:44:35 +0100, Pavel Stehule wrote:
2016-12-20 10:28 GMT+01:00 Andres Freund <andres@anarazel.de>:
On 2016-12-20 01:14:10 -0800, Andres Freund wrote:
On 2016-12-20 09:59:43 +0100, Pavel Stehule wrote:
In this case some benchmark can be very important (and interesting). I
am
not sure if faster function execution has significant benefit on
Vulcano
like executor.
It's fairly to see function calls as significant overhead. In fact, I
moved things *away* from a pure Vulcano style executor, and the benefits
weren't huge, primarily due to expression evaluation overhead (of which
function call overhead is one part). After JITing of expressions, it
becomes even more noticeable, because the overhead of the expression
evaluation is reduced.For me a Vulcano style is row by row processing. Using JIT or not using has
not significant impact.Interesting change can be block level processing.
I don't think that's true. The largest bottlenecks atm have relatively
little to do with block level processing. I know, because I went
there. We have so many other bottlenecks that row-by-row processing
vanishes behind them. Without changing the tuple flow, the performance
with either applied or posted patches for TPCH-Q1 already went up by
more than a factor of 2.5.
Anyway, this seems like a side-track discussion, better done in another
thread.
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Dec 20, 2016 at 3:11 AM, Andres Freund <andres@anarazel.de> wrote:
I think a more efficient variant would make the function signature look
something like:Datum /* directly returned argument */
pgfunc(
/* extra information about function call */
FunctionCallInfo *fcinfo,
/* bitmap of NULL arguments */
uint64_t nulls,
/* first argument */
Datum arg0,
/* second argument */
Datum arg1,
/* returned NULL */
bool *isnull
);
Yeah, that's kind of nice. I like the uint64 for nulls, although
FUNC_MAX_ARGS > 64 by default and certainly can be configured that
way. It wouldn't be a problem for any common cases, of course.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-20 08:15:07 -0500, Robert Haas wrote:
On Tue, Dec 20, 2016 at 3:11 AM, Andres Freund <andres@anarazel.de> wrote:
I think a more efficient variant would make the function signature look
something like:Datum /* directly returned argument */
pgfunc(
/* extra information about function call */
FunctionCallInfo *fcinfo,
/* bitmap of NULL arguments */
uint64_t nulls,
/* first argument */
Datum arg0,
/* second argument */
Datum arg1,
/* returned NULL */
bool *isnull
);Yeah, that's kind of nice. I like the uint64 for nulls, although
FUNC_MAX_ARGS > 64 by default and certainly can be configured that
way. It wouldn't be a problem for any common cases, of course.
Well, I suspect we'd have to change that then. Is anybody seriously
interested in supporting FUNC_MAX_ARGS > 64? We don't have to make our
life hard by supporting useless features... I'd even question using
64bit for this on 32bit platforms.
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Dec 20, 2016 at 8:21 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-12-20 08:15:07 -0500, Robert Haas wrote:
On Tue, Dec 20, 2016 at 3:11 AM, Andres Freund <andres@anarazel.de> wrote:
I think a more efficient variant would make the function signature look
something like:Datum /* directly returned argument */
pgfunc(
/* extra information about function call */
FunctionCallInfo *fcinfo,
/* bitmap of NULL arguments */
uint64_t nulls,
/* first argument */
Datum arg0,
/* second argument */
Datum arg1,
/* returned NULL */
bool *isnull
);Yeah, that's kind of nice. I like the uint64 for nulls, although
FUNC_MAX_ARGS > 64 by default and certainly can be configured that
way. It wouldn't be a problem for any common cases, of course.Well, I suspect we'd have to change that then. Is anybody seriously
interested in supporting FUNC_MAX_ARGS > 64? We don't have to make our
life hard by supporting useless features... I'd even question using
64bit for this on 32bit platforms.
Advanced Server uses 256, and we had a customer complain recently that
256 wasn't high enough. So apparently the use case for functions with
ridiculous numbers of arguments isn't exactly 0. For most people it's
not a big problem in practice, but actually I think that it's pretty
lame that we have a limit at all. I mean, there's no reason that we
can't use dynamic allocation here. If we put the first, say, 8
arguments in the FunctionCallInfoData itself and then separately
allocated space any extras, the structure could be a lot smaller
without needing an arbitrary limit on the argument count.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-12-20 08:35:05 -0500, Robert Haas wrote:
On Tue, Dec 20, 2016 at 8:21 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-12-20 08:15:07 -0500, Robert Haas wrote:
On Tue, Dec 20, 2016 at 3:11 AM, Andres Freund <andres@anarazel.de> wrote:
I think a more efficient variant would make the function signature look
something like:Datum /* directly returned argument */
pgfunc(
/* extra information about function call */
FunctionCallInfo *fcinfo,
/* bitmap of NULL arguments */
uint64_t nulls,
/* first argument */
Datum arg0,
/* second argument */
Datum arg1,
/* returned NULL */
bool *isnull
);Yeah, that's kind of nice. I like the uint64 for nulls, although
FUNC_MAX_ARGS > 64 by default and certainly can be configured that
way. It wouldn't be a problem for any common cases, of course.Well, I suspect we'd have to change that then. Is anybody seriously
interested in supporting FUNC_MAX_ARGS > 64? We don't have to make our
life hard by supporting useless features... I'd even question using
64bit for this on 32bit platforms.Advanced Server uses 256, and we had a customer complain recently that
256 wasn't high enough. So apparently the use case for functions with
ridiculous numbers of arguments isn't exactly 0.
Well, there's a cost/benefit analysis involved here, as in a lot of
other places. There's a lot of things one can conceive a use-case for,
doesn't mean it's worth catering for all of them.
I mean, there's no reason that we can't use dynamic allocation here.
If we put the first, say, 8 arguments in the FunctionCallInfoData
itself and then separately allocated space any extras, the structure
could be a lot smaller without needing an arbitrary limit on the
argument count.
Except that that'll mean additional overhead during evaluation of
function arguments, an overhead that everyone will have to pay. Suddenly
you need two loops that fill in arguments, depending on their
argument-number (i.e. additional branches in a thight spot). And not
being able to pass the null bitmap via an register argument also costs
everyone.
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Dec 20, 2016 at 8:44 AM, Andres Freund <andres@anarazel.de> wrote:
Advanced Server uses 256, and we had a customer complain recently that
256 wasn't high enough. So apparently the use case for functions with
ridiculous numbers of arguments isn't exactly 0.Well, there's a cost/benefit analysis involved here, as in a lot of
other places. There's a lot of things one can conceive a use-case for,
doesn't mean it's worth catering for all of them.
Clearly true.
I mean, there's no reason that we can't use dynamic allocation here.
If we put the first, say, 8 arguments in the FunctionCallInfoData
itself and then separately allocated space any extras, the structure
could be a lot smaller without needing an arbitrary limit on the
argument count.Except that that'll mean additional overhead during evaluation of
function arguments, an overhead that everyone will have to pay. Suddenly
you need two loops that fill in arguments, depending on their
argument-number (i.e. additional branches in a thight spot). And not
being able to pass the null bitmap via an register argument also costs
everyone.
Well, I'm hoping there is a way to have a fast-path and a slow-path
without slowing things down too much. You're proposing something like
that anyway, because you're proposing to put the first two arguments
in actual function arguments and the rest someplace else. I wouldn't
be surprised if 99% of function calls had 2 arguments or less because
most people probably call functions mostly or entirely as operators
and even user-defined functions don't necessarily have lots of
arguments. But we wouldn't propose restricting all function calls to
2 arguments just so +(int4, int4) can be fast. There's clearly got to
be a slow path for calls with more than 2 arguments and, if that's the
case, I don't see why there can't be a really-slow path, perhaps
guarded by unlikely(), for cases involving more than 8 or 16 or
whatever. I find it really hard to believe that in 2016 that we can't
find a way to make simple things fast and complicated things possible.
Saying we can't make SQL-callable functions with large number of
arguments work feels to me like saying that we have to go back to
8-character filenames with 3-character extensions for efficiency - in
the 1980s, that argument might have held some water, but nobody buys
it any more. Human time is worth more than computer time, and humans
save time when programs don't impose inconveniently-small limits.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
Well, I'm hoping there is a way to have a fast-path and a slow-path
without slowing things down too much.
Seems like this discussion has veered way off into the weeds.
I suggest we confine it to the stated topic; if you want to discuss
ways to optimize V1 protocol (or invent a V2, which some of this
sounds like it would need), that should be a distinct thread.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 easilyI don't see any reason to keep them around. If seriously doubt anybody
is using them seriously in anything but error.
Patches attached.
Attachments:
0001-Move-contrib-seg-to-only-use-V1-calling-conventions.patchtext/x-patch; charset=us-asciiDownload
From dc6de0b47e1013c165c1026df65bda62cba8d8b7 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.k3mbno4twhg2qf7g@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
0002-Remove-support-for-version-0-calling-conventions.patchtext/x-patch; charset=us-asciiDownload
From 6a61db32b4d52fee02157d6a93593a8cceab9fff Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.k3mbno4twhg2qf7g@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
On 2017-02-28 23:15:15 -0800, Andres Freund wrote:
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 easilyI don't see any reason to keep them around. If seriously doubt anybody
is using them seriously in anything but error.Patches attached.
One unaddressed question in those patches is what we do with
src/backend/utils/fmgr/README - I'm not quite sure what its purpose is,
in its current state. If we want to keep it, we'd probably have to
pretty aggressively revise it?
- Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/1/17 02:22, Andres Freund wrote:
One unaddressed question in those patches is what we do with
src/backend/utils/fmgr/README - I'm not quite sure what its purpose is,
in its current state. If we want to keep it, we'd probably have to
pretty aggressively revise it?
Some of the information in there, such as the use of the FmgrInfo and
FunctionCallInfoData structs, doesn't seem to appear anywhere else in
that amount of detail, so it would be a loss to just delete the file, I
think. Perhaps just lightly editing out the "I propose to do this" tone
would work.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
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 easilyI don't see any reason to keep them around. If seriously doubt anybody
is using them seriously in anything but error.
I find these arguments pretty weak. In particular I don't buy your claim
that we could stop zero-initializing Datums, because I think we'd just
start getting valgrind complaints if we did. It's too common to copy both
a Datum and its isnull flag without any particular inquiry into whether
the datum is null.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017-03-01 11:18:34 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
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 easilyI don't see any reason to keep them around. If seriously doubt anybody
is using them seriously in anything but error.I find these arguments pretty weak. In particular I don't buy your claim
that we could stop zero-initializing Datums, because I think we'd just
start getting valgrind complaints if we did.
I tink we've litigated that in a side-thread - I'm not planning to change
any policy around this. Perhaps I shouldn't have quoted the original
start of the thread...
At this point I primarily am concerned about
a) the way they're confusing for authors, by causing spurious crashes
b) at some point not too far away in the future I want to introduce a
faster interface, and I don't want unnecessarily many around.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I think we have consensus to go ahead with this, and the patches are
mostly mechanical, so I only have a few comments on style and one
possible bug below:
0001-Move-contrib-seg-to-only-use-V1-calling-conventions.patch
static int restore(char *s, float val, int n);
-
/*****************************************************************************
* Input/Output functions
*****************************************************************************/
+
doesn't seem like the right whitespace change
@@ -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));
There would be a little bit less back-and-forth here if you kept
datum_l and datum_r of type SEG *.
case RTOverlapStrategyNumber:
- retval = (bool) seg_overlap(key, query);
+ retval =
+ !DatumGetBool(DirectFunctionCall2(seg_overlap, key, query));
break;
Accidentally flipped the logic?
-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)));
}
Maybe in seg_contained also assign the arguments to a and b, so it's
easier to see the relationship between contains and contained.
-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);
}
I would write this as
int32 cmp = DatumGetInt32(DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1));
Having a variable of type Datum is a bit meaningless.
Oh, I see you did it that way later in seg_lt() etc., so same here.
0002-Remove-support-for-version-0-calling-conventions.patch
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)
These changes are neither right nor wrong, but we have had previous
discussions about this and settled on leaving the whitespace as psql
outputs it. In any case it seems unrelated here.
<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>
extra blank line
@@ -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>
unrelated?
@@ -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 */
Perhaps plug in the actual C function name into the error message, like
errhint("SQL-callable functions need an accompanying PG_FUNCTION_INFO_V1(%s).", funcname)
@@ -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));
}
Could be PG_RETURN_POINTER().
Attached is a patch for src/backend/utils/fmgr/README that edits out the
transitional comments and just keeps the parts still relevant.
Tests pass for me. So this is mostly ready to go AFAICS.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
fmgr-README.patchtext/x-patch; name=fmgr-README.patchDownload
diff --git a/src/backend/utils/fmgr/README b/src/backend/utils/fmgr/README
index e7e7ae9c6e..5a2331ff15 100644
--- a/src/backend/utils/fmgr/README
+++ b/src/backend/utils/fmgr/README
@@ -3,62 +3,19 @@ src/backend/utils/fmgr/README
Function Manager
================
-Proposal For Function-Manager Redesign 19-Nov-2000
---------------------------------------
-
-We know that the existing mechanism for calling Postgres functions needs
-to be redesigned. It has portability problems because it makes
-assumptions about parameter passing that violate ANSI C; it fails to
-handle NULL arguments and results cleanly; and "function handlers" that
-support a class of functions (such as fmgr_pl) can only be done via a
-really ugly, non-reentrant kluge. (Global variable set during every
-function call, forsooth.) Here is a proposal for fixing these problems.
-
-In the past, the major objections to redoing the function-manager
-interface have been (a) it'll be quite tedious to implement, since every
-built-in function and everyplace that calls such functions will need to
-be touched; (b) such wide-ranging changes will be difficult to make in
-parallel with other development work; (c) it will break existing
-user-written loadable modules that define "C language" functions. While
-I have no solution to the "tedium" aspect, I believe I see an answer to
-the other problems: by use of function handlers, we can support both old
-and new interfaces in parallel for both callers and callees, at some
-small efficiency cost for the old styles. That way, most of the changes
-can be done on an incremental file-by-file basis --- we won't need a
-"big bang" where everything changes at once. Support for callees
-written in the old style can be left in place indefinitely, to provide
-backward compatibility for user-written C functions.
-
-
-Changes In pg_proc (System Data About a Function)
--------------------------------------------------
-
-A new column "proisstrict" will be added to the system pg_proc table.
-This is a boolean value which will be TRUE if the function is "strict",
-that is it always returns NULL when any of its inputs are NULL. The
-function manager will check this field and skip calling the function when
-it's TRUE and there are NULL inputs. This allows us to remove explicit
-NULL-value tests from many functions that currently need them (not to
-mention fixing many more that need them but don't have them). A function
-that is not marked "strict" is responsible for checking whether its inputs
-are NULL or not. Most builtin functions will be marked "strict".
-
-An optional WITH parameter will be added to CREATE FUNCTION to allow
-specification of whether user-defined functions are strict or not. I am
-inclined to make the default be "not strict", since that seems to be the
-more useful case for functions expressed in SQL or a PL language, but
-am open to arguments for the other choice.
-
-
-The New Function-Manager Interface
-----------------------------------
-
-The core of the new design is revised data structures for representing
-the result of a function lookup and for representing the parameters
-passed to a specific function invocation. (We want to keep function
-lookup separate from function call, since many parts of the system apply
-the same function over and over; the lookup overhead should be paid once
-per query, not once per tuple.)
+[This file originally explained the transition from the V0 to the V1
+interface. Now it just explains some internals and rationale for the V1
+interface, while the V0 interface has been removed.]
+
+The V1 Function-Manager Interface
+---------------------------------
+
+The core of the design is data structures for representing the result of a
+function lookup and for representing the parameters passed to a specific
+function invocation. (We want to keep function lookup separate from
+function call, since many parts of the system apply the same function over
+and over; the lookup overhead should be paid once per query, not once per
+tuple.)
When a function is looked up in pg_proc, the result is represented as
@@ -183,50 +140,6 @@ should have no portability or optimization problems.
Function Coding Conventions
---------------------------
-As an example, int4 addition goes from old-style
-
-int32
-int4pl(int32 arg1, int32 arg2)
-{
- return arg1 + arg2;
-}
-
-to new-style
-
-Datum
-int4pl(FunctionCallInfo fcinfo)
-{
- /* we assume the function is marked "strict", so we can ignore
- * NULL-value handling */
-
- return Int32GetDatum(DatumGetInt32(fcinfo->arg[0]) +
- DatumGetInt32(fcinfo->arg[1]));
-}
-
-This is, of course, much uglier than the old-style code, but we can
-improve matters with some well-chosen macros for the boilerplate parts.
-I propose below macros that would make the code look like
-
-Datum
-int4pl(PG_FUNCTION_ARGS)
-{
- int32 arg1 = PG_GETARG_INT32(0);
- int32 arg2 = PG_GETARG_INT32(1);
-
- PG_RETURN_INT32( arg1 + arg2 );
-}
-
-This is still more code than before, but it's fairly readable, and it's
-also amenable to machine processing --- for example, we could probably
-write a script that scans code like this and extracts argument and result
-type info for comparison to the pg_proc table.
-
-For the standard data types float4, float8, and int8, these macros should hide
-whether the types are pass-by-value or pass-by reference, by incorporating
-indirection and space allocation if needed. This will offer a considerable
-gain in readability, and it also opens up the opportunity to make these types
-be pass-by-value on machines where it's feasible to do so.
-
Here are the proposed macros and coding conventions:
The definition of an fmgr-callable function will always look like
@@ -291,67 +204,6 @@ fields of FunctionCallInfo, it should just do it. I doubt that providing
syntactic-sugar macros for these cases is useful.
-Call-Site Coding Conventions
-----------------------------
-
-There are many places in the system that call either a specific function
-(for example, the parser invokes "textin" by name in places) or a
-particular group of functions that have a common argument list (for
-example, the optimizer invokes selectivity estimation functions with
-a fixed argument list). These places will need to change, but we should
-try to avoid making them significantly uglier than before.
-
-Places that invoke an arbitrary function with an arbitrary argument list
-can simply be changed to fill a FunctionCallInfoData structure directly;
-that'll be no worse and possibly cleaner than what they do now.
-
-When invoking a specific built-in function by name, we have generally
-just written something like
- result = textin ( ... args ... )
-which will not work after textin() is converted to the new call style.
-I suggest that code like this be converted to use "helper" functions
-that will create and fill in a FunctionCallInfoData struct. For
-example, if textin is being called with one argument, it'd look
-something like
- result = DirectFunctionCall1(textin, PointerGetDatum(argument));
-These helper routines will have declarations like
- Datum DirectFunctionCall2(PGFunction func, Datum arg1, Datum arg2);
-Note it will be the caller's responsibility to convert to and from
-Datum; appropriate conversion macros should be used.
-
-The DirectFunctionCallN routines will not bother to fill in
-fcinfo->flinfo (indeed cannot, since they have no idea about an OID for
-the target function); they will just set it NULL. This is unlikely to
-bother any built-in function that could be called this way. Note also
-that this style of coding cannot pass a NULL input value nor cope with
-a NULL result (it couldn't before, either!). We can make the helper
-routines ereport an error if they see that the function returns a NULL.
-
-When invoking a function that has a known argument signature, we have
-usually written either
- result = fmgr(targetfuncOid, ... args ... );
-or
- result = fmgr_ptr(FmgrInfo *finfo, ... args ... );
-depending on whether an FmgrInfo lookup has been done yet or not.
-This kind of code can be recast using helper routines, in the same
-style as above:
- result = OidFunctionCall1(funcOid, PointerGetDatum(argument));
- result = FunctionCall2(funcCallInfo,
- PointerGetDatum(argument),
- Int32GetDatum(argument));
-Again, this style of coding does not allow for expressing NULL inputs
-or receiving a NULL result.
-
-As with the callee-side situation, I propose adding argument conversion
-macros that hide whether int8, float4, and float8 are pass-by-value or
-pass-by-reference.
-
-The existing helper functions fmgr(), fmgr_c(), etc will be left in
-place until all uses of them are gone. Of course their internals will
-have to change in the first step of implementation, but they can
-continue to support the same external appearance.
-
-
Support for TOAST-Able Data Types
---------------------------------
@@ -474,83 +326,3 @@ context. fn_mcxt normally points at the context that was
CurrentMemoryContext at the time the FmgrInfo structure was created;
in any case it is required to be a context at least as long-lived as the
FmgrInfo itself.
-
-
-Telling the Difference Between Old- and New-Style Functions
------------------------------------------------------------
-
-During the conversion process, we carried two different pg_language
-entries, "internal" and "newinternal", for internal functions. The
-function manager used the language code to distinguish which calling
-convention to use. (Old-style internal functions were supported via
-a function handler.) As of Nov. 2000, no old-style internal functions
-remain, so we can drop support for them. We will remove the old "internal"
-pg_language entry and rename "newinternal" to "internal".
-
-The interim solution for dynamically-loaded compiled functions has been
-similar: two pg_language entries "C" and "newC". This naming convention
-is not desirable for the long run, and yet we cannot stop supporting
-old-style user functions. Instead, it seems better to use just one
-pg_language entry "C", and require the dynamically-loaded library to
-provide additional information that identifies new-style functions.
-This avoids compatibility problems --- for example, existing dump
-scripts will identify PL language handlers as being in language "C",
-which would be wrong under the "newC" convention. Also, this approach
-should generalize more conveniently for future extensions to the function
-interface specification.
-
-Given a dynamically loaded function named "foo" (note that the name being
-considered here is the link-symbol name, not the SQL-level function name),
-the function manager will look for another function in the same dynamically
-loaded library named "pg_finfo_foo". If this second function does not
-exist, then foo is assumed to be called old-style, thus ensuring backwards
-compatibility with existing libraries. If the info function does exist,
-it is expected to have the signature
-
- Pg_finfo_record * pg_finfo_foo (void);
-
-The info function will be called by the fmgr, and must return a pointer
-to a Pg_finfo_record struct. (The returned struct will typically be a
-statically allocated constant in the dynamic-link library.) The current
-definition of the struct is just
-
- typedef struct {
- int api_version;
- } Pg_finfo_record;
-
-where api_version is 0 to indicate old-style or 1 to indicate new-style
-calling convention. In future releases, additional fields may be defined
-after api_version, but these additional fields will only be used if
-api_version is greater than 1.
-
-These details will be hidden from the author of a dynamically loaded
-function by using a macro. To define a new-style dynamically loaded
-function named foo, write
-
- PG_FUNCTION_INFO_V1(foo);
-
- Datum
- foo(PG_FUNCTION_ARGS)
- {
- ...
- }
-
-The function itself is written using the same conventions as for new-style
-internal functions; you just need to add the PG_FUNCTION_INFO_V1() macro.
-Note that old-style and new-style functions can be intermixed in the same
-library, depending on whether or not you write a PG_FUNCTION_INFO_V1() for
-each one.
-
-The SQL declaration for a dynamically-loaded function is CREATE FUNCTION
-foo ... LANGUAGE C regardless of whether it is old- or new-style.
-
-New-style dynamic functions will be invoked directly by fmgr, and will
-therefore have the same performance as internal functions after the initial
-pg_proc lookup overhead. Old-style dynamic functions will be invoked via
-a handler, and will therefore have a small performance penalty.
-
-To allow old-style dynamic functions to work safely on toastable datatypes,
-the handler for old-style functions will automatically detoast toastable
-arguments before passing them to the old-style function. A new-style
-function is expected to take care of toasted arguments by using the
-standard argument access macros defined above.
On 7 March 2017 at 22:50, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
I think we have consensus to go ahead with this, and the patches are
mostly mechanical, so I only have a few comments on style and one
possible bug below:0001-Move-contrib-seg-to-only-use-V1-calling-conventions.patch
static int restore(char *s, float val, int n);
-
/*****************************************************************************
* Input/Output functions
*****************************************************************************/+
doesn't seem like the right whitespace change
Fixed.
@@ -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));There would be a little bit less back-and-forth here if you kept
datum_l and datum_r of type SEG *.
Also, currently it does:
v->spl_ldatum = PointerGetDatum(datum_l);
v->spl_rdatum = PointerGetDatum(datum_r);
even though they're already Datum.
Downside of keeping them as SEG is we land up with:
seg_l = DatumGetPointer(DirectFunctionCall2(seg_union,
PointerGetDatum(datum_l),
PointerGetDatum(sort_items[i].data)));
but at least it's tied to the fmgr call.
case RTOverlapStrategyNumber: - retval = (bool) seg_overlap(key, query); + retval = + !DatumGetBool(DirectFunctionCall2(seg_overlap, key, query)); break;Accidentally flipped the logic?
Looks like it. I don't see a reason for it; there's no corresponding
change around seg_overlap and the other callsite isn't negated:
case RTOverlapStrategyNumber:
- retval = (bool) seg_overlap(key, query);
+ retval = DirectFunctionCall2(seg_overlap, key, query);
so I'd say copy-pasteo, given nearby negated bools.
Fixed. Didn't find any other cases.
-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))); }Maybe in seg_contained also assign the arguments to a and b, so it's
easier to see the relationship between contains and contained.
Done. Makes for nicer formatting too.
-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); }I would write this as
int32 cmp = DatumGetInt32(DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1));
Having a variable of type Datum is a bit meaningless.
If you're passing things around between other fmgr-using functions
it's often useful to just carry the Datum form around.
In this case it doesn't make much sense though. Done.
0002-Remove-support-for-version-0-calling-conventions.patch
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)These changes are neither right nor wrong, but we have had previous
discussions about this and settled on leaving the whitespace as psql
outputs it. In any case it seems unrelated here.
Removed. Though personally since the patch touches the file anyway it
doesn't seem to matter much either way.
+ + 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>extra blank line
Fixed.
@@ -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>unrelated?
Reverted. Though personally I'd like to be more forgiving of
unrelated-ish changes in docs, since they often need a tidy up when
you're touching the file anyway.
@@ -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 */
Perhaps plug in the actual C function name into the error message, like
errhint("SQL-callable functions need an accompanying PG_FUNCTION_INFO_V1(%s).", funcname)
Doesn't make sense unless we then make it singlular, IMO, otherwise it
just reads weirdly. I'd rather keep the 'funcname'. If you're writing
C code it shouldn't be too much of an ask.
@@ -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)); }Could be PG_RETURN_POINTER().
Done.
Attached is a patch for src/backend/utils/fmgr/README that edits out the
transitional comments and just keeps the parts still relevant.
Applied.
Attached with the suggested amendments. I'll have a read-through, but
you seem to have done the fine-tooth comb treatment already.
Passes "make check" and recovery tests, check-world running now.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Move-contrib-seg-to-only-use-V1-calling-conventions.patchtext/x-patch; charset=US-ASCII; name=0001-Move-contrib-seg-to-only-use-V1-calling-conventions.patchDownload
From c3449f2b5ce5650cb420e70b2c8de951954b9975 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.k3mbno4twhg2qf7g@alap3.anarazel.de
---
contrib/cube/cube.c | 2 +-
contrib/seg/seg.c | 448 ++++++++++++++++++++++++++++------------------------
2 files changed, 240 insertions(+), 210 deletions(-)
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 2bb2ed0..43ecccf 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -101,7 +101,7 @@ bool g_cube_leaf_consistent(NDBOX *key, NDBOX *query, StrategyNumber strategy);
bool g_cube_internal_consistent(NDBOX *key, NDBOX *query, StrategyNumber strategy);
/*
-** Auxiliary funxtions
+** Auxiliary functions
*/
static double distance_1D(double a1, double a2, double b1, double b2);
static bool cube_is_point_internal(NDBOX *cube);
diff --git a/contrib/seg/seg.c b/contrib/seg/seg.c
index 895d879..c2b23b5 100644
--- a/contrib/seg/seg.c
+++ b/contrib/seg/seg.c
@@ -47,56 +47,48 @@ 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
+** Auxiliary functions
*/
static int restore(char *s, float val, int n);
-
/*****************************************************************************
* Input/Output functions
*****************************************************************************/
@@ -193,13 +185,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 +204,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 +281,7 @@ gseg_penalty(GISTENTRY *origentry, GISTENTRY *newentry, float *result)
fprintf(stderr, "\t%g\n", *result);
#endif
- return (result);
+ PG_RETURN_POINTER(result);
}
/*
@@ -309,14 +308,13 @@ 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;
+ SEG *seg, *seg_l, *seg_r;
gseg_picksplit_item *sort_items;
OffsetNumber *left,
*right;
@@ -359,13 +357,13 @@ 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));
+ seg_l = palloc(sizeof(SEG));
+ memcpy(seg_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);
+ seg_l = DatumGetPointer(DirectFunctionCall2(seg_union, PointerGetDatum(seg_l), PointerGetDatum(sort_items[i].data)));
*left++ = sort_items[i].index;
v->spl_nleft++;
}
@@ -373,13 +371,13 @@ gseg_picksplit(GistEntryVector *entryvec,
/*
* Likewise for the right page.
*/
- datum_r = (SEG *) palloc(sizeof(SEG));
+ seg_r = palloc(sizeof(SEG));
memcpy(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);
+ seg_r = DatumGetPointer(DirectFunctionCall2(seg_union, PointerGetDatum(seg_r), PointerGetDatum(sort_items[i].data)));
*right++ = sort_items[i].index;
v->spl_nright++;
}
@@ -387,16 +385,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 +405,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 +423,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 +468,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));
+ Datum a = PG_GETARG_DATUM(0);
+ Datum b = PG_GETARG_DATUM(1);
+
+ PG_RETURN_DATUM( DirectFunctionCall2(seg_contains, a, b) );
}
/*****************************************************************************
* 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;
+ int cmp = DatumGetInt32(
+ DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1)));
+
+ PG_RETURN_BOOL(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 +641,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 +681,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 +707,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 +731,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 +760,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 +774,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 +788,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 +806,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 +818,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 +1033,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.5.5
0002-Remove-support-for-version-0-calling-conventions.patchtext/x-patch; charset=US-ASCII; name=0002-Remove-support-for-version-0-calling-conventions.patchDownload
From 42c911255750813894e0d46897250ca2cd52533c Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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 and obsolete fmgr README contents relating
to it.
Author: Andres Freund
Discussion: https://postgr.es/m/20161208213441.k3mbno4twhg2qf7g@alap3.anarazel.de
---
contrib/earthdistance/earthdistance.c | 22 --
doc/src/sgml/xfunc.sgml | 269 ++++------------
src/backend/utils/fmgr/README | 254 +--------------
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 ++-
10 files changed, 106 insertions(+), 911 deletions(-)
diff --git a/contrib/earthdistance/earthdistance.c b/contrib/earthdistance/earthdistance.c
index 861b166..6ad6d87 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 94a7ad7..e6313dd 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -1610,14 +1610,10 @@ 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">
@@ -2138,160 +2134,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 +2158,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 +2172,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"
@@ -2427,27 +2271,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
@@ -2461,7 +2345,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,
@@ -2493,9 +2377,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>
@@ -2630,7 +2512,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[
@@ -2641,31 +2523,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/README b/src/backend/utils/fmgr/README
index e7e7ae9..5a2331f 100644
--- a/src/backend/utils/fmgr/README
+++ b/src/backend/utils/fmgr/README
@@ -3,62 +3,19 @@ src/backend/utils/fmgr/README
Function Manager
================
-Proposal For Function-Manager Redesign 19-Nov-2000
---------------------------------------
-
-We know that the existing mechanism for calling Postgres functions needs
-to be redesigned. It has portability problems because it makes
-assumptions about parameter passing that violate ANSI C; it fails to
-handle NULL arguments and results cleanly; and "function handlers" that
-support a class of functions (such as fmgr_pl) can only be done via a
-really ugly, non-reentrant kluge. (Global variable set during every
-function call, forsooth.) Here is a proposal for fixing these problems.
-
-In the past, the major objections to redoing the function-manager
-interface have been (a) it'll be quite tedious to implement, since every
-built-in function and everyplace that calls such functions will need to
-be touched; (b) such wide-ranging changes will be difficult to make in
-parallel with other development work; (c) it will break existing
-user-written loadable modules that define "C language" functions. While
-I have no solution to the "tedium" aspect, I believe I see an answer to
-the other problems: by use of function handlers, we can support both old
-and new interfaces in parallel for both callers and callees, at some
-small efficiency cost for the old styles. That way, most of the changes
-can be done on an incremental file-by-file basis --- we won't need a
-"big bang" where everything changes at once. Support for callees
-written in the old style can be left in place indefinitely, to provide
-backward compatibility for user-written C functions.
-
-
-Changes In pg_proc (System Data About a Function)
--------------------------------------------------
-
-A new column "proisstrict" will be added to the system pg_proc table.
-This is a boolean value which will be TRUE if the function is "strict",
-that is it always returns NULL when any of its inputs are NULL. The
-function manager will check this field and skip calling the function when
-it's TRUE and there are NULL inputs. This allows us to remove explicit
-NULL-value tests from many functions that currently need them (not to
-mention fixing many more that need them but don't have them). A function
-that is not marked "strict" is responsible for checking whether its inputs
-are NULL or not. Most builtin functions will be marked "strict".
-
-An optional WITH parameter will be added to CREATE FUNCTION to allow
-specification of whether user-defined functions are strict or not. I am
-inclined to make the default be "not strict", since that seems to be the
-more useful case for functions expressed in SQL or a PL language, but
-am open to arguments for the other choice.
-
-
-The New Function-Manager Interface
-----------------------------------
-
-The core of the new design is revised data structures for representing
-the result of a function lookup and for representing the parameters
-passed to a specific function invocation. (We want to keep function
-lookup separate from function call, since many parts of the system apply
-the same function over and over; the lookup overhead should be paid once
-per query, not once per tuple.)
+[This file originally explained the transition from the V0 to the V1
+interface. Now it just explains some internals and rationale for the V1
+interface, while the V0 interface has been removed.]
+
+The V1 Function-Manager Interface
+---------------------------------
+
+The core of the design is data structures for representing the result of a
+function lookup and for representing the parameters passed to a specific
+function invocation. (We want to keep function lookup separate from
+function call, since many parts of the system apply the same function over
+and over; the lookup overhead should be paid once per query, not once per
+tuple.)
When a function is looked up in pg_proc, the result is represented as
@@ -183,50 +140,6 @@ should have no portability or optimization problems.
Function Coding Conventions
---------------------------
-As an example, int4 addition goes from old-style
-
-int32
-int4pl(int32 arg1, int32 arg2)
-{
- return arg1 + arg2;
-}
-
-to new-style
-
-Datum
-int4pl(FunctionCallInfo fcinfo)
-{
- /* we assume the function is marked "strict", so we can ignore
- * NULL-value handling */
-
- return Int32GetDatum(DatumGetInt32(fcinfo->arg[0]) +
- DatumGetInt32(fcinfo->arg[1]));
-}
-
-This is, of course, much uglier than the old-style code, but we can
-improve matters with some well-chosen macros for the boilerplate parts.
-I propose below macros that would make the code look like
-
-Datum
-int4pl(PG_FUNCTION_ARGS)
-{
- int32 arg1 = PG_GETARG_INT32(0);
- int32 arg2 = PG_GETARG_INT32(1);
-
- PG_RETURN_INT32( arg1 + arg2 );
-}
-
-This is still more code than before, but it's fairly readable, and it's
-also amenable to machine processing --- for example, we could probably
-write a script that scans code like this and extracts argument and result
-type info for comparison to the pg_proc table.
-
-For the standard data types float4, float8, and int8, these macros should hide
-whether the types are pass-by-value or pass-by reference, by incorporating
-indirection and space allocation if needed. This will offer a considerable
-gain in readability, and it also opens up the opportunity to make these types
-be pass-by-value on machines where it's feasible to do so.
-
Here are the proposed macros and coding conventions:
The definition of an fmgr-callable function will always look like
@@ -291,67 +204,6 @@ fields of FunctionCallInfo, it should just do it. I doubt that providing
syntactic-sugar macros for these cases is useful.
-Call-Site Coding Conventions
-----------------------------
-
-There are many places in the system that call either a specific function
-(for example, the parser invokes "textin" by name in places) or a
-particular group of functions that have a common argument list (for
-example, the optimizer invokes selectivity estimation functions with
-a fixed argument list). These places will need to change, but we should
-try to avoid making them significantly uglier than before.
-
-Places that invoke an arbitrary function with an arbitrary argument list
-can simply be changed to fill a FunctionCallInfoData structure directly;
-that'll be no worse and possibly cleaner than what they do now.
-
-When invoking a specific built-in function by name, we have generally
-just written something like
- result = textin ( ... args ... )
-which will not work after textin() is converted to the new call style.
-I suggest that code like this be converted to use "helper" functions
-that will create and fill in a FunctionCallInfoData struct. For
-example, if textin is being called with one argument, it'd look
-something like
- result = DirectFunctionCall1(textin, PointerGetDatum(argument));
-These helper routines will have declarations like
- Datum DirectFunctionCall2(PGFunction func, Datum arg1, Datum arg2);
-Note it will be the caller's responsibility to convert to and from
-Datum; appropriate conversion macros should be used.
-
-The DirectFunctionCallN routines will not bother to fill in
-fcinfo->flinfo (indeed cannot, since they have no idea about an OID for
-the target function); they will just set it NULL. This is unlikely to
-bother any built-in function that could be called this way. Note also
-that this style of coding cannot pass a NULL input value nor cope with
-a NULL result (it couldn't before, either!). We can make the helper
-routines ereport an error if they see that the function returns a NULL.
-
-When invoking a function that has a known argument signature, we have
-usually written either
- result = fmgr(targetfuncOid, ... args ... );
-or
- result = fmgr_ptr(FmgrInfo *finfo, ... args ... );
-depending on whether an FmgrInfo lookup has been done yet or not.
-This kind of code can be recast using helper routines, in the same
-style as above:
- result = OidFunctionCall1(funcOid, PointerGetDatum(argument));
- result = FunctionCall2(funcCallInfo,
- PointerGetDatum(argument),
- Int32GetDatum(argument));
-Again, this style of coding does not allow for expressing NULL inputs
-or receiving a NULL result.
-
-As with the callee-side situation, I propose adding argument conversion
-macros that hide whether int8, float4, and float8 are pass-by-value or
-pass-by-reference.
-
-The existing helper functions fmgr(), fmgr_c(), etc will be left in
-place until all uses of them are gone. Of course their internals will
-have to change in the first step of implementation, but they can
-continue to support the same external appearance.
-
-
Support for TOAST-Able Data Types
---------------------------------
@@ -474,83 +326,3 @@ context. fn_mcxt normally points at the context that was
CurrentMemoryContext at the time the FmgrInfo structure was created;
in any case it is required to be a context at least as long-lived as the
FmgrInfo itself.
-
-
-Telling the Difference Between Old- and New-Style Functions
------------------------------------------------------------
-
-During the conversion process, we carried two different pg_language
-entries, "internal" and "newinternal", for internal functions. The
-function manager used the language code to distinguish which calling
-convention to use. (Old-style internal functions were supported via
-a function handler.) As of Nov. 2000, no old-style internal functions
-remain, so we can drop support for them. We will remove the old "internal"
-pg_language entry and rename "newinternal" to "internal".
-
-The interim solution for dynamically-loaded compiled functions has been
-similar: two pg_language entries "C" and "newC". This naming convention
-is not desirable for the long run, and yet we cannot stop supporting
-old-style user functions. Instead, it seems better to use just one
-pg_language entry "C", and require the dynamically-loaded library to
-provide additional information that identifies new-style functions.
-This avoids compatibility problems --- for example, existing dump
-scripts will identify PL language handlers as being in language "C",
-which would be wrong under the "newC" convention. Also, this approach
-should generalize more conveniently for future extensions to the function
-interface specification.
-
-Given a dynamically loaded function named "foo" (note that the name being
-considered here is the link-symbol name, not the SQL-level function name),
-the function manager will look for another function in the same dynamically
-loaded library named "pg_finfo_foo". If this second function does not
-exist, then foo is assumed to be called old-style, thus ensuring backwards
-compatibility with existing libraries. If the info function does exist,
-it is expected to have the signature
-
- Pg_finfo_record * pg_finfo_foo (void);
-
-The info function will be called by the fmgr, and must return a pointer
-to a Pg_finfo_record struct. (The returned struct will typically be a
-statically allocated constant in the dynamic-link library.) The current
-definition of the struct is just
-
- typedef struct {
- int api_version;
- } Pg_finfo_record;
-
-where api_version is 0 to indicate old-style or 1 to indicate new-style
-calling convention. In future releases, additional fields may be defined
-after api_version, but these additional fields will only be used if
-api_version is greater than 1.
-
-These details will be hidden from the author of a dynamically loaded
-function by using a macro. To define a new-style dynamically loaded
-function named foo, write
-
- PG_FUNCTION_INFO_V1(foo);
-
- Datum
- foo(PG_FUNCTION_ARGS)
- {
- ...
- }
-
-The function itself is written using the same conventions as for new-style
-internal functions; you just need to add the PG_FUNCTION_INFO_V1() macro.
-Note that old-style and new-style functions can be intermixed in the same
-library, depending on whether or not you write a PG_FUNCTION_INFO_V1() for
-each one.
-
-The SQL declaration for a dynamically-loaded function is CREATE FUNCTION
-foo ... LANGUAGE C regardless of whether it is old- or new-style.
-
-New-style dynamic functions will be invoked directly by fmgr, and will
-therefore have the same performance as internal functions after the initial
-pg_proc lookup overhead. Old-style dynamic functions will be invoked via
-a handler, and will therefore have a small performance penalty.
-
-To allow old-style dynamic functions to work safely on toastable datatypes,
-the handler for old-style functions will automatically detoast toastable
-arguments before passing them to the old-style function. A new-style
-function is expected to take care of toasted arguments by using the
-standard argument access macros defined above.
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9fb6952..68d2110 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
@@ -2081,58 +1772,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 6128752..0c695e2 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -336,10 +336,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 3c26b2f..b167c8a 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 dd2d1b2..b1dbc57 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 bdd1b1b..8f28bff 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 574ef0d..b9595cc 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 986d54c..d7fb849 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_POINTER(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.5.5
On 27 March 2017 at 10:45, Craig Ringer <craig@2ndquadrant.com> wrote:
Passes "make check" and recovery tests, check-world running now.
A couple of fixes pending.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 27 March 2017 at 10:59, Craig Ringer <craig@2ndquadrant.com> wrote:
On 27 March 2017 at 10:45, Craig Ringer <craig@2ndquadrant.com> wrote:
Passes "make check" and recovery tests, check-world running now.
A couple of fixes pending.
Updated.
I didn't have any way to make
seg_l = (SEG *) DatumGetPointer(DirectFunctionCall2(seg_union,
PointerGetDatum(seg_l),
PointerGetDatum(sort_items[i].data)));
pretty, but *shrug*.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Move-contrib-seg-to-only-use-V1-calling-conventions.patchtext/x-patch; charset=US-ASCII; name=0001-Move-contrib-seg-to-only-use-V1-calling-conventions.patchDownload
From a1ae04c2d6aec7d8093d95e616a1e286e101d612 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.k3mbno4twhg2qf7g@alap3.anarazel.de
---
contrib/cube/cube.c | 2 +-
contrib/seg/seg.c | 458 ++++++++++++++++++++++++++++------------------------
2 files changed, 247 insertions(+), 213 deletions(-)
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 2bb2ed0..43ecccf 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -101,7 +101,7 @@ bool g_cube_leaf_consistent(NDBOX *key, NDBOX *query, StrategyNumber strategy);
bool g_cube_internal_consistent(NDBOX *key, NDBOX *query, StrategyNumber strategy);
/*
-** Auxiliary funxtions
+** Auxiliary functions
*/
static double distance_1D(double a1, double a2, double b1, double b2);
static bool cube_is_point_internal(NDBOX *cube);
diff --git a/contrib/seg/seg.c b/contrib/seg/seg.c
index 895d879..3980deb 100644
--- a/contrib/seg/seg.c
+++ b/contrib/seg/seg.c
@@ -47,56 +47,48 @@ 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
+** Auxiliary functions
*/
static int restore(char *s, float val, int n);
-
/*****************************************************************************
* Input/Output functions
*****************************************************************************/
@@ -193,13 +185,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 +204,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 +281,7 @@ gseg_penalty(GISTENTRY *origentry, GISTENTRY *newentry, float *result)
fprintf(stderr, "\t%g\n", *result);
#endif
- return (result);
+ PG_RETURN_POINTER(result);
}
/*
@@ -309,14 +308,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;
+ SEG *seg,
+ *seg_l,
+ *seg_r;
gseg_picksplit_item *sort_items;
OffsetNumber *left,
*right;
@@ -359,13 +359,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));
+ seg_l = palloc(sizeof(SEG));
+ memcpy(seg_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);
+ seg_l = (SEG *) DatumGetPointer(DirectFunctionCall2(seg_union,
+ PointerGetDatum(seg_l), PointerGetDatum(sort_items[i].data)));
*left++ = sort_items[i].index;
v->spl_nleft++;
}
@@ -373,30 +374,33 @@ gseg_picksplit(GistEntryVector *entryvec,
/*
* Likewise for the right page.
*/
- datum_r = (SEG *) palloc(sizeof(SEG));
- memcpy(datum_r, sort_items[firstright].data, sizeof(SEG));
+ seg_r = palloc(sizeof(SEG));
+ memcpy(seg_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);
+ seg_r = (SEG *) DatumGetPointer(DirectFunctionCall2(seg_union,
+ PointerGetDatum(seg_r), PointerGetDatum(sort_items[i].data)));
*right++ = sort_items[i].index;
v->spl_nright++;
}
- v->spl_ldatum = PointerGetDatum(datum_l);
- v->spl_rdatum = PointerGetDatum(datum_r);
+ v->spl_ldatum = PointerGetDatum(seg_l);
+ v->spl_rdatum = PointerGetDatum(seg_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 +409,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 +427,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 +472,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));
+ Datum a = PG_GETARG_DATUM(0);
+ Datum b = PG_GETARG_DATUM(1);
+
+ PG_RETURN_DATUM(DirectFunctionCall2(seg_contains, a, b));
}
/*****************************************************************************
* 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;
+ int cmp = DatumGetInt32(
+ DirectFunctionCall2(seg_cmp, PG_GETARG_DATUM(0), PG_GETARG_DATUM(1)));
+
+ PG_RETURN_BOOL(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 +645,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 +685,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 +711,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 +735,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 +764,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 +778,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 +792,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 +810,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 +822,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 +1037,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.5.5
0002-Remove-support-for-version-0-calling-conventions.patchtext/x-patch; charset=US-ASCII; name=0002-Remove-support-for-version-0-calling-conventions.patchDownload
From 37f9aa5359942775080cc5c787e2c6988a954e06 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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 and obsolete fmgr README contents relating
to it.
Author: Andres Freund
Discussion: https://postgr.es/m/20161208213441.k3mbno4twhg2qf7g@alap3.anarazel.de
---
contrib/earthdistance/earthdistance.c | 22 --
doc/src/sgml/xfunc.sgml | 269 ++++------------
src/backend/utils/fmgr/README | 254 +--------------
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 ++-
10 files changed, 106 insertions(+), 911 deletions(-)
diff --git a/contrib/earthdistance/earthdistance.c b/contrib/earthdistance/earthdistance.c
index 861b166..6ad6d87 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 94a7ad7..e6313dd 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -1610,14 +1610,10 @@ 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">
@@ -2138,160 +2134,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 +2158,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 +2172,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"
@@ -2427,27 +2271,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
@@ -2461,7 +2345,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,
@@ -2493,9 +2377,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>
@@ -2630,7 +2512,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[
@@ -2641,31 +2523,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/README b/src/backend/utils/fmgr/README
index e7e7ae9..5a2331f 100644
--- a/src/backend/utils/fmgr/README
+++ b/src/backend/utils/fmgr/README
@@ -3,62 +3,19 @@ src/backend/utils/fmgr/README
Function Manager
================
-Proposal For Function-Manager Redesign 19-Nov-2000
---------------------------------------
-
-We know that the existing mechanism for calling Postgres functions needs
-to be redesigned. It has portability problems because it makes
-assumptions about parameter passing that violate ANSI C; it fails to
-handle NULL arguments and results cleanly; and "function handlers" that
-support a class of functions (such as fmgr_pl) can only be done via a
-really ugly, non-reentrant kluge. (Global variable set during every
-function call, forsooth.) Here is a proposal for fixing these problems.
-
-In the past, the major objections to redoing the function-manager
-interface have been (a) it'll be quite tedious to implement, since every
-built-in function and everyplace that calls such functions will need to
-be touched; (b) such wide-ranging changes will be difficult to make in
-parallel with other development work; (c) it will break existing
-user-written loadable modules that define "C language" functions. While
-I have no solution to the "tedium" aspect, I believe I see an answer to
-the other problems: by use of function handlers, we can support both old
-and new interfaces in parallel for both callers and callees, at some
-small efficiency cost for the old styles. That way, most of the changes
-can be done on an incremental file-by-file basis --- we won't need a
-"big bang" where everything changes at once. Support for callees
-written in the old style can be left in place indefinitely, to provide
-backward compatibility for user-written C functions.
-
-
-Changes In pg_proc (System Data About a Function)
--------------------------------------------------
-
-A new column "proisstrict" will be added to the system pg_proc table.
-This is a boolean value which will be TRUE if the function is "strict",
-that is it always returns NULL when any of its inputs are NULL. The
-function manager will check this field and skip calling the function when
-it's TRUE and there are NULL inputs. This allows us to remove explicit
-NULL-value tests from many functions that currently need them (not to
-mention fixing many more that need them but don't have them). A function
-that is not marked "strict" is responsible for checking whether its inputs
-are NULL or not. Most builtin functions will be marked "strict".
-
-An optional WITH parameter will be added to CREATE FUNCTION to allow
-specification of whether user-defined functions are strict or not. I am
-inclined to make the default be "not strict", since that seems to be the
-more useful case for functions expressed in SQL or a PL language, but
-am open to arguments for the other choice.
-
-
-The New Function-Manager Interface
-----------------------------------
-
-The core of the new design is revised data structures for representing
-the result of a function lookup and for representing the parameters
-passed to a specific function invocation. (We want to keep function
-lookup separate from function call, since many parts of the system apply
-the same function over and over; the lookup overhead should be paid once
-per query, not once per tuple.)
+[This file originally explained the transition from the V0 to the V1
+interface. Now it just explains some internals and rationale for the V1
+interface, while the V0 interface has been removed.]
+
+The V1 Function-Manager Interface
+---------------------------------
+
+The core of the design is data structures for representing the result of a
+function lookup and for representing the parameters passed to a specific
+function invocation. (We want to keep function lookup separate from
+function call, since many parts of the system apply the same function over
+and over; the lookup overhead should be paid once per query, not once per
+tuple.)
When a function is looked up in pg_proc, the result is represented as
@@ -183,50 +140,6 @@ should have no portability or optimization problems.
Function Coding Conventions
---------------------------
-As an example, int4 addition goes from old-style
-
-int32
-int4pl(int32 arg1, int32 arg2)
-{
- return arg1 + arg2;
-}
-
-to new-style
-
-Datum
-int4pl(FunctionCallInfo fcinfo)
-{
- /* we assume the function is marked "strict", so we can ignore
- * NULL-value handling */
-
- return Int32GetDatum(DatumGetInt32(fcinfo->arg[0]) +
- DatumGetInt32(fcinfo->arg[1]));
-}
-
-This is, of course, much uglier than the old-style code, but we can
-improve matters with some well-chosen macros for the boilerplate parts.
-I propose below macros that would make the code look like
-
-Datum
-int4pl(PG_FUNCTION_ARGS)
-{
- int32 arg1 = PG_GETARG_INT32(0);
- int32 arg2 = PG_GETARG_INT32(1);
-
- PG_RETURN_INT32( arg1 + arg2 );
-}
-
-This is still more code than before, but it's fairly readable, and it's
-also amenable to machine processing --- for example, we could probably
-write a script that scans code like this and extracts argument and result
-type info for comparison to the pg_proc table.
-
-For the standard data types float4, float8, and int8, these macros should hide
-whether the types are pass-by-value or pass-by reference, by incorporating
-indirection and space allocation if needed. This will offer a considerable
-gain in readability, and it also opens up the opportunity to make these types
-be pass-by-value on machines where it's feasible to do so.
-
Here are the proposed macros and coding conventions:
The definition of an fmgr-callable function will always look like
@@ -291,67 +204,6 @@ fields of FunctionCallInfo, it should just do it. I doubt that providing
syntactic-sugar macros for these cases is useful.
-Call-Site Coding Conventions
-----------------------------
-
-There are many places in the system that call either a specific function
-(for example, the parser invokes "textin" by name in places) or a
-particular group of functions that have a common argument list (for
-example, the optimizer invokes selectivity estimation functions with
-a fixed argument list). These places will need to change, but we should
-try to avoid making them significantly uglier than before.
-
-Places that invoke an arbitrary function with an arbitrary argument list
-can simply be changed to fill a FunctionCallInfoData structure directly;
-that'll be no worse and possibly cleaner than what they do now.
-
-When invoking a specific built-in function by name, we have generally
-just written something like
- result = textin ( ... args ... )
-which will not work after textin() is converted to the new call style.
-I suggest that code like this be converted to use "helper" functions
-that will create and fill in a FunctionCallInfoData struct. For
-example, if textin is being called with one argument, it'd look
-something like
- result = DirectFunctionCall1(textin, PointerGetDatum(argument));
-These helper routines will have declarations like
- Datum DirectFunctionCall2(PGFunction func, Datum arg1, Datum arg2);
-Note it will be the caller's responsibility to convert to and from
-Datum; appropriate conversion macros should be used.
-
-The DirectFunctionCallN routines will not bother to fill in
-fcinfo->flinfo (indeed cannot, since they have no idea about an OID for
-the target function); they will just set it NULL. This is unlikely to
-bother any built-in function that could be called this way. Note also
-that this style of coding cannot pass a NULL input value nor cope with
-a NULL result (it couldn't before, either!). We can make the helper
-routines ereport an error if they see that the function returns a NULL.
-
-When invoking a function that has a known argument signature, we have
-usually written either
- result = fmgr(targetfuncOid, ... args ... );
-or
- result = fmgr_ptr(FmgrInfo *finfo, ... args ... );
-depending on whether an FmgrInfo lookup has been done yet or not.
-This kind of code can be recast using helper routines, in the same
-style as above:
- result = OidFunctionCall1(funcOid, PointerGetDatum(argument));
- result = FunctionCall2(funcCallInfo,
- PointerGetDatum(argument),
- Int32GetDatum(argument));
-Again, this style of coding does not allow for expressing NULL inputs
-or receiving a NULL result.
-
-As with the callee-side situation, I propose adding argument conversion
-macros that hide whether int8, float4, and float8 are pass-by-value or
-pass-by-reference.
-
-The existing helper functions fmgr(), fmgr_c(), etc will be left in
-place until all uses of them are gone. Of course their internals will
-have to change in the first step of implementation, but they can
-continue to support the same external appearance.
-
-
Support for TOAST-Able Data Types
---------------------------------
@@ -474,83 +326,3 @@ context. fn_mcxt normally points at the context that was
CurrentMemoryContext at the time the FmgrInfo structure was created;
in any case it is required to be a context at least as long-lived as the
FmgrInfo itself.
-
-
-Telling the Difference Between Old- and New-Style Functions
------------------------------------------------------------
-
-During the conversion process, we carried two different pg_language
-entries, "internal" and "newinternal", for internal functions. The
-function manager used the language code to distinguish which calling
-convention to use. (Old-style internal functions were supported via
-a function handler.) As of Nov. 2000, no old-style internal functions
-remain, so we can drop support for them. We will remove the old "internal"
-pg_language entry and rename "newinternal" to "internal".
-
-The interim solution for dynamically-loaded compiled functions has been
-similar: two pg_language entries "C" and "newC". This naming convention
-is not desirable for the long run, and yet we cannot stop supporting
-old-style user functions. Instead, it seems better to use just one
-pg_language entry "C", and require the dynamically-loaded library to
-provide additional information that identifies new-style functions.
-This avoids compatibility problems --- for example, existing dump
-scripts will identify PL language handlers as being in language "C",
-which would be wrong under the "newC" convention. Also, this approach
-should generalize more conveniently for future extensions to the function
-interface specification.
-
-Given a dynamically loaded function named "foo" (note that the name being
-considered here is the link-symbol name, not the SQL-level function name),
-the function manager will look for another function in the same dynamically
-loaded library named "pg_finfo_foo". If this second function does not
-exist, then foo is assumed to be called old-style, thus ensuring backwards
-compatibility with existing libraries. If the info function does exist,
-it is expected to have the signature
-
- Pg_finfo_record * pg_finfo_foo (void);
-
-The info function will be called by the fmgr, and must return a pointer
-to a Pg_finfo_record struct. (The returned struct will typically be a
-statically allocated constant in the dynamic-link library.) The current
-definition of the struct is just
-
- typedef struct {
- int api_version;
- } Pg_finfo_record;
-
-where api_version is 0 to indicate old-style or 1 to indicate new-style
-calling convention. In future releases, additional fields may be defined
-after api_version, but these additional fields will only be used if
-api_version is greater than 1.
-
-These details will be hidden from the author of a dynamically loaded
-function by using a macro. To define a new-style dynamically loaded
-function named foo, write
-
- PG_FUNCTION_INFO_V1(foo);
-
- Datum
- foo(PG_FUNCTION_ARGS)
- {
- ...
- }
-
-The function itself is written using the same conventions as for new-style
-internal functions; you just need to add the PG_FUNCTION_INFO_V1() macro.
-Note that old-style and new-style functions can be intermixed in the same
-library, depending on whether or not you write a PG_FUNCTION_INFO_V1() for
-each one.
-
-The SQL declaration for a dynamically-loaded function is CREATE FUNCTION
-foo ... LANGUAGE C regardless of whether it is old- or new-style.
-
-New-style dynamic functions will be invoked directly by fmgr, and will
-therefore have the same performance as internal functions after the initial
-pg_proc lookup overhead. Old-style dynamic functions will be invoked via
-a handler, and will therefore have a small performance penalty.
-
-To allow old-style dynamic functions to work safely on toastable datatypes,
-the handler for old-style functions will automatically detoast toastable
-arguments before passing them to the old-style function. A new-style
-function is expected to take care of toasted arguments by using the
-standard argument access macros defined above.
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9fb6952..68d2110 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
@@ -2081,58 +1772,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 6128752..0c695e2 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -336,10 +336,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 3c26b2f..b167c8a 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 dd2d1b2..b1dbc57 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 bdd1b1b..8f28bff 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 574ef0d..b9595cc 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 986d54c..d7fb849 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_POINTER(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.5.5
Craig Ringer <craig@2ndquadrant.com> writes:
I didn't have any way to make
seg_l = (SEG *) DatumGetPointer(DirectFunctionCall2(seg_union,
PointerGetDatum(seg_l),
PointerGetDatum(sort_items[i].data)));
pretty, but *shrug*.
For the builtin types, fmgr.h generally defines a datum-to-specific-
pointer-type macro, eg it has these for bytea:
#define DatumGetByteaP(X) ((bytea *) PG_DETOAST_DATUM(X))
#define PG_GETARG_BYTEA_P(n) DatumGetByteaP(PG_GETARG_DATUM(n))
(A type that doesn't have toast support would just use DatumGetPointer
instead of PG_DETOAST_DATUM.) Replacing "(SEG *) DatumGetPointer(...)"
with "DatumGetSegP(...)" would help a little here.
I wouldn't necessarily do that if this is the only place that would
benefit, but there might be more scope for upgrading seg.c's notation
than just here.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers