Re: doc examples for pghandler

From: Mark Wong <mark(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: doc examples for pghandler
Date: 2020-08-17 23:30:07
Message-ID: 20200817233007.GA20236@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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/

Attachment Content-Type Size
plsample-readme.diff text/plain 474 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message a.pervushina 2020-08-17 23:54:00 Re: psql: add \si, \sm, \st and \sr functions to show CREATE commands for indexes, matviews, triggers and tables
Previous Message Tom Lane 2020-08-17 22:37:47 Re: BUG #16583: merge join on tables with different DB collation behind postgres_fdw fails