Horton

Thread

Commits

  1. Fix INSERT OVERRIDING USER VALUE behavior

  1. INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2019-02-22T14:12:55Z

    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

  2. Re: INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-02-25T12:47:41Z

    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

  3. Re: INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2019-02-25T14:36:30Z

    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

  4. Re: INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-03-27T11:29:23Z

    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

  5. Re: INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2020-03-27T16:33:34Z

    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

  6. Re: INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns

    Vik Fearing <vik@postgresfriends.org> — 2020-03-27T16:58:04Z

    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

  7. Re: INSERT ... OVERRIDING USER VALUE vs GENERATED ALWAYS identity columns

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-03-31T06:51:44Z

    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