On Thu, Apr 23, 2015 at 6:07 PM, Peter Geoghegan wrote:
> Curious about what you think about my proposal to go back to putting
> the inference specification WHERE clause within the parenthesis, since
> this solves several problems, including clarifying to users that the
> predicate is part of the
* Peter Geoghegan (p...@heroku.com) wrote:
> On Fri, Apr 24, 2015 at 5:50 PM, Stephen Frost wrote:
> > * Peter Geoghegan (p...@heroku.com) wrote:
> >> I came up with something that is along the lines of what we discussed.
> >> I'll wait for you to push Dean's code, and rebase on top of that
> >> b
On Fri, Apr 24, 2015 at 5:50 PM, Stephen Frost wrote:
> * Peter Geoghegan (p...@heroku.com) wrote:
>> I came up with something that is along the lines of what we discussed.
>> I'll wait for you to push Dean's code, and rebase on top of that
>> before submitting what I came up with. Maybe this can
Peter,
* Peter Geoghegan (p...@heroku.com) wrote:
> I came up with something that is along the lines of what we discussed.
> I'll wait for you to push Dean's code, and rebase on top of that
> before submitting what I came up with. Maybe this can be rolled into
> my next revision if I work through
On Fri, Apr 24, 2015 at 12:31 AM, Heikki Linnakangas wrote:
> Ok, I see now that I totally screwed up the conditions on "waitMode" in that
> commit. I just pushed a fix to your github repository.
I can confirm that the commit you just pushed prevents the
implementation from attempting to lock an
On 04/24/2015 02:52 AM, Peter Geoghegan wrote:
I found a bug that seems to be down to commit
e3144183562d08e347f832f0b29daefe8bac617b on Github:
"""
commit e3144183562d08e347f832f0b29daefe8bac617b
Author: Heikki Linnakangas
Date: Thu Apr 23 18:38:11 2015 +0300
Minor cleanup of check_exc
On Thu, Apr 23, 2015 at 12:45 PM, Peter Geoghegan wrote:
>>> * We need to figure out the tuple lock strength details. I think this
>>> is doable, but it is the greatest challenge to committing ON CONFLICT
>>> UPDATE at this point. Andres feels that we should require no greater
>>> lock strength th
On Thu, Apr 23, 2015 at 1:08 PM, Heikki Linnakangas wrote:
> That said, I'd actually like to see a separate heap_super_delete() function
> for that, rather than piggybacking on heap_delete(). It's a quite different
> operation. There'll be some duplication, but seems better than a maze of
> if-els
On 2015-04-23 23:08:34 +0300, Heikki Linnakangas wrote:
> The heapam API is not that stable, we've added arguments to those functions
> every once in a while, and I don't recall any complaints.
I heard some, but not too many, that's true. I know that I've written
code that'd be broken/needed even
On 04/23/2015 10:53 PM, Andres Freund wrote:
On 2015-04-23 12:45:59 -0700, Peter Geoghegan wrote:
On Thu, Apr 23, 2015 at 12:55 AM, Andres Freund wrote:
I think you misread my statement: I'm saying we don't need the new
argument anymore, even if we still do the super-deletion in
heap_delete().
On Thu, Apr 23, 2015 at 12:53 PM, Andres Freund wrote:
> Unconvinced. Not breaking an API has its worth.
Yeah, and I acknowledge that - but it's not something that it's
appropriate to encapsulate IMV.
Let's just leave it to Heikki...I'd say he has the deciding vote,
especially as the reviewer th
On 2015-04-23 12:45:59 -0700, Peter Geoghegan wrote:
> On Thu, Apr 23, 2015 at 12:55 AM, Andres Freund wrote:
> > I think you misread my statement: I'm saying we don't need the new
> > argument anymore, even if we still do the super-deletion in
> > heap_delete(). Now that the speculative insertion
On Thu, Apr 23, 2015 at 12:55 AM, Andres Freund wrote:
> I think you misread my statement: I'm saying we don't need the new
> argument anymore, even if we still do the super-deletion in
> heap_delete(). Now that the speculative insertion will not be visible
> (as in seen on a tuple they could dele
On Thu, Apr 23, 2015 at 9:02 AM, Heikki Linnakangas wrote:
> That code in ExecWithCheckOptions is not translatable. See style guide:
> http://www.postgresql.org/docs/devel/static/nls-programmer.html#NLS-GUIDELINES
It's probably going to need to change when I rebase on top of
Dean's/Stephen's work
On 04/20/2015 07:37 AM, Peter Geoghegan wrote:
if (wco->commandType == CMD_INSERT)
command = "INSERT-applicable ";
else if (wco->commandType == CMD_UPDATE)
command = "UPDATE-applicable
On Thu, Apr 23, 2015 at 05:02:19PM +0200, Andres Freund wrote:
> On 2015-04-23 15:52:40 +0100, Geoff Winkless wrote:
> > When I set out I was really only hoping to express a preference as a user;
> > on balance I would really rather not have DO IGNORE, if it were possible to
> > avoid, because it's
On 2015-04-23 15:52:40 +0100, Geoff Winkless wrote:
> When I set out I was really only hoping to express a preference as a user;
> on balance I would really rather not have DO IGNORE, if it were possible to
> avoid, because it's really ugly, but DO UPDATE/DO NOTHING I could just
> about cope with (
On 23 April 2015 at 14:50, Andres Freund wrote:
> > Maybe I'm misreading it, but isn't index_predicate meant to be inside
> the
> > brackets?
> >
> >
> http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-insert.html
>
> That has changed since.
Oh, helpful. :)
On 2015-04-23 14:34:02 +0100, Geoff Winkless wrote:
> > A syntax error. DO is a reserved keyword. Update is just unreserved (and
> > thus can be used as a column label). Ignore is unreserved with the patch
> > and was unreserved before. We obviously can make both reserved, but of so
> > we have to
On 23 April 2015 at 13:51, Andres Freund wrote:
> On April 23, 2015 3:34:07 PM GMT+03:00, Geoff Winkless <
> pgsqlad...@geoff.dj> wrote:
>
> >
> And what's to stop me having "SELECT true
>
> AS
>do" in the where clause (as per your UPDATE objection)?
>
> A syntax error. DO is a reserved k
On April 23, 2015 3:34:07 PM GMT+03:00, Geoff Winkless
wrote:
>Apologies for butting in but can I (as a user) express a preference as
>a
>user against DO?
Sure. If you propose an alternative ;)
>Firstly, it looks horrible. And what's to stop me having "SELECT true
>AS
>do" in the where clause (
On 23/04/15 14:34, Geoff Winkless wrote:
Apologies for butting in but can I (as a user) express a preference as a
user against DO?
Firstly, it looks horrible. And what's to stop me having "SELECT true AS
do" in the where clause (as per your UPDATE objection)?
DO is already reserved keyword. T
Apologies for butting in but can I (as a user) express a preference as a
user against DO?
Firstly, it looks horrible. And what's to stop me having "SELECT true AS
do" in the where clause (as per your UPDATE objection)?
Shouldn't UPDATE be a reserved keyword anyway? AIUI ANSI suggests so.
http://
On 2015-04-22 16:40:07 -0700, Peter Geoghegan wrote:
> On Wed, Apr 22, 2015 at 3:23 PM, Peter Geoghegan wrote:
> > * We need to sort out those issues with the grammar, since that only
> > really applies to the inference specification. Maybe the WHERE clause
> > that the inference specification acc
On 2015-04-22 15:23:16 -0700, Peter Geoghegan wrote:
> On Tue, Apr 21, 2015 at 7:57 AM, Andres Freund wrote:
> > * Iff we're going to have the XLOG_HEAP_AFFIRM record, I'd rather have
> > that guide the logical decoding code. Seems slightly cleaner.
>
> I thought that you didn't think that woul
On Wed, Apr 22, 2015 at 3:23 PM, Peter Geoghegan wrote:
> * We need to sort out those issues with the grammar, since that only
> really applies to the inference specification. Maybe the WHERE clause
> that the inference specification accepts can be broken out. No ON
> CONFLICT UPDATE specific issu
On Tue, Apr 21, 2015 at 7:57 AM, Andres Freund wrote:
> I'm not 100% sure Heikki and I am on exactly the same page here :P
>
> I'm looking at git diff $(git merge-base upstream/master HEAD).. where
> HEAD is e1a5822d164db0.
Merged your stuff into my Github branch. Heikki is pushing changes
there
On 2015-04-21 16:57:45 +0200, Andres Freund wrote:
> * I still think it's unacceptable to redefine
> XLOG_HEAP_LAST_MULTI_INSERT as XLOG_HEAP_SPECULATIVE_TUPLE like you
> did. I'll try to find something better.
I think we should "just" split this into different flag values for
insert/update/de
On 2015-04-19 21:37:51 -0700, Peter Geoghegan wrote:
> Attached patch, V3.4, implements what I believe you and Heikki have in
> mind here.
I'm not 100% sure Heikki and I am on exactly the same page here :P
I'm looking at git diff $(git merge-base upstream/master HEAD).. where
HEAD is e1a5822d164d
On Sun, Apr 19, 2015 at 9:37 PM, Peter Geoghegan wrote:
> Attached patch, V3.4, implements what I believe you and Heikki have in
> mind here.
Any plans to look at this, Svenne? You are signed up as a reviewer on
the commitfest app. A review of the documentation, and interactions
with other featur
On 17 April 2015 at 12:54, Stephen Frost wrote:
> Dean, I've been working through your patches over the past couple of
> days (apologies for the lack of updates, just been busy) and hope to
> push them very shortly (ie: by the end of the weekend).
>
Cool. Thanks.
> One thing that I was hoping to
On Fri, Apr 17, 2015 at 1:30 AM, Andres Freund wrote:
>> However, like my second approach, there would be a "speculative
>> affirmation" WAL record.
>
> I think there should be one, but it's not required for the approach. The
> 'pending speculative insertion' can just be completed whenever there's
On Fri, Apr 17, 2015 at 11:19 AM, Heikki Linnakangas wrote:
>> because he wanted to preserve those by doing the MagicOffsetNumber
>> thing.
>
>
> Yes, that's the way to go.
>
> Glad we cleared that up :-).
Got it, thanks!
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hack
On 04/17/2015 09:02 PM, Peter Geoghegan wrote:
I'm slightly surprised that Heikki now wants
to use an infomask2 bit (if that is what you mean),
No, I don't.
because he wanted to preserve those by doing the MagicOffsetNumber
thing.
Yes, that's the way to go.
Glad we cleared that up :-).
-
On Fri, Apr 17, 2015 at 1:38 AM, Andres Freund wrote:
>> Do you just mean that you think that speculative insertions should be
>> explicitly affirmed by a second record (making it not a speculative
>> tuple, but rather, a fully fledged tuple)? IOW, an MVCC snapshot has
>> no chance of seeing a tup
On Fri, Apr 17, 2015 at 4:54 AM, Stephen Frost wrote:
> Dean, I've been working through your patches over the past couple of
> days (apologies for the lack of updates, just been busy) and hope to
> push them very shortly (ie: by the end of the weekend).
>
> One thing that I was hoping to discuss a
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> In all of this, I think we should try to keep things as simple as
> possible, to give the end user a chance to understand it --- although
> I'm not sure I've achieved that with my explanation :-)
Thanks a lot for this. It goes along with m
On 2015-04-16 09:43:54 -0700, Peter Geoghegan wrote:
> On Thu, Apr 16, 2015 at 2:23 AM, Andres Freund wrote:
> > I'm, completely independent of logical decoding, of the *VERY* strong
> > opinion that 'speculative insertions' should never be visible when
> > looking with normal snapshots. For one i
On 2015-04-16 10:25:29 -0700, Peter Geoghegan wrote:
> On Thu, Apr 16, 2015 at 2:18 AM, Andres Freund wrote:
> > If we go that way that's the way I think it should be done: Whenever we
> > encounter a speculative record we 'unlink' it from the changes that will
> > be reused for spooling from disk
On 9 April 2015 at 22:18, Peter Geoghegan wrote:
> The big idea (the fine details of which Stephen appeared to be in
> tentative agreement with [1]) is that an UPSERT is a hybrid between an
> INSERT and an UPDATE, and not simply an INSERT and separate UPDATE
> tied together. So at the very least t
On Thu, Apr 16, 2015 at 2:18 AM, Andres Freund wrote:
> On 2015-04-15 18:53:15 +0300, Heikki Linnakangas wrote:
>> Hmm, ok, I've read the "INSERT ... ON CONFLICT UPDATE and logical decoding"
>> thread now, and I have to say that IMHO it's a lot more sane to handle this
>> in ReorderBufferCommit()
On Thu, Apr 16, 2015 at 2:23 AM, Andres Freund wrote:
> I'm, completely independent of logical decoding, of the *VERY* strong
> opinion that 'speculative insertions' should never be visible when
> looking with normal snapshots. For one it allows to simplify
> considerations around wraparound (whic
On 04/16/2015 12:18 PM, Andres Freund wrote:
On 2015-04-15 18:53:15 +0300, Heikki Linnakangas wrote:
Hmm, ok, I've read the "INSERT ... ON CONFLICT UPDATE and logical decoding"
thread now, and I have to say that IMHO it's a lot more sane to handle this
in ReorderBufferCommit() like Peter first d
On 2015-04-15 17:58:54 +0300, Heikki Linnakangas wrote:
> When the speculative insertion is finished, write a new kind of a WAL record
> for that. The record only needs to contain the ctid of the tuple. Replaying
> that record will clear the flag on the heap tuple that said that it was a
> speculat
On 2015-04-15 18:53:15 +0300, Heikki Linnakangas wrote:
> Hmm, ok, I've read the "INSERT ... ON CONFLICT UPDATE and logical decoding"
> thread now, and I have to say that IMHO it's a lot more sane to handle this
> in ReorderBufferCommit() like Peter first did, than to make the main
> insertion path
On 04/15/2015 06:01 PM, Andres Freund wrote:
On 2015-04-15 17:58:54 +0300, Heikki Linnakangas wrote:
On 04/15/2015 07:51 AM, Peter Geoghegan wrote:
+heap_finish_speculative(Relation relation, HeapTuple tuple, bool conflict)
+{
+ if (!conflict)
+ {
+ /*
+
On 2015-04-15 17:58:54 +0300, Heikki Linnakangas wrote:
> On 04/15/2015 07:51 AM, Peter Geoghegan wrote:
> >+heap_finish_speculative(Relation relation, HeapTuple tuple, bool conflict)
> >+{
> >+if (!conflict)
> >+{
> >+/*
> >+ * Update the tuple in-place, in the comm
On 04/15/2015 07:51 AM, Peter Geoghegan wrote:
+heap_finish_speculative(Relation relation, HeapTuple tuple, bool conflict)
+{
+ if (!conflict)
+ {
+ /*
+* Update the tuple in-place, in the common case where no
conflict was
+* detected dur
On Wed, Mar 18, 2015 at 2:59 AM, Dean Rasheed wrote:
> Yes, I read that, and I agree with the intention to not leak data
> according to both the INSERT and UPDATE policies, however...
>
>> You're seeing a failure that applies to the target tuple of the UPDATE
>> (the tuple that we can't leak the c
On Tue, Apr 7, 2015 at 10:59 PM, Peter Geoghegan wrote:
> The documentation has been updated to reflect all of this.
By the way, for the convenience of reviewers I continue to maintain a
mirror of pre-built documentation as outlined here:
https://wiki.postgresql.org/wiki/UPSERT#Documentation
--
On Mon, Mar 30, 2015 at 9:20 AM, Peter Geoghegan wrote:
>> * The code uses LockTupleExclusive to lock rows. That means the fkey key
>> locking doesn't work; That's annoying. This means that using upsert
>> will potentially cause deadlocks in other sessions :(. I think you'll
>> have to deter
On Tue, Mar 31, 2015 at 2:26 PM, Peter Geoghegan wrote:
> Andres' wish to do things that way is at least partially motivated by
> having logical decoding just work.
I should add that there appears to be some need to terminate the loop
of speculative token waiting. By that I mean that since we're
On Tue, Mar 31, 2015 at 1:09 PM, Heikki Linnakangas wrote:
> I'm pretty sceptical of that. ISTM you'll need to do modify the page twice
> for each insertion, first to insert the promise tuple, and then to turn the
> promise tuple into a real tuple. And WAL-log both updates. That's going to
> hurt
On 03/30/2015 07:20 PM, Peter Geoghegan wrote:
>* I think we should decouple the insertion and wal logging more. I think
> the promise tuple insertion should be different from the final
> insertion of the actual tuple. For one it seems cleaner to me, for
> another it will avoid the uglyne
On Sat, Mar 28, 2015 at 6:36 PM, Andres Freund wrote:
> Just had a longer chat with Peter about this patch.
It was a very useful chat. Thanks for making yourself available to do it.
> * Not a fan of the heap flags usage, the reusage seems sketch to me
> * Explain should show the arbiter index in
Hi,
Just had a longer chat with Peter about this patch.
* Not a fan of the heap flags usage, the reusage seems sketch to me
* Explain should show the arbiter index in text as well
* AddUniqueSpeculative is a bad name, it should refer IndexInfo
* Work on the ExecInsert() comments
* Let's remove th
On 27/03/15 09:14, Peter Geoghegan wrote:
On Thu, Mar 26, 2015 at 2:51 PM, Heikki Linnakangas wrote:
[...]
Oops - You're right. I find it interesting that this didn't result in
breaking my test cases.
[...]
Reminds of the situation where I got an A++ for a COBOL programming
assignment that
On Thu, Mar 26, 2015 at 2:51 PM, Heikki Linnakangas wrote:
>> Attached revision, V3.1, implements this slightly different way of
>> figuring out if an insertion is speculative, as discussed. We reuse
>> t_ctid for speculatively inserted tuples. That's where the counter
>> goes. I think that this i
On 03/26/2015 08:00 PM, Peter Geoghegan wrote:
On Wed, Mar 25, 2015 at 12:42 PM, Peter Geoghegan wrote:
My next revision will have a more polished version of this scheme. I'm
not going to immediately act on Robert's feedback elsewhere (although
I'd like to), owing to time constraints - no reaso
On Wed, Mar 18, 2015 at 2:41 PM, Heikki Linnakangas wrote:
> Here's what I had in mind: the inserter tags the tuple with the speculative
> insertion token, by storing the token in the t_ctid field. If the inserter
> needs to super-delete the tuple, it sets xmax like in a regular deletion,
> but al
On Thu, Mar 19, 2015 at 1:02 PM, Robert Haas wrote:
> I think this is pretty lousy. The reasons why the user wants things
> that way is because they created a UNIQUE index and it got bloated
> somehow with lots of dead tuples. So they made a new UNIQUE index on
> the same column and then they're
On Wed, Mar 18, 2015 at 3:40 PM, Peter Geoghegan wrote:
>>> I think Heikki's concern is something different, although I am not
>>> altogether up to speed on this and may be confused. The issue is:
>>> suppose that process A and process B are both furiously upserting into
>>> the same table with t
On Wed, Mar 18, 2015 at 9:19 AM, Robert Haas wrote:
> On Tue, Mar 17, 2015 at 3:11 PM, Heikki Linnakangas wrote:
>> I've been thinking that it would be nice to be able to specify a constraint
>> name. Naming an index directly feels wrong, as in relational and SQL
>> philosophy, indexes are just a
On Wed, Mar 18, 2015 at 11:41 AM, Heikki Linnakangas wrote:
> AFAICS, there is no need to go and clear the tag after the insert has
> completed.
>
> Here's what I had in mind: the inserter tags the tuple with the speculative
> insertion token, by storing the token in the t_ctid field. If the inser
On 03/18/2015 06:23 PM, Robert Haas wrote:
On Tue, Mar 17, 2015 at 6:27 PM, Peter Geoghegan wrote:
On Tue, Mar 17, 2015 at 12:11 PM, Heikki Linnakangas wrote:
I'm still not sure the way the speculative locking works is the best
approach. Instead of clearing xmin on super-deletion, a new flag
On Tue, Mar 17, 2015 at 6:27 PM, Peter Geoghegan wrote:
> On Tue, Mar 17, 2015 at 12:11 PM, Heikki Linnakangas wrote:
>> I'm still not sure the way the speculative locking works is the best
>> approach. Instead of clearing xmin on super-deletion, a new flag on the heap
>> tuple seems more straigh
On Tue, Mar 17, 2015 at 3:11 PM, Heikki Linnakangas wrote:
> I've been thinking that it would be nice to be able to specify a constraint
> name. Naming an index directly feels wrong, as in relational and SQL
> philosophy, indexes are just an implementation detail, but naming a
> constraint is a fa
On 17 March 2015 at 23:25, Peter Geoghegan wrote:
>> Possibly I'm missing something though.
>
> I think that you may have. Did you read the commit message/docs of the
> RLS commit 0004-*? You must consider the second point here, I believe:
>
>
> The 3 places that RLS policies are enforced are
On Mon, Mar 16, 2015 at 8:10 AM, Dean Rasheed wrote:
> (Note there is some bitrot in gram.y that prevents the first patch
> from applying cleanly to HEAD)
That's trivially fixable. I'll have those fixes in the next revision,
once I firm some things up with Heikki.
> I tested using the attached s
On Tue, Mar 17, 2015 at 12:11 PM, Heikki Linnakangas wrote:
> I'm still not sure the way the speculative locking works is the best
> approach. Instead of clearing xmin on super-deletion, a new flag on the heap
> tuple seems more straightforward. And we could put the speculative insertion
> token i
On Thu, Mar 5, 2015 at 3:44 PM, Peter Geoghegan wrote:
> Bruce Momjian kindly made available a server for stress-testing [1].
> I'm using jjanes_upsert for this task (I stopped doing this for a
> little while, and was in need of a new server).
BTW, this was run for about another week on Bruce's s
On 03/05/2015 03:18 AM, Peter Geoghegan wrote:
Attached patch series forms what I'm calling V3.0 of the INSERT ... ON
CONFLICT IGNORE/UPDATE feature. (Sorry about all the threads. I feel
this development justifies a new thread, though.)
I'm still not sure the way the speculative locking works i
On 5 March 2015 at 23:44, Peter Geoghegan wrote:
> On Wed, Mar 4, 2015 at 5:18 PM, Peter Geoghegan wrote:
>> Attached patch series forms what I'm calling V3.0 of the INSERT ... ON
>> CONFLICT IGNORE/UPDATE feature. (Sorry about all the threads. I feel
>> this development justifies a new thread, t
On Wed, Mar 4, 2015 at 5:18 PM, Peter Geoghegan wrote:
> Attached patch series forms what I'm calling V3.0 of the INSERT ... ON
> CONFLICT IGNORE/UPDATE feature. (Sorry about all the threads. I feel
> this development justifies a new thread, though.)
Bruce Momjian kindly made available a server f
74 matches
Mail list logo
