Horton

Thread

Commits

  1. Only evaluate default values as required when doing COPY FROM

  2. Add a DEFAULT option to COPY FROM

  1. Regression in COPY FROM caused by 9f8377f7a2

    Laurenz Albe <laurenz.albe@cybertec.at> — 2023-09-25T07:54:22Z

    In v16 and later, the following fails:

CREATE TABLE boom (t character varying(5) DEFAULT 'a long string');

COPY boom FROM STDIN;
ERROR:  value too long for type character varying(5)

In PostgreSQL v15 and earlier, the COPY statement succeeds.

The error is thrown in BeginCopyFrom in line 1578 (HEAD)

  defexpr = expression_planner(defexpr);

Bisecting shows that the regression was introduced by commit 9f8377f7a2,
which introduced DEFAULT values for COPY FROM.

The table definition is clearly silly, so I am not sure if that
regression is worth fixing.  On the other hand, it is not cool if
something that worked without an error in v15 starts to fail later on.

Yours,
Laurenz Albe

  2. Re: Regression in COPY FROM caused by 9f8377f7a2

    Laurenz Albe <laurenz.albe@cybertec.at> — 2023-09-25T08:59:21Z

    On Mon, 2023-09-25 at 09:54 +0200, Laurenz Albe wrote:
> In v16 and later, the following fails:
> 
> CREATE TABLE boom (t character varying(5) DEFAULT 'a long string');
> 
> COPY boom FROM STDIN;
> ERROR:  value too long for type character varying(5)
> 
> In PostgreSQL v15 and earlier, the COPY statement succeeds.
> 
> The error is thrown in BeginCopyFrom in line 1578 (HEAD)
> 
>   defexpr = expression_planner(defexpr);
> 
> Bisecting shows that the regression was introduced by commit 9f8377f7a2,
> which introduced DEFAULT values for COPY FROM.

I suggest the attached fix, which evaluates default values only if
the DEFAULT option was specified or if the column does not appear in
the column list of COPY.

Yours,
Laurenz Albe

  3. Re: Regression in COPY FROM caused by 9f8377f7a2

    Andrew Dunstan <andrew@dunslane.net> — 2023-09-25T15:06:58Z

    On 2023-09-25 Mo 04:59, Laurenz Albe wrote:
> On Mon, 2023-09-25 at 09:54 +0200, Laurenz Albe wrote:
>> In v16 and later, the following fails:
>>
>> CREATE TABLE boom (t character varying(5) DEFAULT 'a long string');
>>
>> COPY boom FROM STDIN;
>> ERROR:  value too long for type character varying(5)
>>
>> In PostgreSQL v15 and earlier, the COPY statement succeeds.
>>
>> The error is thrown in BeginCopyFrom in line 1578 (HEAD)
>>
>>    defexpr = expression_planner(defexpr);
>>
>> Bisecting shows that the regression was introduced by commit 9f8377f7a2,
>> which introduced DEFAULT values for COPY FROM.


Oops :-(


> I suggest the attached fix, which evaluates default values only if
> the DEFAULT option was specified or if the column does not appear in
> the column list of COPY.
>

Patch looks reasonable, haven't tested yet.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

  4. Re: Regression in COPY FROM caused by 9f8377f7a2

    Andrew Dunstan <andrew@dunslane.net> — 2023-09-25T20:52:40Z

    On 2023-09-25 Mo 11:06, Andrew Dunstan wrote:
>
>
> On 2023-09-25 Mo 04:59, Laurenz Albe wrote:
>> On Mon, 2023-09-25 at 09:54 +0200, Laurenz Albe wrote:
>>> In v16 and later, the following fails:
>>>
>>> CREATE TABLE boom (t character varying(5) DEFAULT 'a long string');
>>>
>>> COPY boom FROM STDIN;
>>> ERROR:  value too long for type character varying(5)
>>>
>>> In PostgreSQL v15 and earlier, the COPY statement succeeds.
>>>
>>> The error is thrown in BeginCopyFrom in line 1578 (HEAD)
>>>
>>>    defexpr = expression_planner(defexpr);
>>>
>>> Bisecting shows that the regression was introduced by commit 9f8377f7a2,
>>> which introduced DEFAULT values for COPY FROM.
>
>


Thinking about this a little more, wouldn't it be better if we checked 
at the time we set the default that the value is actually valid for the 
given column? This is only one manifestation of a problem you could run 
into given this table definition.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

  5. Re: Regression in COPY FROM caused by 9f8377f7a2

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-09-25T21:49:42Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> On 2023-09-25 Mo 11:06, Andrew Dunstan wrote:
>> On 2023-09-25 Mo 04:59, Laurenz Albe wrote:
>>> CREATE TABLE boom (t character varying(5) DEFAULT 'a long string');

> Thinking about this a little more, wouldn't it be better if we checked 
> at the time we set the default that the value is actually valid for the 
> given column? This is only one manifestation of a problem you could run 
> into given this table definition.

I dunno, it seems at least possible that someone would do this
deliberately as a means of preventing the column from being defaulted.
In any case, the current behavior has stood for a very long time and
no one has complained that an error should be thrown sooner.

			regards, tom lane

  6. Re: Regression in COPY FROM caused by 9f8377f7a2

    Laurenz Albe <laurenz.albe@cybertec.at> — 2023-09-26T06:48:31Z

    On Mon, 2023-09-25 at 17:49 -0400, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > On 2023-09-25 Mo 11:06, Andrew Dunstan wrote:
> > > On 2023-09-25 Mo 04:59, Laurenz Albe wrote:
> > > > CREATE TABLE boom (t character varying(5) DEFAULT 'a long string');
> 
> > Thinking about this a little more, wouldn't it be better if we checked 
> > at the time we set the default that the value is actually valid for the 
> > given column? This is only one manifestation of a problem you could run 
> > into given this table definition.
> 
> I dunno, it seems at least possible that someone would do this
> deliberately as a means of preventing the column from being defaulted.
> In any case, the current behavior has stood for a very long time and
> no one has complained that an error should be thrown sooner.

Moreover, this makes restoring a pg_dump from v15 to v16 fail, which
should never happen.  This is how I got that bug report.

Yours,
Laurenz Albe

  7. Re: Regression in COPY FROM caused by 9f8377f7a2

    Laurenz Albe <laurenz.albe@cybertec.at> — 2023-09-26T08:11:25Z

    Here is an improved version of the patch with regression tests.

Yours,
Laurenz Albe

  8. Re: Regression in COPY FROM caused by 9f8377f7a2

    Andrew Dunstan <andrew@dunslane.net> — 2023-10-01T14:55:33Z

    On 2023-09-26 Tu 04:11, Laurenz Albe wrote:
> Here is an improved version of the patch with regression tests.
>

Thanks, pushed.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  9. Re: Regression in COPY FROM caused by 9f8377f7a2

    Laurenz Albe <laurenz.albe@cybertec.at> — 2023-10-02T04:10:24Z

    On Sun, 2023-10-01 at 10:55 -0400, Andrew Dunstan wrote:
> Thanks, pushed.

Thanks for taking care of that.

Yours,
Laurenz Albe