gSoC add MERGE command new patch -- merge_v104
Hi,
Here comes the new patch for MERGE command. It has the following features:
1. It is based on Heikki's merge_v102-cleanedup.patch. So, it is (hopefully)
clean -- no meaningless white spaces and no overlong clause.
2. The "replaced" mark in MERGE query and plan structures are removed. In
rewriter, the actions replaced by INSTEAD rules will be changed into DO
NOTHING actions.
3. _outDeleteStmt() is removed from code.
4. EXPLAIN MERGE is improved much. You can see the new examples at
https://wiki.postgresql.org/wiki/MergeTestExamples#Explain_Merge
5. The subplan/sublinks are supported in merge actions now. Try the examples
at
https://wiki.postgresql.org/wiki/MergeTestExamples#Subplan.2Fsublinks_in_action
6. Updated merge.sql and merge.out for regress
7. The inheritance is still NOT supported yet.
Thanks
Regards
Yours Boxuan
Attachments:
merge_v104.tarapplication/x-tar; name=merge_v104.tarDownload+2448-40
Hi,
I finished the MERGE on inheritance tables. Now comes the merge_v201
The test example can be found at
https://wiki.postgresql.org/wiki/MergeTestExamples#MERGE_on_inheritance
Thanks
On Thu, Aug 19, 2010 at 10:01 PM, Boxuan Zhai <bxzhai2010@gmail.com> wrote:
Show quoted text
Hi,
Here comes the new patch for MERGE command. It has the following features:
1. It is based on Heikki's merge_v102-cleanedup.patch. So, it is
(hopefully) clean -- no meaningless white spaces and no overlong clause.2. The "replaced" mark in MERGE query and plan structures are removed. In
rewriter, the actions replaced by INSTEAD rules will be changed into DO
NOTHING actions.3. _outDeleteStmt() is removed from code.
4. EXPLAIN MERGE is improved much. You can see the new examples at
https://wiki.postgresql.org/wiki/MergeTestExamples#Explain_Merge5. The subplan/sublinks are supported in merge actions now. Try the
examples at
https://wiki.postgresql.org/wiki/MergeTestExamples#Subplan.2Fsublinks_in_action6. Updated merge.sql and merge.out for regress
7. The inheritance is still NOT supported yet.
Thanks
Regards
Yours Boxuan
Attachments:
merge_v201.tarapplication/x-tar; name=merge_v201.tarDownload+2696-47
On 24/08/10 16:35, Boxuan Zhai wrote:
Hi,
I finished the MERGE on inheritance tables. Now comes the merge_v201
Oh, great! That means that all the known issues are fixed now, and all
that's left is fixing any issues raised in review.
I've added this to the September commitfest, but I hope I'll find some
time to look at this before that. I welcome anyone else to review this too!
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
On Tue, Aug 24, 2010 at 11:02:41PM +0300, Heikki Linnakangas wrote:
On 24/08/10 16:35, Boxuan Zhai wrote:
Hi,
I finished the MERGE on inheritance tables. Now comes the merge_v201
Oh, great! That means that all the known issues are fixed now, and
all that's left is fixing any issues raised in review.I've added this to the September commitfest, but I hope I'll find
some time to look at this before that. I welcome anyone else to
review this too!
I have to ask one question: On a short review of the discussion and
the patch I didn't find anything about the concurrency issues
involved (at least nodeModifyTable.c didnt show any).
Whats the plan to go forward at that subject? I think the patch needs
to lock tables exclusively (the pg level, not access exclusive) as
long as there is no additional handling...
Thanks for the work Boxuan!
Andres
PS: The patch reintroduces some whitespace damage...
On Wed, Aug 25, 2010 at 4:56 AM, Andres Freund <andres@anarazel.de> wrote:
On Tue, Aug 24, 2010 at 11:02:41PM +0300, Heikki Linnakangas wrote:
On 24/08/10 16:35, Boxuan Zhai wrote:
Hi,
I finished the MERGE on inheritance tables. Now comes the merge_v201
Oh, great! That means that all the known issues are fixed now, and
all that's left is fixing any issues raised in review.I've added this to the September commitfest, but I hope I'll find
some time to look at this before that. I welcome anyone else to
review this too!I have to ask one question: On a short review of the discussion and
the patch I didn't find anything about the concurrency issues
involved (at least nodeModifyTable.c didnt show any).
Whats the plan to go forward at that subject? I think the patch needs
to lock tables exclusively (the pg level, not access exclusive) as
long as there is no additional handling...Thanks for the work Boxuan!
The concurrency issues are not involved. I don't know much about this part.
I think we need more discussion on it.
Show quoted text
Andres
PS: The patch reintroduces some whitespace damage...
On Tue, Aug 24, 2010 at 4:56 PM, Andres Freund <andres@anarazel.de> wrote:
Whats the plan to go forward at that subject? I think the patch needs
to lock tables exclusively (the pg level, not access exclusive) as
long as there is no additional handling...
That sounds like it might cause more problems than it solves.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company
On Wed, Aug 25, 2010 at 08:11:18AM +0800, Boxuan Zhai wrote:
On Wed, Aug 25, 2010 at 4:56 AM, Andres Freund <andres@anarazel.de> wrote:
On Tue, Aug 24, 2010 at 11:02:41PM +0300, Heikki Linnakangas wrote:
On 24/08/10 16:35, Boxuan Zhai wrote:
Hi,
I finished the MERGE on inheritance tables. Now comes the
merge_v201Oh, great! That means that all the known issues are fixed now,
and all that's left is fixing any issues raised in review.I've added this to the September commitfest, but I hope I'll
find some time to look at this before that. I welcome anyone
else to review this too!I have to ask one question: On a short review of the discussion
and the patch I didn't find anything about the concurrency issues
involved (at least nodeModifyTable.c didnt show any). Whats the
plan to go forward at that subject? I think the patch needs to
lock tables exclusively (the pg level, not access exclusive) as
long as there is no additional handling...Thanks for the work Boxuan!
The concurrency issues are not involved. I don't know much about
this part. I think we need more discussion on it.
I seem to recall Simon had volunteered some of 2ndQuadrant's time on
this. :)
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
On 24/08/10 23:56, Andres Freund wrote:
I have to ask one question: On a short review of the discussion and
the patch I didn't find anything about the concurrency issues
involved (at least nodeModifyTable.c didnt show any).
The SQL spec doesn't require MERGE to be an atomic "upsert" operation.
Whats the plan to go forward at that subject? I think the patch needs
to lock tables exclusively (the pg level, not access exclusive) as
long as there is no additional handling...
Well, you can always do LOCK TABLE before calling MERGE if that's what
you want, but I don't think doing that automatically would make people
happy.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
On 2010-08-25 9:26 AM +0300, Heikki Linnakangas wrote:
Whats the plan to go forward at that subject? I think the patch needs
to lock tables exclusively (the pg level, not access exclusive) as
long as there is no additional handling...Well, you can always do LOCK TABLE before calling MERGE if that's what
you want, but I don't think doing that automatically would make people
happy.
I don't think having a MERGE that throws UNIQUE violations would make
people happy either.
Regards,
Marko Tiikkaja
On Wed, Aug 25, 2010 at 09:26:51AM +0300, Heikki Linnakangas wrote:
On 24/08/10 23:56, Andres Freund wrote:
I have to ask one question: On a short review of the discussion and
the patch I didn't find anything about the concurrency issues
involved (at least nodeModifyTable.c didnt show any).The SQL spec doesn't require MERGE to be an atomic "upsert" operation.
Whats the plan to go forward at that subject? I think the patch needs
to lock tables exclusively (the pg level, not access exclusive) as
long as there is no additional handling...Well, you can always do LOCK TABLE before calling MERGE if that's
what you want, but I don't think doing that automatically would make
people happy.
But randomly loosing tuples will make much more people unhappy. At a
much more problematic point of time (in production).
There is no locking prohibiting situations like trying to update a
tuple which was concurrently deleted and thus loosing a tuple. Unless
I miss something.
Andres
On 25/08/10 12:41, Andres Freund wrote:
But randomly loosing tuples will make much more people unhappy. At a
much more problematic point of time (in production).
Hmm, how would you lose tuples?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
On Tue, 2010-08-24 at 18:23 -0700, David Fetter wrote:
On Wed, Aug 25, 2010 at 08:11:18AM +0800, Boxuan Zhai wrote:
On Wed, Aug 25, 2010 at 4:56 AM, Andres Freund <andres@anarazel.de> wrote:
The concurrency issues are not involved. I don't know much about
this part. I think we need more discussion on it.I seem to recall Simon had volunteered some of 2ndQuadrant's time on
this. :)
I have offered to work on the concurrency issues, though that will be
after the main body of work is committed to avoid wasting effort. That
seems increasingly likely to happen in next release now, given state of
patch and what looks like a very short dev window for 9.1. This is one
reason why I'm in favour of bringing something to commit as early as
possible, so the additional aspects can be added later.
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services
On 2010-08-25 12:44 PM +0300, Heikki Linnakangas wrote:
On 25/08/10 12:41, Andres Freund wrote:
But randomly loosing tuples will make much more people unhappy. At a
much more problematic point of time (in production).Hmm, how would you lose tuples?
I think what Andres means is: T1 starts a MERGE. INSERT fails because
the tuple already exists, but then another transaction, T2, DELETEs that
tuple. T1 tries to UPDATE it, but fails because it doesn't exist
anymore. Not T1 should go back and INSERT the tuple, but that isn't
what happens with this patch, is it?
Regards,
Marko Tiikkaja