Обсуждение: remove undocumented assign syntax from plpgsql doc

Hello

remove undocumented syntax for assign statements in plpgsql doc examples

related to http://www.postgresql.org/message-id/CAJ4CxLkBFNAAsWdz=CsDkU1nC0De60WO15de=aUf6Q1W9DSj7w@mail.gmail.com
thread

Regards

Pavel Stehule

Applied.

---------------------------------------------------------------------------

On Wed, May 29, 2013 at 10:02:20AM +0200, Pavel Stehule wrote:
> Hello
>
> remove undocumented syntax for assign statements in plpgsql doc examples
>
> related to http://www.postgresql.org/message-id/CAJ4CxLkBFNAAsWdz=CsDkU1nC0De60WO15de=aUf6Q1W9DSj7w@mail.gmail.com
> thread
>
> Regards
>
> Pavel Stehule

> diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
> index 19498c6..1817c9d 100644
> --- a/doc/src/sgml/plpgsql.sgml
> +++ b/doc/src/sgml/plpgsql.sgml
> @@ -3815,20 +3815,20 @@ CREATE OR REPLACE FUNCTION update_emp_view() RETURNS TRIGGER AS $$
>              DELETE FROM emp WHERE empname = OLD.empname;
>              IF NOT FOUND THEN RETURN NULL; END IF;
>
> -            OLD.last_updated = now();
> +            OLD.last_updated := now();
>              INSERT INTO emp_audit VALUES('D', user, OLD.*);
>              RETURN OLD;
>          ELSIF (TG_OP = 'UPDATE') THEN
>              UPDATE emp SET salary = NEW.salary WHERE empname = OLD.empname;
>              IF NOT FOUND THEN RETURN NULL; END IF;
>
> -            NEW.last_updated = now();
> +            NEW.last_updated := now();
>              INSERT INTO emp_audit VALUES('U', user, NEW.*);
>              RETURN NEW;
>          ELSIF (TG_OP = 'INSERT') THEN
>              INSERT INTO emp VALUES(NEW.empname, NEW.salary);
>
> -            NEW.last_updated = now();
> +            NEW.last_updated := now();
>              INSERT INTO emp_audit VALUES('I', user, NEW.*);
>              RETURN NEW;
>          END IF;
> @@ -3913,10 +3913,10 @@ AS $maint_sales_summary_bytime$
>          -- Work out the increment/decrement amount(s).
>          IF (TG_OP = 'DELETE') THEN
>
> -            delta_time_key = OLD.time_key;
> -            delta_amount_sold = -1 * OLD.amount_sold;
> -            delta_units_sold = -1 * OLD.units_sold;
> -            delta_amount_cost = -1 * OLD.amount_cost;
> +            delta_time_key := OLD.time_key;
> +            delta_amount_sold := -1 * OLD.amount_sold;
> +            delta_units_sold := -1 * OLD.units_sold;
> +            delta_amount_cost := -1 * OLD.amount_cost;
>
>          ELSIF (TG_OP = 'UPDATE') THEN
>
> @@ -3928,17 +3928,17 @@ AS $maint_sales_summary_bytime$
>                                                        OLD.time_key, NEW.time_key;
>              END IF;
>
> -            delta_time_key = OLD.time_key;
> -            delta_amount_sold = NEW.amount_sold - OLD.amount_sold;
> -            delta_units_sold = NEW.units_sold - OLD.units_sold;
> -            delta_amount_cost = NEW.amount_cost - OLD.amount_cost;
> +            delta_time_key := OLD.time_key;
> +            delta_amount_sold := NEW.amount_sold - OLD.amount_sold;
> +            delta_units_sold := NEW.units_sold - OLD.units_sold;
> +            delta_amount_cost := NEW.amount_cost - OLD.amount_cost;
>
>          ELSIF (TG_OP = 'INSERT') THEN
>
> -            delta_time_key = NEW.time_key;
> -            delta_amount_sold = NEW.amount_sold;
> -            delta_units_sold = NEW.units_sold;
> -            delta_amount_cost = NEW.amount_cost;
> +            delta_time_key := NEW.time_key;
> +            delta_amount_sold := NEW.amount_sold;
> +            delta_units_sold := NEW.units_sold;
> +            delta_amount_cost := NEW.amount_cost;
>
>          END IF;
>

