Thread: Pre-v11 appearances of the word "procedure" in v11 docs

Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Geoghegan
Date:
I noticed that the word "procedure" appears in at least a few places
in the v11 docs as a not-quite-apt synonym of "function". For example,
https://www.postgresql.org/docs/11/static/plpgsql-trigger.html talks
about "PL/pgSQL Trigger Procedures" which are actually all functions
in practice.

I think that this has the potential to confuse users, since, of
course, a function is a distinct variety of object to a procedure in
v11. Tightening up the wording seems like a good idea. I'm not sure if
this was discussed already, but didn't find anything during a quick
search.

-- 
Peter Geoghegan


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Tom Lane
Date:
Peter Geoghegan <pg@bowt.ie> writes:
> I noticed that the word "procedure" appears in at least a few places
> in the v11 docs as a not-quite-apt synonym of "function". For example,
> https://www.postgresql.org/docs/11/static/plpgsql-trigger.html talks
> about "PL/pgSQL Trigger Procedures" which are actually all functions
> in practice.

Agreed, we'd better stop being cavalier about whether that's an
exact synonym or not.

            regards, tom lane


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Geoghegan
Date:
On Tue, Aug 14, 2018 at 10:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Agreed, we'd better stop being cavalier about whether that's an
> exact synonym or not.

I made an open item for this.

-- 
Peter Geoghegan


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Eisentraut
Date:
What should we do with the use of the keyword PROCEDURE in the CREATE
OPERATOR and CREATE TRIGGER syntaxes?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Tom Lane
Date:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> What should we do with the use of the keyword PROCEDURE in the CREATE
> OPERATOR and CREATE TRIGGER syntaxes?

We're kinda stuck with that.  We could add FUNCTION as a preferred
synonym, perhaps, but I doubt that'd really be worth the trouble.

            regards, tom lane


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Geoghegan
Date:
On Tue, Aug 14, 2018 at 1:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> We're kinda stuck with that.  We could add FUNCTION as a preferred
> synonym, perhaps, but I doubt that'd really be worth the trouble.

Seems sufficient to note in the relevant docs that it's a legacy thing.

-- 
Peter Geoghegan


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Vik Fearing
Date:
On 14/08/18 22:39, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> What should we do with the use of the keyword PROCEDURE in the CREATE
>> OPERATOR and CREATE TRIGGER syntaxes?
> 
> We're kinda stuck with that.  We could add FUNCTION as a preferred
> synonym, perhaps, but I doubt that'd really be worth the trouble.

If someone were to waste their time doing it, would we want FUNCTION or
ROUTINE?
-- 
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Eisentraut
Date:
Attached are my proposed patches.  The first is the documentation
change, which basically just substitutes the words, with some occasional
rephrasing.  And then patches to extend the syntaxes of CREATE OPERATOR,
CREATE TRIGGER, and CREATE EVENT TRIGGER to accept FUNCTION in place of
PROCEDURE.  I decided to do that because with the adjustments from the
first patch, the documentation had become comically inconsistent in some
places and it was easier to just fix the underlying problem than to
explain the reasons for the inconsistencies everywhere.  I didn't go
around change all the commands in contrib modules etc. to keep the patch
size under control.  This could perhaps be done later.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Geoghegan
Date:
On Fri, Aug 17, 2018 at 7:15 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> Attached are my proposed patches.

I take it that you propose all 3 for backpatch to v11?

-- 
Peter Geoghegan


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
"Jonathan S. Katz"
Date:
> On Aug 17, 2018, at 10:15 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
>
> Attached are my proposed patches.  The first is the documentation
> change, which basically just substitutes the words, with some occasional
> rephrasing.  And then patches to extend the syntaxes of CREATE OPERATOR,
> CREATE TRIGGER, and CREATE EVENT TRIGGER to accept FUNCTION in place of
> PROCEDURE.  I decided to do that because with the adjustments from the
> first patch, the documentation had become comically inconsistent in some
> places and it was easier to just fix the underlying problem than to
> explain the reasons for the inconsistencies everywhere.  I didn't go
> around change all the commands in contrib modules etc. to keep the patch
> size under control.  This could perhaps be done later.

Thank for going through this, I know it’s an arduous task.

I reviewed the patches, here are my comments.

Patch 1
======

Built ok. All tests passed. Manual checks passed. I would recommend
backpatching this to 11.

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d6688e13f4..abe67fa50e 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -19,7 +19,7 @@ <title>Overview</title>
     <itemizedlist>
      <listitem>
       <para>
-       can be used to create functions and trigger procedures,
+       can be used to create functions and triggers,
       </para>

