Re: Documentation about PL transforms
On 2022-02-22 12:59, Chapman Flack wrote:
It would have been painful to write documentation of get_func_trftypes
saying its result isn't what get_transform_{from.to}sql expect, so
patch 1 does add a get_call_trftypes that returns a List *.
Patch 2 then updates the docs as discussed in this thread. It turned
out
plhandler.sgml was kind of a long monolith of text even before adding
transform information, so I broke it into sections first. This patch
adds
the section markup without reindenting, so the changes aren't obscured.
The chapter had also fallen behind the introduction of procedures, so
I have changed many instances of 'function' to the umbrella term
'routine'.
Patch 3 simply reindents for the new section markup and rewraps.
Whitespace only.
Patch 4 updates src/test/modules/plsample to demonstrate handling of
transforms (and to add some more comments generally).
Here is a rebase.From 9c726423d47a114a82ca703c0e03a62e3d74f1f6 Mon Sep 17 00:00:00 2001
From: Chapman Flack
Date: Mon, 21 Feb 2022 20:56:28 -0500
Subject: [PATCH v2 1/4] Warmup: add a get_call_trftypes function
The existing get_func_trftypes function produces an Oid[], where
both existing get_transform_{from,to}sql functions that depend
on the result expect a List*.
Rather than writing documentation awkwardly describing functions
that won't play together, add a get_call_trftypes function that
returns List*. (The name get_call_... to distinguish from
get_func_... follows the naming used in funcapi.h for a function
returning information about either a function or a procedure.)
---
src/backend/utils/cache/lsyscache.c | 18 ++
src/include/funcapi.h | 5 +
src/include/utils/lsyscache.h | 1 +
3 files changed, 24 insertions(+)
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 1b7e11b..3cd94db 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -2117,6 +2117,24 @@ get_transform_tosql(Oid typid, Oid langid, List *trftypes)
return InvalidOid;
}
+/*
+ * get_call_trftypes
+ *
+ * A helper function that does not itself query the transform cache, but
+ * constructs the transform-type List expected by the functions above.
+ */
+List *
+get_call_trftypes(HeapTuple procTup)
+{
+ Datum protrftypes;
+ bool isNull;
+
+ protrftypes = SysCacheGetAttr(PROCOID, procTup,
+ Anum_pg_proc_protrftypes,
+ &isNull);
+ return isNull ? NIL : oid_array_to_list(protrftypes);
+}
+
/*-- TYPE CACHE -- */
diff --git a/src/include/funcapi.h b/src/include/funcapi.h
index dc3d819..70c3e13 100644
--- a/src/include/funcapi.h
+++ b/src/include/funcapi.h
@@ -175,7 +175,12 @@ extern int get_func_arg_info(HeapTuple procTup,
extern int get_func_input_arg_names(Datum proargnames, Datum proargmodes,
char ***arg_names);
+/*
+ * A deprecated earlier version of get_call_trftypes (in lsyscache.h).
+ * That version produces a List, which is the form downstream functions expect.
+ */
extern int get_func_trftypes(HeapTuple procTup, Oid **p_trftypes);
+
extern char *get_func_result_name(Oid functionId);
extern TupleDesc build_function_result_tupdesc_d(char prokind,
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index b8dd27d..93b19e7 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -139,6 +139,7 @@ extern char get_rel_relkind(Oid relid);
extern bool get_rel_relispartition(Oid relid);
extern Oid get_rel_tablespace(Oid relid);
extern char get_rel_persistence(Oid relid);
+extern List *get_call_trftypes(HeapTuple procTup);
extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
extern Oid get_transform_tosql(Oid typid, Oid langid, List *trftypes);
extern bool get_typisdefined(Oid typid);
--
2.7.3
From 7fb3f8971c4c573903b1d08bcbeb126e31a6544c Mon Sep 17 00:00:00 2001
From: Chapman Flack
Date: Mon, 21 Feb 2022 20:59:32 -0500
Subject: [PATCH v2 2/4] Update PL handler implementation docs
The original purpose was to add information on support for
CREATE TRANSFORM (which must be explicitly coded in any PL
implementation intending to support it). But the plhandler
section was about as long as a monolith of text ought to be,
even before adding transform information, so reorganized
first into sections.
Front-loaded with short descriptions of the three possible
functions (call handler, validator, inline handler) registered
with CREATE LANGUAGE. The latter two were afterthoughts in historical
sequence, but the docs don't need to present them that way.
The section had also fallen behind the introduction of procedures,
so updated to generally use the umbrella term 'routine' in place
of 'function'.
New section markup added here without indentation change, to avoid
obscuring changes. A follow-on commit will reindent and rewrap.
---
doc/src/sgml/event-trigger.sgml| 16 ++
doc/src/sgml/plhandler.sgml
Re: Documentation about PL transforms
On 05.02.22 00:55, Chapman Flack wrote: I'm thinking plhandler.sgml is the only place that really needs to be said; readers looking up CREATE TRANSFORM and using an existing PL that supports it don't need to know how the sausage is made. (Maybe it is worth mentioning there, though, that it isn't possible to develop transforms for an arbitrary PL unless that PL applies transforms.) makes sense I noticed the CREATE TRANSFORM docs show the argument list as (argument_type [, ...]) even though check_transform_function will reject any argument list of length other than 1 or with type other than internal. Is that an overly-generic way to show the syntax, or is that a style with precedent elsewhere in the docs? That could be corrected. As long as a PL handler has the sole responsibility for invoking its transforms, I wonder if it would make sense to allow a PL to define what its transforms should look like, maybe replacing check_transform_function with a transform_validator for the PL. I'm not proposing such a patch here, but I am willing to prepare one for plhandler.sgml and maybe pltemplate.c documenting the current situation, if nobody tells me I'm wrong about something here. Maybe. This kind of thing would mostly be driven what a PL wants in actual practice, and then how that could be generalized to many/all PLs.
Re: Documentation about PL transforms
On 02/07/22 10:59, Peter Eisentraut wrote: > On 05.02.22 00:55, Chapman Flack wrote: >> I'm thinking plhandler.sgml is the only place that really needs to be >> ... >> worth mentioning there, though, that it isn't possible to develop >> transforms for an arbitrary PL unless that PL applies transforms.) > > makes sense > >> (argument_type [, ...]) even though check_transform_function will reject >> any argument list of length other than 1 or with type other than internal. > > That could be corrected. I'll work on some doc patches. >> As long as a PL handler has the sole responsibility for invoking >> its transforms, I wonder if it would make sense to allow a PL to >> define what its transforms should look like, maybe replacing >> check_transform_function with a transform_validator for the PL. > > Maybe. This kind of thing would mostly be driven what a PL wants in actual > practice, and then how that could be generalized to many/all PLs. It has since occurred to me that another benefit of having a transform_validator per PL would be immediate error reporting if someone, for whatever reason, tries out CREATE TRANSFORM for a PL that doesn't grok transforms. Regards, -Chap
Re: Documentation about PL transforms
On 02/07/22 15:14, Chapman Flack wrote:
> It has since occurred to me that another benefit of having a
> transform_validator per PL would be immediate error reporting
> if someone, for whatever reason, tries out CREATE TRANSFORM
> for a PL that doesn't grok transforms.
The same could be achieved, I guess, by an event trigger, though that
would take positive effort to set up, where the benefit of a per-PL
transform_validator would be that if a given PL does not bother to
provide one, CREATE TRANSFORM for it would automatically fail. (And
such a validator would not have to spend most of its life ignoring
other DDL.)
That does reveal another documentation gap: table 40.1 does not show
CREATE or DROP TRANSFORM being supported for event triggers. I've
confirmed they work, though. I'll tack that onto the doc patch.
I notice our transforms lack the named groups of 9075-2. With those
(plus our LANGUAGE clause), I could write, for a made-up example:
CREATE TRANSFORM FOR hstore LANGUAGE plpython3u
asdict (
FROM SQL WITH FUNCTION hstore_to_plpython3dict,
TO SQL WITH FUNCTION plpython3dict_to_hstore)
asnamedtuple (
FROM SQL WITH FUNCTION hstore_to_plpython3namedtup,
TO SQL WITH FUNCTION plpython3namedtup_to_hstore);
CREATE FUNCTION f1(val hstore) RETURNS int
LANGUAGE plpython3u
TRANSFORM GROUP asdict FOR TYPE hstore
...
CREATE FUNCTION f2(val hstore) RETURNS int
LANGUAGE plpython3u
TRANSFORM GROUP asnamedtuple FOR TYPE hstore
...
It seems to me that could be useful, in cases where a PL offers
more than one good choice for representing a PostgreSQL type and
the preferred one could depend on the function.
Was that considered and rejected for our transforms, or were ours
just based on an earlier 9075-2 without the named groups?
Also, I am doubtful of our Compatibility note, "There is a CREATE
TRANSFORM command in the SQL standard, but it is for adapting data
types to client languages."
In my reading of 9075-2, I do see the transforms used for client
languages (all the s), but I also see
the to-sql and from-sql functions being applied in
whenever "R is an external routine" and the type "is a user-defined type".
The latter doesn't seem much different from our usage. The differences
I see are (1) our LANGUAGE clause, (2) we don't have a special
"user-defined type" category limiting what types can have transforms
(and (3), we don't have the named groups). And we are applying them
/only/ for server-side routines, rather than for server and client code.
The ISO transforms work by mapping the ("user-defined") type to some
existing SQL type for which the PL has a standard mapping already. Ours
work by mapping the type directly to some suitable type in the PL.
Am I reading this accurately?
Regards,
-Chap
Re: Documentation about PL transforms
On 02/07/22 15:14, Chapman Flack wrote: > I'll work on some doc patches. It seems a bit of an impedance mismatch that there is a get_func_trftypes producing a C Oid[] (with its length returned separately), while get_transform_fromsql and get_transform_tosql both expect a List. There is only one in-core caller of get_func_trftypes (it is print_function_trftypes in ruleutils.c), while both in-core PLs that currently support transforms are reduced to rolling their own List by grabbing the protrftypes Datum and hitting it with oid_array_to_list. Would it be reasonable to deprecate get_func_trftypes and instead provide a get_call_trftypes (following the naming of get_call_result_type where _call_ encompasses functions and procedures) that returns a List, and use that consistently? Regards, -Chap
Re: Documentation about PL transforms
On 02/21/22 16:09, Chapman Flack wrote:
> On 02/07/22 15:14, Chapman Flack wrote:
>> I'll work on some doc patches.
>
> It seems a bit of an impedance mismatch that there is a get_func_trftypes
> producing a C Oid[] (with its length returned separately), while
> get_transform_fromsql and get_transform_tosql both expect a List.
> ...
> Would it be reasonable to deprecate get_func_trftypes and instead
> provide a get_call_trftypes
It would have been painful to write documentation of get_func_trftypes
saying its result isn't what get_transform_{from.to}sql expect, so
patch 1 does add a get_call_trftypes that returns a List *.
Patch 2 then updates the docs as discussed in this thread. It turned out
plhandler.sgml was kind of a long monolith of text even before adding
transform information, so I broke it into sections first. This patch adds
the section markup without reindenting, so the changes aren't obscured.
The chapter had also fallen behind the introduction of procedures, so
I have changed many instances of 'function' to the umbrella term
'routine'.
Patch 3 simply reindents for the new section markup and rewraps.
Whitespace only.
Patch 4 updates src/test/modules/plsample to demonstrate handling of
transforms (and to add some more comments generally).
I'll add this to the CF.
Regards,
-Chap
>From 7e6027cdcf1dbf7a10c0bad5059816cef762b1b9 Mon Sep 17 00:00:00 2001
From: Chapman Flack
Date: Mon, 21 Feb 2022 20:56:28 -0500
Subject: [PATCH 1/4] Warmup: add a get_call_trftypes function
The existing get_func_trftypes function produces an Oid[], where
both existing get_transform_{from,to}sql functions that depend
on the result expect a List*.
Rather than writing documentation awkwardly describing functions
that won't play together, add a get_call_trftypes function that
returns List*. (The name get_call_... to distinguish from
get_func_... follows the naming used in funcapi.h for a function
returning information about either a function or a procedure.)
---
src/backend/utils/cache/lsyscache.c | 18 ++
src/include/funcapi.h | 5 +
src/include/utils/lsyscache.h | 1 +
3 files changed, 24 insertions(+)
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index feef999..a49ccad 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -2107,6 +2107,24 @@ get_transform_tosql(Oid typid, Oid langid, List *trftypes)
return InvalidOid;
}
+/*
+ * get_call_trftypes
+ *
+ * A helper function that does not itself query the transform cache, but
+ * constructs the transform-type List expected by the functions above.
+ */
+List *
+get_call_trftypes(HeapTuple procTup)
+{
+ Datum protrftypes;
+ bool isNull;
+
+ protrftypes = SysCacheGetAttr(PROCOID, procTup,
+ Anum_pg_proc_protrftypes,
+ &isNull);
+ return isNull ? NIL : oid_array_to_list(protrftypes);
+}
+
/*-- TYPE CACHE -- */
diff --git a/src/include/funcapi.h b/src/include/funcapi.h
index ba927c2..7c61560 100644
--- a/src/include/funcapi.h
+++ b/src/include/funcapi.h
@@ -175,7 +175,12 @@ extern int get_func_arg_info(HeapTuple procTup,
extern int get_func_input_arg_names(Datum proargnames, Datum proargmodes,
char ***arg_names);
+/*
+ * A deprecated earlier version of get_call_trftypes (in lsyscache.h).
+ * That version produces a List, which is the form downstream functions expect.
+ */
extern int get_func_trftypes(HeapTuple procTup, Oid **p_trftypes);
+
extern char *get_func_result_name(Oid functionId);
extern TupleDesc build_function_result_tupdesc_d(char prokind,
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index b8dd27d..93b19e7 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -139,6 +139,7 @@ extern char get_rel_relkind(Oid relid);
extern bool get_rel_relispartition(Oid relid);
extern Oid get_rel_tablespace(Oid relid);
extern char get_rel_persistence(Oid relid);
+extern List *get_call_trftypes(HeapTuple procTup);
extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
extern Oid get_transform_tosql(Oid typid, Oid langid, List *trftypes);
extern bool get_typisdefined(Oid typid);
--
2.7.3
>From 447a4aaecdf3a23a47c11ad741b49f81475e34e3 Mon Sep 17 00:00:00 2001
From: Chapman Flack
Date: Mon, 21 Feb 2022 20:59:32 -0500
Subject: [PATCH 2/4] Update PL handler implementation docs
The original purpose was to add information on support for
CREATE TRANSFORM (which must be explicitly coded in any PL
implementation intending to support it). But the plhandler
section was about as long as a monolith of text ought to be,
even before adding transform information, so reorganized
first into sections.
Front-loaded with short descriptions of the three possible
functions (call handler, validator, inline handler) registered
with CREATE LANGUAGE. The latter two were afterthoughts in historical
sequence, but the
Re: Documentation about PL transforms
Hi I am doing an review of this patch and I have two comments 1. I am not sure if get_call_trftypes is a good name - the prefix get_call is used when some runtime data is processed. This function just returns reformatted data from the system catalogue. Maybe get_func_trftypes_list, or just replace function get_func_trftypes (now, the list is an array, so there should not be performance issues). For consistency, the new function should be used in plperl and plpython too. Probably this function is not widely used, so I don't see why we need to have two almost equal functions in code. 2. +It also contains the OID of the intended procedural language and whether +that procedural language is declared as TRUSTED, useful +if a single inline handler is supporting more than one procedural language. I am not sure if this sentence is in the correct place. Maybe can be mentioned separately, so generally handlers can be used by more than one procedural language. But maybe I don't understand this sentence. Regards Pavel
Re: Documentation about PL transforms
On 2022-07-13 00:26, Pavel Stehule wrote: 1. I am not sure if get_call_trftypes is a good name - the prefix get_call is used when some runtime data is processed. I guess I hadn't caught on that the prefix carried that meaning. To me, it appeared to be a prefix used to avoid being specific to 'function' or 'procedure'. This function just returns reformatted data from the system catalogue. Maybe get_func_trftypes_list, or just replace function get_func_trftypes (now, the list is an array, so there should not be performance issues). For consistency, the new function should be used in plperl and plpython too. Probably this function is not If it is acceptable to replace get_func_trftypes like that, I can produce such a patch. 2. +It also contains the OID of the intended procedural language and whether +that procedural language is declared as TRUSTED, useful +if a single inline handler is supporting more than one procedural language. I am not sure if this sentence is in the correct place. Maybe can be mentioned separately, so generally handlers can be used by more than one procedural language. But maybe I don't understand this sentence. My point was that, if the structure did /not/ include the OID of the PL and its TRUSTED property, then it would not be possible for a single inline handler to support more than one PL. So that is why it is a good thing that those are included in the structure, and why it would be a bad thing if they were not. Would it be clearer to say: It also contains the OID of the intended procedural language and whether that procedural language is declared as TRUSTED. While these values are redundant if the inline handler is serving only a single procedural language, they are necessary to allow one inline handler to serve more than one PL. Regards, -Chap
Re: Documentation about PL transforms
čt 28. 7. 2022 v 4:06 odesílatel napsal: > On 2022-07-13 00:26, Pavel Stehule wrote: > > > 1. I am not sure if get_call_trftypes is a good name - the prefix > > get_call > > is used when some runtime data is processed. > > I guess I hadn't caught on that the prefix carried that meaning. > To me, it appeared to be a prefix used to avoid being specific to > 'function' or 'procedure'. > I am sure this function is in Postgres significantly longer than Postgres has support for 'procedures'. > > > This function just returns > > reformatted data from the system catalogue. Maybe > > get_func_trftypes_list, > > or just replace function get_func_trftypes (now, the list is an array, > > so > > there should not be performance issues). For consistency, the new > > function > > should be used in plperl and plpython too. Probably this function is > > not > > If it is acceptable to replace get_func_trftypes like that, I can > produce > such a patch. > > > 2. > > > > +It also contains the OID of the intended procedural language and > > whether > > +that procedural language is declared as > > TRUSTED, > > useful > > +if a single inline handler is supporting more than one procedural > > language. > > > > I am not sure if this sentence is in the correct place. Maybe can be > > mentioned separately, > > so generally handlers can be used by more than one procedural language. > > But > > maybe > > I don't understand this sentence. > > My point was that, if the structure did /not/ include the OID of > the PL and its TRUSTED property, then it would not be possible > for a single inline handler to support more than one PL. So that > is why it is a good thing that those are included in the structure, > and why it would be a bad thing if they were not. > > Would it be clearer to say: > > It also contains the OID of the intended procedural language and whether > that procedural language is declared as TRUSTED. > While these values are redundant if the inline handler is serving only > a single procedural language, they are necessary to allow one inline > handler to serve more than one PL. > ok Regards Pavel > > Regards, > -Chap >
Re: Documentation about PL transforms
On 2022-07-28 01:51, Pavel Stehule wrote: Would it be clearer to say: It also contains the OID of the intended procedural language and whether that procedural language is declared as TRUSTED. While these values are redundant if the inline handler is serving only a single procedural language, they are necessary to allow one inline handler to serve more than one PL. ok I will probably be unable to produce a new patch for this CF. Family obligation. Regards, -Chap
Re: Documentation about PL transforms
pá 29. 7. 2022 v 18:35 odesílatel napsal: > On 2022-07-28 01:51, Pavel Stehule wrote: > >> Would it be clearer to say: > >> > >> It also contains the OID of the intended procedural language and > >> whether > >> that procedural language is declared as TRUSTED. > >> While these values are redundant if the inline handler is serving only > >> a single procedural language, they are necessary to allow one inline > >> handler to serve more than one PL. > > > > ok > > I will probably be unable to produce a new patch for this CF. > Family obligation. > ok Regards Pavel > > Regards, > -Chap >