>
> --
> Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-docs


--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

Bruce Momjian <bruce@momjian.us> writes:
> On Wed, May 29, 2013 at 10:02:20AM +0200, Pavel Stehule wrote:
>> remove undocumented syntax for assign statements in plpgsql doc examples

> Applied.

I thought the consensus in the referenced thread had been to go the other
way.  We're not going to remove the syntax option to use "=", so shouldn't
we document it rather than pretending it doesn't exist?

The end of the other thread was
http://www.postgresql.org/message-id/1370122048.6266.YahooMailNeo@web162902.mail.bf1.yahoo.com

            regards, tom lane

is not better to be consistent in doc?

Im not aganst to documentation second syntax, but examples in doc should be consistent

On Sat, Jan 11, 2014 at 01:52:30PM -0500, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Wed, May 29, 2013 at 10:02:20AM +0200, Pavel Stehule wrote:
> >> remove undocumented syntax for assign statements in plpgsql doc examples
>
> > Applied.
>
> I thought the consensus in the referenced thread had been to go the other
> way.  We're not going to remove the syntax option to use "=", so shouldn't
> we document it rather than pretending it doesn't exist?
>
> The end of the other thread was
> http://www.postgresql.org/message-id/1370122048.6266.YahooMailNeo@web162902.mail.bf1.yahoo.com

Oh, I think you are right.  I have reverted the patch.  Attached is
proposed documentation for '='.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

Bruce Momjian <bruce@momjian.us> writes:
> Oh, I think you are right.  I have reverted the patch.  Attached is
> proposed documentation for '='.