Perhaps: “can be used to create functions used in queries and triggers” or
just “can be used to create functions and trigger functions.”

Similarly for:

diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index 01f6207d36..97906a67df 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -16,7 +16,7 @@ <title>PL/Tcl - Tcl Procedural Language</title>
    <productname>PostgreSQL</productname> database system
    that enables the <ulink url="http://www.tcl.tk/">
    Tcl language</ulink> to be used to write functions and
-   trigger procedures.
+   triggers.
   </para>

I’d be more specific, as the trigger executes the trigger function.

Patch 2
======

Built ok. Tests passed. Manual tests passed. Did a few pg_dump / restore
scenarios as well.

Code looked ok to me, though I’d consider adding a comment in
“operatorcmds.c" about how function / procedure are interchangeable,
which is why both outcomes are identical:

@@ -120,6 +120,8 @@ DefineOperator(List *names, List *parameters)
                         (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
                          errmsg("SETOF type not allowed for operator argument")));
         }
+        else if (strcmp(defel->defname, "function") == 0)
+            functionName = defGetQualifiedName(defel);
         else if (strcmp(defel->defname, "procedure") == 0)
             functionName = defGetQualifiedName(defel);
         else if (strcmp(defel->defname, "commutator") == 0)

I can also make arguments both ways about backpatching Patch 2 to v11. It
would be less confusing for users (“I have to create a procedure to create an
operator??”), though the type of user who is creating a custom operator is more
sophisticated.

I think it would be better to backpatch to v11, but I can be convinced otherwise
by a strong argument.

Patch 3
======

I was trying to determine if changing to “EXECUTE FUNCTION” would be part
of the SQL standard, which with some (poorly done?) searching it looks like it is.

Before commenting further, this patch would give me the most cause-for-pause about
backporting to v11 at this point as triggers are highly utilized. Like Patch 2 I can
make arguments for both, but I need to think about it a bit more.

Anyway, builds ok, tests passed.

When manually testing, I noticed pg_dump still outputs “EXECUTE PROCEDURE”
as part of the trigger syntax. Here is the example I used to produce it:

    CREATE TABLE a (
        x int GENERATED BY DEFAULT AS iDENTITY PRIMARY KEY,
        y int NOT NULL
    );
    CREATE TABLE b (
        a_id int PRIMARY KEY REFERENCES a (x) UNIQUE,
        created_at timestamptz NOT NULL DEFAULT CURRENT_TIMESTAMP
    );

    CREATE OR REPLACE FUNCTION a_insrt ()
    RETURNS trigger
    AS $$
        BEGIN
            IF TG_OP = 'INSERT' THEN
                INSERT INTO b VALUES (NEW.x, CURRENT_TIMESTAMP);
                RETURN NEW;
            END IF;
        END
    $$ LANGUAGE plpgsql;

    CREATE TRIGGER a_insrt
    AFTER INSERT ON a
    FOR EACH ROW
    EXECUTE FUNCTION a_insrt();

Other than that, the patch looked ok to me.

Thanks again for pulling all of this together,

Jonathan

Attachment

Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Eisentraut
Date:
On 17/08/2018 21:57, Peter Geoghegan wrote:
> On Fri, Aug 17, 2018 at 7:15 AM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> Attached are my proposed patches.
> 
> I take it that you propose all 3 for backpatch to v11?

yes

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Eisentraut
Date:
On 18/08/2018 19:37, Jonathan S. Katz wrote:
> I reviewed the patches, here are my comments.

Committed all three with some adjustments.  Thanks.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
"Jonathan S. Katz"
Date:
> On Aug 22, 2018, at 9:15 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
>
> On 18/08/2018 19:37, Jonathan S. Katz wrote:
>> I reviewed the patches, here are my comments.
>
> Committed all three with some adjustments.  Thanks.

Awesome, thanks! I removed the open item.

Jonathan

Attachment

Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Eisentraut
Date:
Some things from this thread were left for post-11; see attached patch.

Specifically, this changes pg_dump and ruleutils.c to use the FUNCTION
keyword instead of PROCEDURE in trigger and event trigger definitions,
which was postponed because of the required catversion bump.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

Re: Pre-v11 appearances of the word "procedure" in v11 docs

From
Peter Eisentraut
Date:
On 04/02/2019 23:02, Peter Eisentraut wrote:
> Some things from this thread were left for post-11; see attached patch.
> 
> Specifically, this changes pg_dump and ruleutils.c to use the FUNCTION
> keyword instead of PROCEDURE in trigger and event trigger definitions,
> which was postponed because of the required catversion bump.

done

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services