Re: INSERT ... ON CONFLICT {UPDATE | IGNORE}

From: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
To: Peter Geoghegan <pg(at)heroku(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Anssi Kääriäinen <anssi(dot)kaariainen(at)thl(dot)fi>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Kevin Grittner <kgrittn(at)ymail(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: INSERT ... ON CONFLICT {UPDATE | IGNORE}
Date: 2014-12-18 17:20:36
Message-ID: CAMkU=1zYuNU1ciBG6rRabV5r7k9iur-n7-Fdyi9hU3C98HvPGw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Dec 15, 2014 at 11:06 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>
> On Mon, Dec 15, 2014 at 4:59 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> > On Mon, Dec 15, 2014 at 4:22 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
> wrote:
> >> Also, in both Linux and MinGW under option 1 patch I get an OID
> conflict on
> >> OID 3261.
> >
> > I'll take a pass at fixing this bitrot soon. I'll follow Tom's advice
> > about macro collisions on MinGW while I'm at it, since his explanation
> > seems plausible.
>
> Attached pair of revised patch sets fix the OID collision, and
> presumably fix the MinGW issue (because IGNORE_P is now used as a
> token name). It also polishes approach #2 to value locking in a few
> places (e.g. better comments). Finally, both patches have a minor
> buglet around EXPLAIN ANALYZE output fixed -- the output now indicates
> if tuples are pulled up from auxiliary update nodes.
>

I naively tried this in vallock1 patch:

create table foo(index int, count int);

create unique index on foo(index);

insert into foo (index, count) values (0,1) on conflict (index) update set
count=foo.count + 1 returning foo.count;

insert into foo (index, count) values (0,1) on conflict (index) update set
count=foo.count + 1 returning foo.count;

insert into foo (index, count) values (0,1) on conflict (index) update set
count=foo.count + 1 returning foo.count;

insert into foo (index, count) values (0,1) on conflict (index) update set
count=foo.count + 1 returning foo.count;

After actually reading the documentation more closely, I decided this
should be an error because "foo" is not a valid table alias in the "update
set" expression. Instead of being a parsing/planning error, this executes
and the foo.count on the RHS of the assignment always evaluates as zero
(even on subsequent invocations when TARGET.count is 1).

If I switch to a text type, then I get seg faults under the same condition:

create table foo(index int, count text);
create unique index on foo(index);
insert into foo (index, count) values (0,'start ') on conflict (index)
update set count=foo.count||' bar' returning count;
insert into foo (index, count) values (0,'start ') on conflict (index)
update set count=foo.count||' bar' returning count;
<boom>

#0 pg_detoast_datum_packed (datum=0x0) at fmgr.c:2270
#1 0x000000000074fb7a in textcat (fcinfo=0x1e67a78) at varlena.c:662
#2 0x00000000005a63a5 in ExecMakeFunctionResultNoSets (fcache=0x1e67a08,
econtext=0x1e67848, isNull=0x1e68b11 "", isDone=<value optimized out>)
at execQual.c:2026
#3 0x00000000005a2353 in ExecTargetList (projInfo=<value optimized out>,
isDone=0x7fffa7fa346c) at execQual.c:5358
#4 ExecProject (projInfo=<value optimized out>, isDone=0x7fffa7fa346c) at
execQual.c:5573
#5 0x00000000005a86c2 in ExecScan (node=0x1e67738, accessMtd=0x5baf00
<SeqNext>, recheckMtd=0x5bad60 <SeqRecheck>) at execScan.c:207
#6 0x00000000005a1918 in ExecProcNode (node=0x1e67738) at
execProcnode.c:406
#7 0x000000000059ef32 in EvalPlanQualNext (epqstate=<value optimized out>)
at execMain.c:2380
#8 0x00000000005b8fcd in ExecLockUpdateTuple (node=0x1e5f750) at
nodeModifyTable.c:1098
#9 ExecInsert (node=0x1e5f750) at nodeModifyTable.c:372
#10 ExecModifyTable (node=0x1e5f750) at nodeModifyTable.c:1396
#11 0x00000000005a1958 in ExecProcNode (node=0x1e5f750) at
execProcnode.c:383
#12 0x00000000005a0642 in ExecutePlan (queryDesc=0x1dd0908,
direction=<value optimized out>, count=0) at execMain.c:1515
#13 standard_ExecutorRun (queryDesc=0x1dd0908, direction=<value optimized
out>, count=0) at execMain.c:308
#14 0x00007f601416b9cb in pgss_ExecutorRun (queryDesc=0x1dd0908,
direction=ForwardScanDirection, count=0) at pg_stat_statements.c:874
#15 0x000000000069385f in ProcessQuery (plan=0x1e47df0,
....

So I think there needs to be some kind of logic to de-recognize the table
alias "foo".

Once I rewrote the query to use TARGET and EXCLUDED correctly, I've put
this through an adaptation of my usual torture test, and it ran fine until
wraparound shutdown. I'll poke at it more later.

Cheers,

Jeff

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2014-12-18 17:27:15 Re: exitArchiveRecovery woes
Previous Message Mark Dilger 2014-12-18 17:16:56 Re: WIP patch for Oid formatting in printf/elog strings