Meh.  Variable initialization is only one of multiple cases (assignment,
GET DIAGNOSTICS; maybe others, I've not examined the grammar).  Also,
if we do it like this, we're implying that both := and = are equally
preferred, which might not be the impression we want to leave.

I'd be a bit inclined to just stick a NOTE somewhere saying that "="
can be used in place of ":=" for assignment.

            regards, tom lane

On Sat, Jan 11, 2014 at 02:12:49PM -0500, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Oh, I think you are right.  I have reverted the patch.  Attached is
> > proposed documentation for '='.
>
> Meh.  Variable initialization is only one of multiple cases (assignment,
> GET DIAGNOSTICS; maybe others, I've not examined the grammar).  Also,
> if we do it like this, we're implying that both := and = are equally
> preferred, which might not be the impression we want to leave.
>
> I'd be a bit inclined to just stick a NOTE somewhere saying that "="
> can be used in place of ":=" for assignment.

OK, here is an updated doc patch that does that.  The next question is
whether we want examples using '=' instead of ':='?  Right now we have
them, and Pavel's patch removed them.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

Bruce Momjian <bruce@momjian.us> writes:
> OK, here is an updated doc patch that does that.

You're still only touching the variable-initialization case, which seems
like the least important place to document it.  I'd have put it with
the assignment-statement documentation.  Perhaps something like

    <note>
     <para>
      Here, and in other contexts such as variable initialization,
      the assignment operator can be written <literal>=</> as well
      as <literal>:=</>.  The latter is the preferred spelling, though.
     </para>
    </note>

> The next question is
> whether we want examples using '=' instead of ':='?  Right now we have
> them, and Pavel's patch removed them.

AFAIR, we have some of both, and I'm fine with that status quo.

            regards, tom lane

On Sat, Jan 11, 2014 at 10:06:27PM +0100, Pavel Stehule wrote:
>
>
>
> 2014/1/11 Tom Lane <tgl@sss.pgh.pa.us>
>
>     Bruce Momjian <bruce@momjian.us> writes:
>     > Oh, I think you are right.  I have reverted the patch.  Attached is
>     > proposed documentation for '='.
>
>     Meh.  Variable initialization is only one of multiple cases (assignment,
>     GET DIAGNOSTICS; maybe others, I've not examined the grammar).  Also,
>     if we do it like this, we're implying that both := and = are equally
>     preferred, which might not be the impression we want to leave.
>
>
> GET DIAGNOSTICS is defined by standard - and there "=" should be allowed only -
> although we allow ":=" too. It is a embedded SQL statement - although it is
> implemented as plpgsql statement.

OK, docs updated for that.  I assume OPEN and FOR also can take := or =,
right?

> Same situation is with UPDATE statement - we don't allow ":=" there.
>  
>
>
>     I'd be a bit inclined to just stick a NOTE somewhere saying that "="
>     can be used in place of ":=" for assignment.
>
>
> ok
>
> If we accept it and we close this topic, then following comment should be
> removed
>
> assign_operator : '='   /* not documented because it might be removed someday *

Comment updated.  Patch attached.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Sun, Jan 12, 2014 at 06:38:04AM +0100, Pavel Stehule wrote:
>
>
>
> 2014/1/12 Bruce Momjian <bruce@momjian.us>
>
>     On Sat, Jan 11, 2014 at 10:06:27PM +0100, Pavel Stehule wrote:
>     >
>     >
>     >
>     > 2014/1/11 Tom Lane <tgl@sss.pgh.pa.us>
>     >
>     >     Bruce Momjian <bruce@momjian.us> writes:
>     >     > Oh, I think you are right.  I have reverted the patch.  Attached is
>     >     > proposed documentation for '='.
>     >
>     >     Meh.  Variable initialization is only one of multiple cases
>     (assignment,
>     >     GET DIAGNOSTICS; maybe others, I've not examined the grammar).  Also,
>     >     if we do it like this, we're implying that both := and = are equally
>     >     preferred, which might not be the impression we want to leave.
>     >
>     >
>     > GET DIAGNOSTICS is defined by standard - and there "=" should be allowed
>     only -
>     > although we allow ":=" too. It is a embedded SQL statement - although it
>     is
>     > implemented as plpgsql statement.
>
>     OK, docs updated for that.  I assume OPEN and FOR also can take := or =,
>     right?
>
>
> no, there are not used assign_operator
>
> It is used only in DECLARE DEFAULT, ASSIGN and GET DIAGNOSTICS
>  

OK, patch updated and attached.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Sun, Jan 12, 2014 at 07:00:09AM +0100, Pavel Stehule wrote:
>     Comment updated.  Patch attached.
>
>
>
> Still I am missing message about preferred syntax (or we should to enhance GET
> DIAGNOSTICS doc about proprietary syntax).
>
> PL/pgSQL, PL/SQL is mix of two languages: ADA and SQL - and their designers
> decided so embedded SQL statements will be placed in native SQL syntax (and ADA
> in simplified ADA syntax)
>
> We have to find a agreement what we will prefer for PL/pgSQL:
>
> The assign statement is "ADA" statement - ":=" is preferred
>
> The GET DIAGNOSTICS is "SQL" statement - "=" is preferred
>
> A newer SQL/PSM was designed differently - without this dichotomy - A
> assignment is emphased by syntax SET varname = value
>  
> A proper syntax is good information what is coming from - and It is good mental
> helper to understand a philosophy of stored procedures (that shares concepts
> with PL/SQL). Next argument is similarity with PL/SQL and SQL PL. This
> statement (GET DIAGNOSTICS) is supported on both environments and only "=" is
> allowed there.

I find the rules above so complex that I don't see how we can recommend
a best syntax.  What I did in the patch I just posted was to put the
preferred operator first, but I don't see why we want to get into these
details in the docs.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Mon, Jan 13, 2014 at 06:10:24AM +0100, Pavel Stehule wrote:
>     I find the rules above so complex that I don't see how we can recommend
>     a best syntax.  What I did in the patch I just posted was to put the
>     preferred operator first, but I don't see why we want to get into these
>     details in the docs.
>
>
> just a note about preferred variant should be ok. Using ':=' in GET DIAGNOSTICS
> statement is just bad idea.

Agreed.  I added a sentence in the first mention of GET DIAGNOSTICS, and
indicated it was generic for that command.  What I also added was your
description of when to use := and =, but I put it in the C comments so
we have a record of it and if we need to expand the description or
behavior later.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Mon, Jan 13, 2014 at 03:36:18PM +0100, Pavel Stehule wrote:
>     Agreed.  I added a sentence in the first mention of GET DIAGNOSTICS, and
>     indicated it was generic for that command.  What I also added was your
>     description of when to use := and =, but I put it in the C comments so
>     we have a record of it and if we need to expand the description or
>     behavior later.
>
>
> I'll be more happy if a comment about GET DIAGNOSTICS statement will be in user
> space.

I thought you would say that.  :-)  I don't see how this detail makes
sense in the sections related to the syntax usage, so I looked in the
section Porting from Oracle PL/SQL, and I don't see how it fits there
either.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Wed, Jan 15, 2014 at 11:07:29AM +0100, Pavel Stehule wrote:
>     I thought you would say that.  :-)  I don't see how this detail makes
>     sense in the sections related to the syntax usage, so I looked in the
>     section Porting from Oracle PL/SQL, and I don't see how it fits there
>     either.
>
>
>
> :)
>
> just notice - sorry for my English
>
> ==Assign==
> Using ":=" is preffered as assign statement due conformity with ADA language (a
> plpgsql ancestor).
>
>
> ==GET DIAGNOSTICS==
>
> Using "=" is highly preferred due conformity with ANSI/SQL

The problem is that these are philosophical issues that are not normally
covered in our docs.  What I have done is to add a mention of which
option is compliant to the new text.  Patch attached.

Is GET DIAGNOSTICS defined in the standard for SQL/PSM only or for
generic SQL?

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Wed, Jan 15, 2014 at 04:55:11PM +0100, Pavel Stehule wrote:
>     The problem is that these are philosophical issues that are not normally
>     covered in our docs.  What I have done is to add a mention of which
>     option is compliant to the new text.  Patch attached.
>
>     Is GET DIAGNOSTICS defined in the standard for SQL/PSM only or for
>     generic SQL?
>
>
>
> I found this statement in ANSI SQL 92 - and few minutes searching - it is in
> generic SQL - today "SQL framework" part and it is enhanced in "SQL/PSM"

OK, patch updated.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Wed, Jan 15, 2014 at 05:39:22PM +0100, Pavel Stehule wrote:
> I don't understand last sentence of comment
>
>  ! /*
> !  * Ada-based PL/SQL uses := for assignment and variable defaults, while
> !  * the SQL standard uses equals for these cases and for GET
> !  * DIAGNOSTICS, so we support both.  FOR and OPEN only support :=.
> !  */
> ! assign_operator    : '='
>
> "FOR and OPEN only support :="
>
> FOR statement nor OPEN statement doesn't use ":="

Uh, I see it in the documented syntax:

    OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional>
    <replaceable>argument_name</replaceable> := </optional>  <---
    <replaceable>argument_value

    FOR <replaceable>recordvar</replaceable> IN
    <replaceable>bound_cursorvar</replaceable> <optional> ( <optional>
    <replaceable>argument_name</replaceable> :=   <---
    </optional> <replaceable>argument_value

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

Bruce Momjian <bruce@momjian.us> writes:
> On Wed, Jan 15, 2014 at 05:39:22PM +0100, Pavel Stehule wrote:
> Uh, I see it in the documented syntax:

>     OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional>
>     <replaceable>argument_name</replaceable> := </optional>  <---
>     <replaceable>argument_value

It is there, if you look into read_cursor_args().

This appears to be the only place in pl_gram.y where we don't treat '='
and COLON_EQUALS interchangeably.

I was about to suggest that we should accept "=" here too, but on second
thought that could break existing code, if anyone's using boolean equality
expressions in cursor arguments.  So we'd better accept the inconsistency.

            regards, tom lane

On Wed, Jan 15, 2014 at 11:32:59AM -0500, Bruce Momjian wrote:
> On Wed, Jan 15, 2014 at 04:55:11PM +0100, Pavel Stehule wrote:
> >     The problem is that these are philosophical issues that are not normally
> >     covered in our docs.  What I have done is to add a mention of which
> >     option is compliant to the new text.  Patch attached.
> >
> >     Is GET DIAGNOSTICS defined in the standard for SQL/PSM only or for
> >     generic SQL?
> >
> >
> >
> > I found this statement in ANSI SQL 92 - and few minutes searching - it is in
> > generic SQL - today "SQL framework" part and it is enhanced in "SQL/PSM"
>
> OK, patch updated.

Patch applied.  Yeah!

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +