Horton

Thread

  1. Fix bug of COPY (on_error set_null)

    Chao Li <li.evan.chao@gmail.com> — 2026-05-15T01:12:35Z

    Hi,

I just tested “Add COPY (column list) (on_error set_null) option”. While tracing a normal case, I found a mistake:

In BeginCopyFrom(), cstate->domain_with_constraint is allocated using the length of the specified column list, and set using the index in that column list:
```
		int			attr_count = list_length(cstate->attnumlist);

		/*
		 * When data type conversion fails and ON_ERROR is SET_NULL, we need
		 * ensure that the input column allow null values.  ExecConstraints()
		 * will cover most of the cases, but it does not verify domain
		 * constraints.  Therefore, for constrained domains, the null value
		 * check must be performed during the initial string-to-datum
		 * conversion (see CopyFromTextLikeOneRow()).
		 */
		cstate->domain_with_constraint = palloc0_array(bool, attr_count); <== allocate with length of column list from SQL

		foreach_int(attno, cstate->attnumlist)
		{
			int			i = foreach_current_index(attno);

			Form_pg_attribute att = TupleDescAttr(tupDesc, attno - 1);

			cstate->domain_with_constraint[i] = DomainHasConstraints(att->atttypid, NULL); <= set with index of column list from SQL
		}
```

However, cstate->domain_with_constraint is read in CopyFromTextLikeOneRow(), where it is accessed using the actual attribute number:
```
	/* Loop to read the user attributes on the line. */
	foreach(cur, cstate->attnumlist)
	{
		int			attnum = lfirst_int(cur);
		int			m = attnum - 1;

	...
				if (!cstate->domain_with_constraint[m] ||
```

So, if the column list specified in SQL is shorter than the table’s actual attribute list, this may cause an out-of-bounds read.

Based on this finding, it was easy to create a repro:
```
evantest=# CREATE DOMAIN dnn AS int NOT NULL;
CREATE DOMAIN
evantest=# CREATE TABLE t (a int, b dnn);
CREATE TABLE
evantest=# COPY t(b) FROM STDIN WITH (on_error set_null);
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> x
>> \.
NOTICE:  in 1 row, columns were set to null due to data type incompatibility
COPY 1
evantest=# select * from t;
 a | b
---+---
   |
(1 row)
```

Here, the table has two columns, and the COPY command only specifies column b. Since I gave invalid input for b, COPY tries to set it to NULL. However, b is a NOT NULL domain, so the COPY should fail. Instead, a row with NULL in b is successfully inserted, which proves this is a real bug.

To fix the bug, there are two options:

1. In BeginCopyFrom(), allocate cstate->domain_with_constraint using the actual number of table attributes, and set it by actual attribute number.
2. In CopyFromTextLikeOneRow(), access cstate->domain_with_constraint using the index in the SQL column list.

I chose option 1, because it keeps cstate->domain_with_constraint in the same form as other arrays such as cstate->typioparams[m], which makes the code easier to read. This may use a little extra memory, but since cstate->domain_with_constraint is a small boolean array and is short-lived, I don’t think that matters much.

While here, I also noticed a few palloc calls that could be switched to palloc_array, so I changed them as well.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  2. Re: Fix bug of COPY (on_error set_null)

    jian he <jian.universality@gmail.com> — 2026-05-16T05:17:24Z

    On Fri, May 15, 2026 at 9:13 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi,
