Shinoda-san,
Thanks a lot for testing.
On 2017/03/30 10:30, Shinoda, Noriyoshi wrote:
> Hello,
>
> I tried this feature using most recently snapshot. In case of added constraint PRIMARY KEY for partition table,
INSERTON CONFLICT DO NOTHING statement failed with segmentaion fault.
> If the primary key constraint was not created on the partition, this statement executed successfully.
>
> - Test
> postgres=> CREATE TABLE part1(c1 NUMERIC, c2 VARCHAR(10)) PARTITION BY RANGE (c1) ;
> CREATE TABLE
> postgres=> CREATE TABLE part1p1 PARTITION OF part1 FOR VALUES FROM (100) TO (200) ;
> CREATE TABLE
> postgres=> ALTER TABLE part1p1 ADD CONSTRAINT pk_part1p1 PRIMARY KEY (c1) ;
> ALTER TABLE
> postgres=> INSERT INTO part1 VALUES (100, 'init') ON CONFLICT DO NOTHING ;
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> !> \q
I found out why the crash occurs, but while I was trying to fix it, I
started growing doubtful about the way this is being handled currently.
Patch to fix the crash would be to pass 'true' instead of 'false' for
speculative when ExecSetupPartitionTupleRouting() calls ExecOpenIndices()
on leaf partitions. That will initialize the information needed when
ExecInsert() wants check for conflicts using the constraint-enforcing
indexes. If we do initialize the speculative insertion info (which will
fix the crash), ExecCheckIndexConstraints() will be called on a given leaf
partition's index to check if there is any conflict. But since the insert
was performed on the root table, conflicts should be checked across all
the partitions, which won't be the case. Even though the action is
NOTHING, the check for conflicts still uses only that one leaf partition's
index, which seems insufficient.
Commit 8355a011a0 enabled specifying ON CONFLICT DO NOTHING on when
inserting into a partitioned root table, but given the above, I think we
might need to reconsider it.
Thanks,
Amit
