Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-30 Thread Robert Haas
On Tue, Jul 29, 2014 at 1:28 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jul 29, 2014 at 9:56 AM, Robert Haas robertmh...@gmail.com wrote: I think it would be advisable to separate the syntax from the implementation. Presumably you can make your implementation use some reasonable

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-30 Thread Peter Geoghegan
On Wed, Jul 30, 2014 at 10:36 AM, Robert Haas robertmh...@gmail.com wrote: You've convinced me that only indexable predicates can be sensibly used here. I'm not sure that's the same as saying that upserts are driven by inserts, though. I'd tend to interpret that to mean

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-30 Thread Bruce Momjian
On Mon, Jul 28, 2014 at 11:37:07AM -0400, Robert Haas wrote: Yes, but what if you don't see a conflict because it isn't visible to your snapshot, and then you insert, and only then (step 5), presumably with a dirty snapshot, you find a conflict? How does the loop terminate if that brings

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-29 Thread Robert Haas
On Mon, Jul 28, 2014 at 1:43 PM, Peter Geoghegan p...@heroku.com wrote: On Mon, Jul 28, 2014 at 8:37 AM, Robert Haas robertmh...@gmail.com wrote: AFAIUI, this is because your implementation uses lwlocks in a way that Andres and I both find unacceptable. That's not the case. My implementation

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-29 Thread Peter Geoghegan
On Tue, Jul 29, 2014 at 9:56 AM, Robert Haas robertmh...@gmail.com wrote: I think it would be advisable to separate the syntax from the implementation. Presumably you can make your implementation use some reasonable syntax we can all agree on, and conversely my proposed syntax could be made

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-28 Thread Robert Haas
On Wed, Jul 23, 2014 at 7:32 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Because nobody wants an operation to either insert 1 tuple or update n=1 tuples. The intention is that the predicate should probably be something like WHERE unique_key = 'some_value', but you can use something

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-28 Thread Robert Haas
On Wed, Jul 23, 2014 at 7:35 PM, Peter Geoghegan p...@heroku.com wrote: It's certain arguable whether you should INSERT and then turn failures into an update or try to UPDATE and then turn failures into an INSERT; we might even want to have both options available, though that smells a little

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-28 Thread Peter Geoghegan
On Mon, Jul 28, 2014 at 8:37 AM, Robert Haas robertmh...@gmail.com wrote: AFAIUI, this is because your implementation uses lwlocks in a way that Andres and I both find unacceptable. That's not the case. My implementation uses page-level heavyweight locks. The nbtree AM used to use them for

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-28 Thread Peter Geoghegan
On Mon, Jul 28, 2014 at 10:43 AM, Peter Geoghegan p...@heroku.com wrote: Plus, I ask you to consider that. Excuse me. I meant Plus, you avoid bloat. I ask you to consider that. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-28 Thread Peter Geoghegan
On Mon, Jul 28, 2014 at 10:43 AM, Peter Geoghegan p...@heroku.com wrote: On a mostly unrelated note, I'll remind you of the reason that I felt it was best to lock indexes. It wasn't so much about avoiding bloat as it was about avoiding deadlocks. When I highlighted the issue, Heikki's

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Robert Haas
On Sat, Jul 19, 2014 at 10:04 PM, Peter Geoghegan p...@heroku.com wrote: On Fri, Jul 18, 2014 at 11:23 AM, Andres Freund and...@2ndquadrant.com wrote: Meh. A understandable syntax wouldn't require the pullups with a special scan node and such. Well, in general ExecModifyTable()/ExecUpdate()

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Peter Geoghegan
On Wed, Jul 23, 2014 at 9:58 AM, Robert Haas robertmh...@gmail.com wrote: This all seems completely to one side of Andres's point. I think what he's saying is: don't implement an SQL syntax of the form INSERT ON CONFLICT and let people use that to implement upsert. Instead, directly

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Greg Stark
On Wed, Jul 23, 2014 at 5:58 PM, Robert Haas robertmh...@gmail.com wrote: I'd suggest something like: UPSERT table SET col = value [, ...] WHERE predicate; I don't think this is actually good enough. Typical use cases are things like increment this counter or insert a new one starting at 0.

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Kevin Grittner
Peter Geoghegan p...@heroku.com wrote: I still strongly feel it ought to be driven by an insert Could you clarify that?  Does this mean that you feel that we should write to the heap before reading the index to see if the row will be a duplicate?  If so, I think that is a bad idea, since this

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Robert Haas
On Wed, Jul 23, 2014 at 4:05 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Jul 23, 2014 at 9:58 AM, Robert Haas robertmh...@gmail.com wrote: This all seems completely to one side of Andres's point. I think what he's saying is: don't implement an SQL syntax of the form INSERT ON CONFLICT

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread David G Johnston
Peter Geoghegan-3 wrote with semantics like this: 1. Search the table, using any type of scan you like, for a row matching the given predicate. Perhaps I've misunderstood, but this is fundamentally different to what I'd always thought would need to happen. I always understood that the

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Peter Geoghegan
On Wed, Jul 23, 2014 at 3:01 PM, Kevin Grittner kgri...@ymail.com wrote: Could you clarify that? Does this mean that you feel that we should write to the heap before reading the index to see if the row will be a duplicate? If so, I think that is a bad idea, since this will sometimes be used

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Alvaro Herrera
Robert Haas wrote: On Wed, Jul 23, 2014 at 4:05 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Jul 23, 2014 at 9:58 AM, Robert Haas robertmh...@gmail.com wrote: 2. If you find more than one tuple that is visible to your scan, error. This point seems to concern making the UPSERT

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Peter Geoghegan
On Wed, Jul 23, 2014 at 3:27 PM, Robert Haas robertmh...@gmail.com wrote: To the last question, yes. To the first point, I'm not bent on this particular syntax, but I am +1 for the idea that the command is something specifically upsert-like rather than something more generic like an ON

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-23 Thread Peter Geoghegan
On Wed, Jul 23, 2014 at 9:58 AM, Robert Haas robertmh...@gmail.com wrote: I'd suggest something like: UPSERT table SET col = value [, ...] WHERE predicate; I think I see another problem with this. The UPDATE must provide a WHERE clause Var on a unique indexed column (let's say it's constrained

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-19 Thread Peter Geoghegan
On Fri, Jul 18, 2014 at 11:23 AM, Andres Freund and...@2ndquadrant.com wrote: Meh. A understandable syntax wouldn't require the pullups with a special scan node and such. Well, in general ExecModifyTable()/ExecUpdate() trusts the tid passed to match the qual in the query. Unless you're willing

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-18 Thread Andres Freund
Hi, On 2014-07-17 18:18:41 -0700, Peter Geoghegan wrote: I'm working on UPSERT again. I think that in order to make useful progress, I'll have to find a better way of overcoming the visibility issues (i.e. the problem of what to do about a still-in-progress-to-our-snapshot row being locked at

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-18 Thread Peter Geoghegan
On Fri, Jul 18, 2014 at 10:46 AM, Andres Freund and...@2ndquadrant.com wrote: I think the things that use wierd visibility semantics are pretty much all doing that internally (things being EvalPlanQual stuff for INSERT/UPDATE/DELETE and the referential integrity triggers). I don't see

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-18 Thread Andres Freund
On 2014-07-18 10:53:36 -0700, Peter Geoghegan wrote: On Fri, Jul 18, 2014 at 10:46 AM, Andres Freund and...@2ndquadrant.com wrote: I think the things that use wierd visibility semantics are pretty much all doing that internally (things being EvalPlanQual stuff for INSERT/UPDATE/DELETE and

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-18 Thread Peter Geoghegan
On Fri, Jul 18, 2014 at 11:06 AM, Andres Freund and...@2ndquadrant.com wrote: I don't see why you'd need such a node at all if we had a fully builtin UPSERT. The whole stuff with ON CONFLICT SELECT FOR UPDATE and then UPDATE ... FROM c CONFLICTS is too complicated and exposes stuff that barely

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-18 Thread Andres Freund
On 2014-07-18 11:14:34 -0700, Peter Geoghegan wrote: On Fri, Jul 18, 2014 at 11:06 AM, Andres Freund and...@2ndquadrant.com wrote: I don't see why you'd need such a node at all if we had a fully builtin UPSERT. The whole stuff with ON CONFLICT SELECT FOR UPDATE and then UPDATE ... FROM c

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-18 Thread Peter Geoghegan
On Fri, Jul 18, 2014 at 11:23 AM, Andres Freund and...@2ndquadrant.com wrote: Meh. A understandable syntax wouldn't require the pullups with a special scan node and such. I think you're attempting a sort of genericity that's making your (important!) goal much harder to reach. Well, maybe. If

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-18 Thread Peter Geoghegan
On Fri, Jul 18, 2014 at 11:32 AM, Peter Geoghegan p...@heroku.com wrote: Well, maybe. If the genericity of this syntax isn't what people want, I may go with something else. By the way, I didn't mention that there is now also the optional ability to specify a merge on unique index directly in

[HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-17 Thread Peter Geoghegan
I'm working on UPSERT again. I think that in order to make useful progress, I'll have to find a better way of overcoming the visibility issues (i.e. the problem of what to do about a still-in-progress-to-our-snapshot row being locked at READ COMMITTED isolation level [1][2]). I've made some

Re: [HACKERS] Making joins involving ctid work for the benefit of UPSERT

2014-07-17 Thread Peter Geoghegan
On Thu, Jul 17, 2014 at 6:18 PM, Peter Geoghegan p...@heroku.com wrote: This appears to be a Simple Matter of Programming (at least for someone that happens to already have a good understanding of the optimizer), and is anticipated by this comment within tidpath.c: * There is currently no