Обсуждение: Regression in COPY FROM caused by 9f8377f7a2

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

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

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

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

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

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

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

Yours,
Laurenz Albe

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

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

Thanks for taking care of that.

Yours,
Laurenz Albe