Обсуждение: INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns
So I started looking into the bug noted in [1], but before getting to
multi-row inserts, I concluded that the current single-row behaviour
isn't spec-compliant.
In particular, Syntax Rule 11b of section 14.11 says that an INSERT
statement on a GENERATED ALWAYS identity column must specify an
overriding clause, but it doesn't place any restriction on the type of
overriding clause allowed. In other words it should be possible to use
either OVERRIDING SYSTEM VALUE or OVERRIDING USER VALUE, but we
currently throw an error unless it's the former.
It's useful to allow OVERRIDING USER VALUE for precisely the example
use-case given in the INSERT docs:
This clause is useful for example when copying values between tables.
Writing <literal>INSERT INTO tbl2 OVERRIDING USER VALUE SELECT * FROM
tbl1</literal> will copy from <literal>tbl1</literal> all columns that
are not identity columns in <literal>tbl2</literal> while values for
the identity columns in <literal>tbl2</literal> will be generated by
the sequences associated with <literal>tbl2</literal>.
which currently only works for a GENERATED BY DEFAULT identity column,
but should work equally well for a GENERATED ALWAYS identity column.
So I propose the attached patch.
Regards,
Dean
[1] https://postgr.es/m/CAEZATCUmSp3-8nLOpgGcPkpUEXK9TJGM%3DiA6q4E2Sn%3D%2BbwkKNA%40mail.gmail.com
Вложения
On 2019-02-22 15:12, Dean Rasheed wrote:
> In particular, Syntax Rule 11b of section 14.11 says that an INSERT
> statement on a GENERATED ALWAYS identity column must specify an
> overriding clause, but it doesn't place any restriction on the type of
> overriding clause allowed. In other words it should be possible to use
> either OVERRIDING SYSTEM VALUE or OVERRIDING USER VALUE, but we
> currently throw an error unless it's the former.
It appears you are right.
> - and the column in new rows will automatically have values from the
> - sequence assigned to it.
> + and new rows in the column will automatically have values from the
> + sequence assigned to them.
The "it" refers to "the column", so I think it's correct.
> specifies <literal>OVERRIDING SYSTEM VALUE</literal>. If
<literal>BY
> DEFAULT</literal> is specified, then the user-specified value takes
> - precedence. See <xref linkend="sql-insert"/> for details. (In
> + precedence, unless the <command>INSERT</command> statement
specifies
> + <literal>OVERRIDING USER VALUE</literal>.
> + See <xref linkend="sql-insert"/> for details. (In
Isn't your change that it now applies to both ALWAYS and BY DEFAULT? So
why attach this phrase to the BY DEFAULT explanation?
> <para>
> + Additionally, if <literal>ALWAYS</literal> is specified, any
attempt to
> + update the value of the column using an <command>UPDATE</command>
> + statement specifying any value other than
<literal>DEFAULT</literal>
> + will be rejected. If <literal>BY DEFAULT</literal> is
specified, the
> + system will allow values in the column to be updated.
> + </para>
This is already documented on the INSERT reference page.
> - errhint("Use OVERRIDING SYSTEM VALUE to override.")));
> + errhint("You must specify either OVERRIDING SYSTEM VALUE or
OVERRIDING USER VALUE.")));
Is this a good hint? If the user wanted to insert something, then
specifying OVERRIDING USER VALUE won't really accomplish that.
OVERRIDING USER VALUE is only useful in the specific situations that the
documentation discussed. Can we detect those?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, 25 Feb 2019 at 12:47, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> > - and the column in new rows will automatically have values from the
> > - sequence assigned to it.
> > + and new rows in the column will automatically have values from the
> > + sequence assigned to them.
>
> The "it" refers to "the column", so I think it's correct.
>
Ah, OK. I failed to parse the original wording.
> > specifies <literal>OVERRIDING SYSTEM VALUE</literal>. If
> <literal>BY
> > DEFAULT</literal> is specified, then the user-specified value takes
> > - precedence. See <xref linkend="sql-insert"/> for details. (In
> > + precedence, unless the <command>INSERT</command> statement
> specifies
> > + <literal>OVERRIDING USER VALUE</literal>.
> > + See <xref linkend="sql-insert"/> for details. (In
>
> Isn't your change that it now applies to both ALWAYS and BY DEFAULT? So
> why attach this phrase to the BY DEFAULT explanation?
>
The last couple of sentences of that paragraph are describing the
circumstances under which the user-specified value will be applied. So
for the ALWAYS case, it's only if OVERRIDING SYSTEM VALUE is
specified, and for the BY DEFAULT case, it's only if OVERRIDING USER
VALUE isn't specified. Without that additional text, the original
wording could be taken to mean that for a BY DEFAULT column, the
user-specified value always gets applied.
> > <para>
> > + Additionally, if <literal>ALWAYS</literal> is specified, any
> attempt to
> > + update the value of the column using an <command>UPDATE</command>
> > + statement specifying any value other than
> <literal>DEFAULT</literal>
> > + will be rejected. If <literal>BY DEFAULT</literal> is
> specified, the
> > + system will allow values in the column to be updated.
> > + </para>
>
> This is already documented on the INSERT reference page.
>
I can't see anywhere where we document how UPDATE behaves with identity columns.
> > - errhint("Use OVERRIDING SYSTEM VALUE to override.")));
> > + errhint("You must specify either OVERRIDING SYSTEM VALUE or
> OVERRIDING USER VALUE.")));
>
> Is this a good hint? If the user wanted to insert something, then
> specifying OVERRIDING USER VALUE won't really accomplish that.
> OVERRIDING USER VALUE is only useful in the specific situations that the
> documentation discussed. Can we detect those?
>
Hmm, I'm not sure that we reliably guess what the user intended. What
exactly did you have in mind?
Regards,
Dean
We appear to have lost track of this. I have re-read everything and expanded your patch a bit with additional documentation and comments in the tests. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On Fri, 27 Mar 2020 at 11:29, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > > We appear to have lost track of this. Ah yes, indeed! > I have re-read everything and > expanded your patch a bit with additional documentation and comments in > the tests. I looked that over, and it all looks good to me. Regards, Dean
On 3/27/20 9:33 AM, Dean Rasheed wrote: > On Fri, 27 Mar 2020 at 11:29, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> wrote: >> >> I have re-read everything and >> expanded your patch a bit with additional documentation and comments in >> the tests. > > I looked that over, and it all looks good to me. I concur. And it matches my reading of the standard (apart from the intentional derivation). -- Vik Fearing
On 2020-03-27 17:58, Vik Fearing wrote: > On 3/27/20 9:33 AM, Dean Rasheed wrote: >> On Fri, 27 Mar 2020 at 11:29, Peter Eisentraut >> <peter.eisentraut@2ndquadrant.com> wrote: >>> >>> I have re-read everything and >>> expanded your patch a bit with additional documentation and comments in >>> the tests. >> >> I looked that over, and it all looks good to me. > > I concur. And it matches my reading of the standard (apart from the > intentional derivation). Committed, thanks! -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services