>
> I just tested “Add COPY (column list) (on_error set_null) option”. While tracing a normal case, I found a mistake:
>
> In BeginCopyFrom(), cstate->domain_with_constraint is allocated using the length of the specified column list, and set using the index in that column list:
> ```
>                 int                     attr_count = list_length(cstate->attnumlist);
>
>                 /*
>                  * When data type conversion fails and ON_ERROR is SET_NULL, we need
>                  * ensure that the input column allow null values.  ExecConstraints()
>                  * will cover most of the cases, but it does not verify domain
>                  * constraints.  Therefore, for constrained domains, the null value
>                  * check must be performed during the initial string-to-datum
>                  * conversion (see CopyFromTextLikeOneRow()).
>                  */
>                 cstate->domain_with_constraint = palloc0_array(bool, attr_count); <== allocate with length of column list from SQL
>
>                 foreach_int(attno, cstate->attnumlist)
>                 {
>                         int                     i = foreach_current_index(attno);
>
>                         Form_pg_attribute att = TupleDescAttr(tupDesc, attno - 1);
>
>                         cstate->domain_with_constraint[i] = DomainHasConstraints(att->atttypid, NULL); <= set with index of column list from SQL
>                 }
> ```
>
> However, cstate->domain_with_constraint is read in CopyFromTextLikeOneRow(), where it is accessed using the actual attribute number:
> ```
>         /* Loop to read the user attributes on the line. */
>         foreach(cur, cstate->attnumlist)
>         {
>                 int                     attnum = lfirst_int(cur);
>                 int                     m = attnum - 1;
>
>         ...
>                                 if (!cstate->domain_with_constraint[m] ||
> ```
>
> So, if the column list specified in SQL is shorter than the table’s actual attribute list, this may cause an out-of-bounds read.
>
Hi.

This appears to be the same issue as reported here:
https://postgr.es/m/CAHg+QDdej0c0gWJi2FnbirzhgzyZNPiTwC1P5B_-dSNCzq-91A@mail.gmail.com

  3. Re: Fix bug of COPY (on_error set_null)

    Chao Li <li.evan.chao@gmail.com> — 2026-05-18T22:22:15Z

    
> On May 16, 2026, at 13:17, jian he <jian.universality@gmail.com> wrote:
> 
> On Fri, May 15, 2026 at 9:13 AM Chao Li <li.evan.chao@gmail.com> wrote:
>> 
>> Hi,
>> 
>> I just tested “Add COPY (column list) (on_error set_null) option”. While tracing a normal case, I found a mistake:
>> 
>> In BeginCopyFrom(), cstate->domain_with_constraint is allocated using the length of the specified column list, and set using the index in that column list:
>> ```
>>                int                     attr_count = list_length(cstate->attnumlist);
>> 
>>                /*
>>                 * When data type conversion fails and ON_ERROR is SET_NULL, we need
>>                 * ensure that the input column allow null values.  ExecConstraints()
>>                 * will cover most of the cases, but it does not verify domain
>>                 * constraints.  Therefore, for constrained domains, the null value
>>                 * check must be performed during the initial string-to-datum
>>                 * conversion (see CopyFromTextLikeOneRow()).
>>                 */
>>                cstate->domain_with_constraint = palloc0_array(bool, attr_count); <== allocate with length of column list from SQL
>> 
>>                foreach_int(attno, cstate->attnumlist)
>>                {
>>                        int                     i = foreach_current_index(attno);
>> 
>>                        Form_pg_attribute att = TupleDescAttr(tupDesc, attno - 1);
>> 
>>                        cstate->domain_with_constraint[i] = DomainHasConstraints(att->atttypid, NULL); <= set with index of column list from SQL
>>                }
>> ```
>> 
>> However, cstate->domain_with_constraint is read in CopyFromTextLikeOneRow(), where it is accessed using the actual attribute number:
>> ```
>>        /* Loop to read the user attributes on the line. */
>>        foreach(cur, cstate->attnumlist)
>>        {
>>                int                     attnum = lfirst_int(cur);
>>                int                     m = attnum - 1;
>> 
>>        ...
>>                                if (!cstate->domain_with_constraint[m] ||
>> ```
>> 
>> So, if the column list specified in SQL is shorter than the table’s actual attribute list, this may cause an out-of-bounds read.
>> 
> Hi.
> 
> This appears to be the same issue as reported here:
> https://postgr.es/m/CAHg+QDdej0c0gWJi2FnbirzhgzyZNPiTwC1P5B_-dSNCzq-91A@mail.gmail.com

Thanks for pointing out that. I will review that patch.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/