Thread

1. a new problem in MERGE

   Boxuan Zhai <bxzhai2010@gmail.com> — 2010-10-31T04:16:14Z

   Dear Greg,
   
   How are you?
   
   As we talked last week, there is a bug about the "origslot" field
   assertion.
   
   TRAP: FailedAssertion("!(epqstate->origslot != ((void *)0))", File:
   "execMain.c", Line: 1732)
   
   
   I checked my codes and found the reason of this problem. In the original
   process of "ExecModifyTable()" function, there is a line of code
   "EvalPlanQualSetSlot(&node->mt_epqstate,
   planSlot);"
   
   It is specially used to init the epqstate->origslot field with the tuple
   slot returned by the underlying plan. However, in our implementation of
   MERGE, this part is missed in "ExecMerge()" function.
   
   At the beginning, I just tried to form a new slot in "ExecMerge()" and use
   it as the epqstate->origslot for the merge actions' execution functions.
   This part is simple.
   
   But, during this process, I found a more serious problem: our MERGE
   currently does not support the use of system attributes in the action
   conditions and target lists.
   
   For example, if we raise queries like the following, there will pops some
   ERROR message.
   
   (table target has oid attribute)
   MERGE INTO target t USING source  s ON t.id = s.id
   WHEN MATCHED AND* t.oid >100000* THEN update SET val = t.val + s.add;
   
   or
   
   MERGE INTO target t USING source  s ON t.id = s.id
   WHEN MATCHED AND* t.xmin = 1000* THEN update SET val = t.val + s.add;
   
   
   
   The reason for the above problem is the missing of system attribute in the
   result slot returned by the main plan of MERGE.
   
   In our implementation,  the Main Pan of MERGE just generate a tuple slot
   contain all the direct vars from source table and target table. The
   expressions in MERGE action conditions and target lists are then calculated
   based on the result slot of main plan.
   
   Currently, all the system attributes (except the ctid of target table) are
   not included in main plan. So, the evaluation of action condition and target
   entries will fail if they involve some system attributes.
   
   If we want to solve this problem, we need to extend the target list of main
   plan before execution (in analyzer or planner), make sure it contains all
   the attributes appeared in any of the MERGE actions. This may lead to
   adjustments to many part of the process of MERGE. For example, within the
   planner, I created a function specially for mapping the Var nodes in actions
   to attributes in the result slot. This function should be rewritten for this
   new case.
   
   I have plan to fix the above two bugs together. (in fact, I have already
   started coding in merge_v202 edition). My question is how should I make my
   update be consistent with yours. Is it possible for you to give me an
   edition that I can work on?
   
   Thanks!
   
   Yours Boxuan
   

2. Re: a new problem in MERGE

   Greg Smith <greg@2ndquadrant.com> — 2010-11-14T19:41:51Z

   Boxuan Zhai wrote:
   > I have plan to fix the above two bugs together. (in fact, I have 
   > already started coding in merge_v202 edition). My question is how 
   > should I make my update be consistent with yours. Is it possible for 
   > you to give me an edition that I can work on? 
   
   I just got this reconciled with HEAD again.  There have been two changes 
   I made in the code you'll eventually want in your working copy:
   
   1) Fix NIL/NULL confusion:  
   https://github.com/greg2ndQuadrant/postgres/commit/9013ba9e81490e3623add1b029760817021297c0
   
   2) Update ExecMerge to accept and pass through an oldtuple value.  This 
   is needed to make the code compatible with the PostgreSQL git HEAD after 
   the changes made in the 2009-09 CommitFest.  Bit rot updates made: 
   https://github.com/greg2ndQuadrant/postgres/commit/be03bd201720f42a666f7e356ec8507c1357f502
   
   I'm not sure if how I did (2) is correct for all cases, but at least the 
   code compiles again now and the server will start.  Attached is an 
   updated patch that applies to HEAD as of right now, and that code has 
   been pushed to 
   https://github.com/greg2ndQuadrant/postgres/tree/merge-unstable with the 
   changes rebased to be the last two commits.
   
   It fails "make installcheck" on my system.  But as the initial diffs I 
   looked at relate to enums and such, I don't think that's a problem with 
   your patch.  Will investigate further here with some of my own patches 
   I'm working on today.  Hopefully this is enough to unblock what you were 
   looking for more details from me about.
   
   -- 
   Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
   
   
   

3. Re: a new problem in MERGE

   Robert Haas <robertmhaas@gmail.com> — 2010-11-21T20:37:53Z

   On Sun, Nov 14, 2010 at 2:41 PM, Greg Smith <greg@2ndquadrant.com> wrote:
   > Boxuan Zhai wrote:
   >>
   >> I have plan to fix the above two bugs together. (in fact, I have already
   >> started coding in merge_v202 edition). My question is how should I make my
   >> update be consistent with yours. Is it possible for you to give me an
   >> edition that I can work on?
   >
   > I just got this reconciled with HEAD again.  There have been two changes I
   > made in the code you'll eventually want in your working copy:
   
   Boxuan, are you still working on this patch?
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company