Thread

Commits

Add PL/Sample to src/test/modules/
- adbe62d04b36 14.0 landed

doc examples for pghandler

Mark Wong <mark@2ndquadrant.com> — 2020-06-12T17:26:48Z

Hi everyone,

Would some additional procedure language handler code examples in the
documentation be good to add?  I've put some together in the attached
patch, and can log it to a future commitfest if people think it would
be a good addition.

Regards,
Mark
-- 
Mark Wong
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/

Re: doc examples for pghandler

Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-12T19:10:20Z

Mark Wong <mark@2ndquadrant.com> writes:
> Would some additional procedure language handler code examples in the
> documentation be good to add?  I've put some together in the attached
> patch, and can log it to a future commitfest if people think it would
> be a good addition.

Hmm.  The existing doc examples are really pretty laughable, because
there's such a large gap between the offered skeleton and a workable
handler.  So I agree it'd be nice to do better, but I'm suspicious of
having large chunks of sample code in the docs --- that's a maintenance
problem, if only because we likely won't notice when we break it.
Also, if somebody is hoping to copy-and-paste such code, it isn't
that nice to work from if it's embedded in SGML.

I wonder if it'd be possible to adapt what you have here into some
tiny contrib module that doesn't do very much useful, but can at
least be tested to see that it compiles; moreover it could be
copied verbatim to serve as a starting point for a new PL.

			regards, tom lane

Re: doc examples for pghandler

Mark Wong <mark@2ndquadrant.com> — 2020-06-12T22:35:25Z

On Fri, Jun 12, 2020 at 03:10:20PM -0400, Tom Lane wrote:
> Mark Wong <mark@2ndquadrant.com> writes:
> > Would some additional procedure language handler code examples in the
> > documentation be good to add?  I've put some together in the attached
> > patch, and can log it to a future commitfest if people think it would
> > be a good addition.
> 
> Hmm.  The existing doc examples are really pretty laughable, because
> there's such a large gap between the offered skeleton and a workable
> handler.  So I agree it'd be nice to do better, but I'm suspicious of
> having large chunks of sample code in the docs --- that's a maintenance
> problem, if only because we likely won't notice when we break it.
> Also, if somebody is hoping to copy-and-paste such code, it isn't
> that nice to work from if it's embedded in SGML.
> 
> I wonder if it'd be possible to adapt what you have here into some
> tiny contrib module that doesn't do very much useful, but can at
> least be tested to see that it compiles; moreover it could be
> copied verbatim to serve as a starting point for a new PL.

I do have the code examples in a repo. [1]  The 0.4 directory consists
of everything the examples show.  

It would be easy enough to adapt that for contrib, and move some of the
content from the doc patch into that.  Then touch up the handler chapter
to reference the contrib module.

Does that sound more useful?

[1] https://gitlab.com/markwkm/yappl

-- 
Mark Wong
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/

Re: doc examples for pghandler

Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-13T02:13:41Z

Mark Wong <mark@2ndquadrant.com> writes:
> On Fri, Jun 12, 2020 at 03:10:20PM -0400, Tom Lane wrote:
>> I wonder if it'd be possible to adapt what you have here into some
>> tiny contrib module that doesn't do very much useful, but can at
>> least be tested to see that it compiles; moreover it could be
>> copied verbatim to serve as a starting point for a new PL.

> I do have the code examples in a repo. [1]  The 0.4 directory consists
> of everything the examples show.  

> It would be easy enough to adapt that for contrib, and move some of the
> content from the doc patch into that.  Then touch up the handler chapter
> to reference the contrib module.

On second thought, contrib/ is not quite the right place, because we
typically expect modules there to actually get installed, meaning they
have to have at least some end-user usefulness.  The right place for
a toy PL handler is probably src/test/modules/; compare for example
src/test/modules/test_parser/, which is serving quite the same sort
of purpose as a skeleton text search parser.

			regards, tom lane

Re: doc examples for pghandler

Michael Paquier <michael@paquier.xyz> — 2020-06-13T04:19:17Z

On Fri, Jun 12, 2020 at 10:13:41PM -0400, Tom Lane wrote:
> On second thought, contrib/ is not quite the right place, because we
> typically expect modules there to actually get installed, meaning they
> have to have at least some end-user usefulness.  The right place for
> a toy PL handler is probably src/test/modules/; compare for example
> src/test/modules/test_parser/, which is serving quite the same sort
> of purpose as a skeleton text search parser.

+1 for src/test/modules/, and if you can provide some low-level API
coverage through this module, that's even better.
--
Michael

Re: doc examples for pghandler

Mark Wong <mark@2ndquadrant.com> — 2020-06-15T03:45:17Z

On Sat, Jun 13, 2020 at 01:19:17PM +0900, Michael Paquier wrote:
> On Fri, Jun 12, 2020 at 10:13:41PM -0400, Tom Lane wrote:
> > On second thought, contrib/ is not quite the right place, because we
> > typically expect modules there to actually get installed, meaning they
> > have to have at least some end-user usefulness.  The right place for
> > a toy PL handler is probably src/test/modules/; compare for example
> > src/test/modules/test_parser/, which is serving quite the same sort
> > of purpose as a skeleton text search parser.
> 
> +1 for src/test/modules/, and if you can provide some low-level API
> coverage through this module, that's even better.

Sounds good to me.  Something more like the attached patch?

Regards,
Mark
-- 
Mark Wong
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/

Re: doc examples for pghandler

Michael Paquier <michael@paquier.xyz> — 2020-06-15T07:47:01Z

On Sun, Jun 14, 2020 at 08:45:17PM -0700, Mark Wong wrote:
> Sounds good to me.  Something more like the attached patch?

