Thread: 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
Attachment
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
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
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
Attachment
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