That's the idea.  I have not gone in details into what you have here,
but perhaps it would make sense to do a bit more and show how things
are done in the context of a PL function called in a trigger?  Your
patch removes from the docs a code block that outlined that.
--
Michael

Re: doc examples for pghandler

Mark Wong <mark@2ndquadrant.com> — 2020-08-11T20:01:10Z

On Mon, Jun 15, 2020 at 04:47:01PM +0900, Michael Paquier wrote:
> On Sun, Jun 14, 2020 at 08:45:17PM -0700, Mark Wong wrote:
> > Sounds good to me.  Something more like the attached patch?
> 
> That's the idea.  I have not gone in details into what you have here,
> but perhaps it would make sense to do a bit more and show how things
> are done in the context of a PL function called in a trigger?  Your
> patch removes from the docs a code block that outlined that.

Ah, right.  For the moment I've added some empty conditionals for
trigger and event trigger handling.

I've created a new entry in the commitfest app. [1]  I'll keep at it. :)

Regards,
Mark

[1] https://commitfest.postgresql.org/29/2678/

-- 
Mark Wong
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/

Re: doc examples for pghandler

Michael Paquier <michael@paquier.xyz> — 2020-08-14T05:25:52Z

On Tue, Aug 11, 2020 at 01:01:10PM -0700, Mark Wong wrote:
> Ah, right.  For the moment I've added some empty conditionals for
> trigger and event trigger handling.
> 
> I've created a new entry in the commitfest app. [1]  I'll keep at it. :)

Thanks for the patch.  I have reviewed and reworked it as the
attached.  Some comments below.

+PGFILEDESC = "PL/Sample - procedural language"
+
+REGRESS = create_pl create_func select_func
+
+EXTENSION = plsample
+EXTVERSION = 0.1

This makefile has a couple of mistakes, and can be simplified a lot:
- make check does not work, as you forgot a PGXS part.
- MODULES can just be used as there is only one file (forgot WIN32RES
in OBJS for example)
- DATA does not need the .control file.

.gitignore was missing.

We could just use 1.0 instead of 0.1 for the version number.  That's
not a big deal one way or another, but 1.0 is more consistent with the
other modules.

plsample--1.0.sql should complain if attempting to load the file from
psql.  Also I have cleaned up the README.

Not sure that there is a point in having three different files for the
regression tests.  create_pl.sql is actually not necessary as you
can do the same with CREATE EXTENSION.

The header list of plsample.c was inconsistent with the style used
normally in modules, and I have extended a bit the handler function so
as we return a result only if the return type of the procedure is text
for the source text of the function, tweaked the results a bit, etc.
There was a family of small issues, like using ALLOCSET_SMALL_SIZES
for the context creation.  We could of course expand the sample
handler more in the future to check for pseudotype results, have a
validator, but that could happen later, if necessary.
--
Michael

Re: doc examples for pghandler

Mark Wong <mark@2ndquadrant.com> — 2020-08-17T23:30:07Z

On Fri, Aug 14, 2020 at 02:25:52PM +0900, Michael Paquier wrote:
> On Tue, Aug 11, 2020 at 01:01:10PM -0700, Mark Wong wrote:
> > Ah, right.  For the moment I've added some empty conditionals for
> > trigger and event trigger handling.
> > 
> > I've created a new entry in the commitfest app. [1]  I'll keep at it. :)
> 
> Thanks for the patch.  I have reviewed and reworked it as the
> attached.  Some comments below.
> 
> +PGFILEDESC = "PL/Sample - procedural language"
> +
> +REGRESS = create_pl create_func select_func
> +
> +EXTENSION = plsample
> +EXTVERSION = 0.1
> 
> This makefile has a couple of mistakes, and can be simplified a lot:
> - make check does not work, as you forgot a PGXS part.
> - MODULES can just be used as there is only one file (forgot WIN32RES
> in OBJS for example)
> - DATA does not need the .control file.
> 
> .gitignore was missing.
> 
> We could just use 1.0 instead of 0.1 for the version number.  That's
> not a big deal one way or another, but 1.0 is more consistent with the
> other modules.
> 
> plsample--1.0.sql should complain if attempting to load the file from
> psql.  Also I have cleaned up the README.
> 
> Not sure that there is a point in having three different files for the
> regression tests.  create_pl.sql is actually not necessary as you
> can do the same with CREATE EXTENSION.
> 
> The header list of plsample.c was inconsistent with the style used
> normally in modules, and I have extended a bit the handler function so
> as we return a result only if the return type of the procedure is text
> for the source text of the function, tweaked the results a bit, etc.
> There was a family of small issues, like using ALLOCSET_SMALL_SIZES
> for the context creation.  We could of course expand the sample
> handler more in the future to check for pseudotype results, have a
> validator, but that could happen later, if necessary.

Thanks for fixing all of that up for me.  I did have a couple mental
notes for a couple of the last items. :)

I've attached a small word diff to suggest a few different words to use
in the README, if that sounds better?

Regards,
Mark
-- 
Mark Wong
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/

Re: doc examples for pghandler

Michael Paquier <michael@paquier.xyz> — 2020-08-18T02:14:06Z

On Mon, Aug 17, 2020 at 04:30:07PM -0700, Mark Wong wrote:
> I've attached a small word diff to suggest a few different words to use
> in the README, if that sounds better?

Sounds good to me.  So applied with those changes.  It is really
tempting to add an example of validator (one simple thing would be to
return an error if trying to use TRIGGEROID or EVTTRIGGEROID), but
that may not be the best thing to do for a test module.  And what we
have here is already much better than the original docs.
--
Michael