Thread: MERGE command for inheritance

MERGE command for inheritance

From
Boxuan Zhai
Date:
Hi,
 
These days I am considering what else can be done for MERGE, And, I find inheritance tables in postgres is not supported by our MERGE command yet.

Currently, MERGE command is only able to handle the target table itself, and its children tables are not involved in the process.

I am not sure if inheritance of MERGE is needed by postgres. If we need, I may propose two methods for implementing it.
 
An easy way to do it is use a rule-like strategy. We can generate new MERGE query statements with the children table as their target tables. Then the new
query statement will be planned and executed in the normal way. This process can be put in the rewriter, before the queries are planned.
This method is quite easy but seems not follow the tradition of inheritance in Postgres.
 
 
The difficult way is to generate the plans for children table in planner, as the other commands like UPDATE and DELETE. However, because the structure of
MERGE plan is much more complex than the ordinary ModifyTable plans, this job may not as simple as we expected. We need to adjust both the main plan and the
merge actions to fit the children tables, which is not straight forward.
 
I would like to know your opinions on this problem.
 
PS: for my investigation on the inheritance actions, I find that although the children tables are modified by the UPDATE or DELETE commands on their ancestor tables, the rules defined on them are not activated during the query. Is this the case (I hope I am not asking a stupid question)? And, if so, I may ask why we want it to act like this.
 
Boxuan

Re: MERGE command for inheritance

From
Peter Eisentraut
Date:
On tis, 2010-08-10 at 17:38 +0800, Boxuan Zhai wrote:
> I am not sure if inheritance of MERGE is needed by postgres.

Yes, it is.

> PS: for my investigation on the inheritance actions, I find that
> although the children tables are modified by the UPDATE or DELETE
> commands on their ancestor tables, the rules defined on them are not
> activated during the query. Is this the case (I hope I am not asking a
> stupid question)? And, if so, I may ask why we want it to act like
> this. 

Your observation is correct.  You could probably argue this way or that
about how it should have been designed 20+ years ago, but this is how it
is.

In general, I wouldn't design new functionality on top of rules.  Rules
are pretty broken in many ways.




Re: MERGE command for inheritance

From
Heikki Linnakangas
Date:
On 10/08/10 12:38, Boxuan Zhai wrote:
> The difficult way is to generate the plans for children table in planner, as
> the other commands like UPDATE and DELETE. However, because the structure of
> MERGE plan is much more complex than the ordinary ModifyTable plans, this
> job may not as simple as we expected. We need to adjust both the main plan
> and the
> merge actions to fit the children tables, which is not straight forward.

This the approach you'll have to take. But actually, I'm surprised it 
doesn't happen to just work already. It should be opaque to the merge 
facility that the reference to the parent target table has inherited 
child tables - expanding the inherited table to scans of all the 
children should already be handled by the planner.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: MERGE command for inheritance

From
Heikki Linnakangas
Date:
On 10/08/10 12:38, Boxuan Zhai wrote:
> These days I am considering what else can be done for MERGE, And, I
> find inheritance tables in postgres is not supported by our MERGE command
> yet.

I played with your latest patch version a bit, and actually, it seems to 
me that inherited tables work just fine. I ran into the assertion 
failures earlier while trying that, but that has now been fixed. Can you 
give an example of the kind of query that's not working yet?

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: MERGE command for inheritance

From
Simon Riggs
Date:
On Tue, 2010-08-10 at 17:15 +0300, Heikki Linnakangas wrote:
> On 10/08/10 12:38, Boxuan Zhai wrote:
> > The difficult way is to generate the plans for children table in planner, as
> > the other commands like UPDATE and DELETE. However, because the structure of
> > MERGE plan is much more complex than the ordinary ModifyTable plans, this
> > job may not as simple as we expected. We need to adjust both the main plan
> > and the
> > merge actions to fit the children tables, which is not straight forward.
> 
> This the approach you'll have to take. But actually, I'm surprised it 
> doesn't happen to just work already. It should be opaque to the merge 
> facility that the reference to the parent target table has inherited 
> child tables - expanding the inherited table to scans of all the 
> children should already be handled by the planner.

The support for UPDATE and SELECT of partitioned cases is very different
in the planner and was handled as separate implementation projects.

If we want a working MERGE in the next release, I suggest that we break
down this project in the same way and look at partitioned target tables
as a separate project.

One reason for suggesting this is that all MERGE statements have a
source table, whereas UPDATE and DELETEs did not always. The plan for a
simple UPDATE and DELETE against a partitioned table is simple, but the
plan (and performance) of a joined UPDATE or DELETE is not good:

postgres=# explain update p set col2 = x.col2 from x where x.col1 =
p.col1;                               QUERY
PLAN                                 
---------------------------------------------------------------------------Update  (cost=299.56..1961.18 rows=68694
width=20) ->  Merge Join  (cost=299.56..653.73 rows=22898 width=20)        Merge Cond: (public.p.col1 = x.col1)
-> Sort  (cost=149.78..155.13 rows=2140 width=10)              Sort Key: public.p.col1              ->  Seq Scan on p
(cost=0.00..31.40rows=2140 width=10)        ->  Sort  (cost=149.78..155.13 rows=2140 width=14)              Sort Key:
x.col1             ->  Seq Scan on x  (cost=0.00..31.40 rows=2140 width=14)  ->  Merge Join  (cost=299.56..653.73
rows=22898width=20)        Merge Cond: (public.p.col1 = x.col1)        ->  Sort  (cost=149.78..155.13 rows=2140
width=10)             Sort Key: public.p.col1              ->  Seq Scan on p1 p  (cost=0.00..31.40 rows=2140
 
width=10)        ->  Sort  (cost=149.78..155.13 rows=2140 width=14)              Sort Key: x.col1              ->  Seq
Scanon x  (cost=0.00..31.40 rows=2140 width=14)  ->  Merge Join  (cost=299.56..653.73 rows=22898 width=20)        Merge
Cond:(public.p.col1 = x.col1)        ->  Sort  (cost=149.78..155.13 rows=2140 width=10)              Sort Key:
public.p.col1             ->  Seq Scan on p2 p  (cost=0.00..31.40 rows=2140
 
width=10)        ->  Sort  (cost=149.78..155.13 rows=2140 width=14)              Sort Key: x.col1              ->  Seq
Scanon x  (cost=0.00..31.40 rows=2140 width=14)
 

Those plans could use some love and attention before forcing Boxuan to
implement that.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Development, 24x7 Support, Training and Services



Re: MERGE command for inheritance

From
Heikki Linnakangas
Date:
On 11/08/10 11:45, Simon Riggs wrote:
> On Tue, 2010-08-10 at 17:15 +0300, Heikki Linnakangas wrote:
>> On 10/08/10 12:38, Boxuan Zhai wrote:
>>> The difficult way is to generate the plans for children table in planner, as
>>> the other commands like UPDATE and DELETE. However, because the structure of
>>> MERGE plan is much more complex than the ordinary ModifyTable plans, this
>>> job may not as simple as we expected. We need to adjust both the main plan
>>> and the
>>> merge actions to fit the children tables, which is not straight forward.
>>
>> This the approach you'll have to take. But actually, I'm surprised it
>> doesn't happen to just work already. It should be opaque to the merge
>> facility that the reference to the parent target table has inherited
>> child tables - expanding the inherited table to scans of all the
>> children should already be handled by the planner.
>
> The support for UPDATE and SELECT of partitioned cases is very different
> in the planner and was handled as separate implementation projects.

Ok, thinking and experminting this some more I finally understand what 
the problem is. Yeah, the patch doesn't currently work when the target 
table has inherited child tables, it only takes the parent table into 
account and ignores all child tables.

> If we want a working MERGE in the next release, I suggest that we break
> down this project in the same way and look at partitioned target tables
> as a separate project.
>
> One reason for suggesting this is that all MERGE statements have a
> source table, whereas UPDATE and DELETEs did not always. The plan for a
> simple UPDATE and DELETE against a partitioned table is simple, but the
> plan (and performance) of a joined UPDATE or DELETE is not good:

I don't think we can just leave it as it is. If the performance sucks, 
that's fine and can be handled in a future release, but it should at 
least produce the correct result.

I concur that Boxuan's suggested "difficult" approach seems like the 
right one.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: MERGE command for inheritance

From
Simon Riggs
Date:
On Wed, 2010-08-11 at 13:25 +0300, Heikki Linnakangas wrote:

> I concur that Boxuan's suggested "difficult" approach seems like the 
> right one. 

Right, but you've completely ignored my proposal: lets do this in two
pieces. Get what we have now ready to commit, then add support for
partitioning later, as a second project.

Two reasons for this: we endanger the current project by adding more to
it in one go, plus work on other aspects of partitioning is happening
concurrently and the two are likely to conflict and/or waste effort.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Development, 24x7 Support, Training and Services



Re: MERGE command for inheritance

From
Boxuan Zhai
Date:


On Wed, Aug 11, 2010 at 4:27 PM, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
On 10/08/10 12:38, Boxuan Zhai wrote:
These days I am considering what else can be done for MERGE, And, I
find inheritance tables in postgres is not supported by our MERGE command
yet.

I played with your latest patch version a bit, and actually, it seems to me that inherited tables work just fine. I ran into the assertion failures earlier while trying that, but that has now been fixed. Can you give an example of the kind of query that's not working yet?

 
Well, in the patch I submitted, the target relation is forced not to scan any inheritance tables. That is, the command always acts like
MERGE into ONLY foo USING bar ....
 
So, the inheritance in current MERGE should not work, I think.

--
 Heikki Linnakangas
 EnterpriseDB   http://www.enterprisedb.com

Re: MERGE command for inheritance

From
Heikki Linnakangas
Date:
On 11/08/10 14:44, Simon Riggs wrote:
> On Wed, 2010-08-11 at 13:25 +0300, Heikki Linnakangas wrote:
>
>> I concur that Boxuan's suggested "difficult" approach seems like the
>> right one.
>
> Right, but you've completely ignored my proposal: lets do this in two
> pieces. Get what we have now ready to commit, then add support for
> partitioning later, as a second project.

It seems like a pretty serious omission. What would you do, thrown a 
"MERGE to inherited tables not implemented" error?

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: MERGE command for inheritance

From
Simon Riggs
Date:
On Wed, 2010-08-11 at 15:53 +0300, Heikki Linnakangas wrote:
> On 11/08/10 14:44, Simon Riggs wrote:
> > On Wed, 2010-08-11 at 13:25 +0300, Heikki Linnakangas wrote:
> >
> >> I concur that Boxuan's suggested "difficult" approach seems like the
> >> right one.
> >
> > Right, but you've completely ignored my proposal: lets do this in two
> > pieces. Get what we have now ready to commit, then add support for
> > partitioning later, as a second project.
> 
> It seems like a pretty serious omission. What would you do, thrown a 
> "MERGE to inherited tables not implemented" error?

It's not a "serious omission" to do work in multiple phases. I have not
proposed that we neglect that work, only that it happens afterwards.
Phasing work often allows the whole to be delivered quicker and it
reduces the risk that we end up with nothing at all or spaghetti code
through rushing things.

We have already split MERGE into two phases from its original scope,
where the majority thought for many years that MERGE without concurrent
locking was unacceptable. Splitting MERGE into 3 phases now is hardly an
earth shaking proposal.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Development, 24x7 Support, Training and Services



Re: MERGE command for inheritance

From
Boxuan Zhai
Date:


On Wed, Aug 11, 2010 at 4:45 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
On Tue, 2010-08-10 at 17:15 +0300, Heikki Linnakangas wrote:
> On 10/08/10 12:38, Boxuan Zhai wrote:
> > The difficult way is to generate the plans for children table in planner, as
> > the other commands like UPDATE and DELETE. However, because the structure of
> > MERGE plan is much more complex than the ordinary ModifyTable plans, this
> > job may not as simple as we expected. We need to adjust both the main plan
> > and the
> > merge actions to fit the children tables, which is not straight forward.
>
> This the approach you'll have to take. But actually, I'm surprised it
> doesn't happen to just work already. It should be opaque to the merge
> facility that the reference to the parent target table has inherited
> child tables - expanding the inherited table to scans of all the
> children should already be handled by the planner.

The support for UPDATE and SELECT of partitioned cases is very different
in the planner and was handled as separate implementation projects.

If we want a working MERGE in the next release, I suggest that we break
down this project in the same way and look at partitioned target tables
as a separate project.

One reason for suggesting this is that all MERGE statements have a
source table, whereas UPDATE and DELETEs did not always. The plan for a
simple UPDATE and DELETE against a partitioned table is simple, but the
plan (and performance) of a joined UPDATE or DELETE is not good:

postgres=# explain update p set col2 = x.col2 from x where x.col1 =
p.col1;
                               QUERY
PLAN
---------------------------------------------------------------------------
 Update  (cost=299.56..1961.18 rows=68694 width=20)
  ->  Merge Join  (cost=299.56..653.73 rows=22898 width=20)
        Merge Cond: (public.p.col1 = x.col1)
        ->  Sort  (cost=149.78..155.13 rows=2140 width=10)
              Sort Key: public.p.col1
              ->  Seq Scan on p  (cost=0.00..31.40 rows=2140 width=10)
        ->  Sort  (cost=149.78..155.13 rows=2140 width=14)
              Sort Key: x.col1
              ->  Seq Scan on x  (cost=0.00..31.40 rows=2140 width=14)
  ->  Merge Join  (cost=299.56..653.73 rows=22898 width=20)
        Merge Cond: (public.p.col1 = x.col1)
        ->  Sort  (cost=149.78..155.13 rows=2140 width=10)
              Sort Key: public.p.col1
              ->  Seq Scan on p1 p  (cost=0.00..31.40 rows=2140
width=10)
        ->  Sort  (cost=149.78..155.13 rows=2140 width=14)
              Sort Key: x.col1
              ->  Seq Scan on x  (cost=0.00..31.40 rows=2140 width=14)
  ->  Merge Join  (cost=299.56..653.73 rows=22898 width=20)
        Merge Cond: (public.p.col1 = x.col1)
        ->  Sort  (cost=149.78..155.13 rows=2140 width=10)
              Sort Key: public.p.col1
              ->  Seq Scan on p2 p  (cost=0.00..31.40 rows=2140
width=10)
        ->  Sort  (cost=149.78..155.13 rows=2140 width=14)
              Sort Key: x.col1
              ->  Seq Scan on x  (cost=0.00..31.40 rows=2140 width=14)

Those plans could use some love and attention before forcing Boxuan to
implement that.

 
 
It seems that we have not decided whether to put the inheritance for MERGE off for a latter implementation. But, I think we can discuss how to do it now.  
 
 First of all, the inheritance of MERGE should not be implemented in the rule-like way. I agree that the easy way I proposed is not consistent with the general inheritance process in postgres.
 
The normal way of doing this is to handle it in planner, to be more specific, we need to extend the function "inheritance_planner()" for processing MERGE queries.
 
For UPDATE and DELETE commands (INSERT is not an inheritable command), if "inheritance_planner" finds that the target table has children tables, it will generate a list of queries. These queries are almost the same as the original query input by user, except for the different target relations. Each child table has it corresponding query in this list. 
 
This list of queries will then be processed by "grouping_planner()" and transformed into a list of plans. One most important work finished in this function is to extend the target list of target relations to make sure that all attributes of a target relation appears in the final result tuple of its plan.
 
As for MERGE command, we need to do the same thing. But, since the main query body is a LEFT JOIN query between source table and target table, the top-level target list is a combination of all the attributes from source table and target table. Thus, when we extend the target list, we should only extent the part of target relations, and keep the source table part untouched.
 
Once a main query in this style has been transformed to plan, we need to prepare the merge actions for it too. That is, extend the target list of all UPDATE and INSERT actions for the corresponding target relation. In this way, each target relation will have its own "main plan + merge action" set.
 
The main plan will be executed one by one, so is the merge action sets, each for one target relation.
 
One more thing I want to point out is that, the INSERT is also an inheritable action in MERGE. For a plain INSERT command, all the inserted tuples are put in the target table ONLY. It is easy to understand. We don't want to duplicate all the new tuples in all children tables. However, in MERGE command, an INSERT action is activated by the tuples fitting its matching conditions. The main plan of a MERGE command will scan all the tuples in target relation and its children tables. If one tuple in a child table meets the requirements of INSERT actions, the insertion should be taken on the child table itself rather than its ancestor.  
 
PS: Since I have taken this project, I will do my best to make it perfect. I will keep working on MERGE until it is really finished, even after the gSoC. (unless you guys has other plans).
 
--
 Simon Riggs           www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


Re: MERGE command for inheritance

From
Robert Haas
Date:
On Wed, Aug 11, 2010 at 10:09 AM, Boxuan Zhai <bxzhai2010@gmail.com> wrote:
> PS: Since I have taken this project, I will do my best to make it perfect.
> I will keep working on MERGE until it is really finished, even after the
> gSoC. (unless you guys has other plans).

That is great to hear!

FWIW, I agree with Heikki that we should try to have the inheritance
stuff working properly in the first version.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company


Re: MERGE command for inheritance

From
Simon Riggs
Date:
On Wed, 2010-08-11 at 22:09 +0800, Boxuan Zhai wrote:
> One more thing I want to point out is that, the INSERT is also an
> inheritable action in MERGE. For a plain INSERT command, all the
> inserted tuples are put in the target table ONLY. It is easy to
> understand. We don't want to duplicate all the new tuples in all
> children tables. However, in MERGE command, an INSERT action is
> activated by the tuples fitting its matching conditions. The main plan
> of a MERGE command will scan all the tuples in target relation and its
> children tables. If one tuple in a child table meets the
> requirements of INSERT actions, the insertion should be taken on the
> child table itself rather than its ancestor.  

It seems clear that your work in this area will interfere with the work
on partitioning and insert routing. We've seen it time and time again
that big projects that aim to deliver towards end of a release cycle
interfere with dev of other projects and leave loose ends from
unforeseen interactions. There's no need for that.

> PS: Since I have taken this project, I will do my best to make it
> perfect. I will keep working on MERGE until it is really finished,
> even after the gSoC. (unless you guys has other plans).

You can make things perfect in more than one phase, as indeed you
already are: concurrent locking has already been placed out of scope of
your current work.

I don't question your good intentions to both complete this work and do
it on time. I question the need for us to rely on that. I also question
the ability of the community to deliver super-size features in a single
release. Breaking things down is always the best way.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Development, 24x7 Support, Training and Services



Re: MERGE command for inheritance

From
Heikki Linnakangas
Date:
On 11/08/10 17:45, Simon Riggs wrote:
> It seems clear that your work in this area will interfere with the work
> on partitioning and insert routing.

Nothing concrete has come out of that work yet. And we should have MERGE 
work with inherited tables, regardless of any future work that may 
happen with partitioning.

> We've seen it time and time again
> that big projects that aim to deliver towards end of a release cycle
> interfere with dev of other projects and leave loose ends from
> unforeseen interactions. There's no need for that.

I don't understand what you're saying, we're not in the end of a release 
cycle.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: MERGE command for inheritance

From
Tom Lane
Date:
Simon Riggs <simon@2ndQuadrant.com> writes:
> On Wed, 2010-08-11 at 13:25 +0300, Heikki Linnakangas wrote:
>> I concur that Boxuan's suggested "difficult" approach seems like the 
>> right one. 

> Right, but you've completely ignored my proposal: lets do this in two
> pieces. Get what we have now ready to commit, then add support for
> partitioning later, as a second project.

Do we really think this is anywhere near committable now?

If it's committable in every other respect, I could see just having it
throw a NOT_IMPLEMENTED error when the target table has children.
I thought we were still a very long way from that though.
        regards, tom lane


Re: MERGE command for inheritance

From
Simon Riggs
Date:
On Wed, 2010-08-11 at 11:03 -0400, Tom Lane wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
> > On Wed, 2010-08-11 at 13:25 +0300, Heikki Linnakangas wrote:
> >> I concur that Boxuan's suggested "difficult" approach seems like the 
> >> right one. 
> 
> > Right, but you've completely ignored my proposal: lets do this in two
> > pieces. Get what we have now ready to commit, then add support for
> > partitioning later, as a second project.
> 
> Do we really think this is anywhere near committable now?
> 
> If it's committable in every other respect, I could see just having it
> throw a NOT_IMPLEMENTED error when the target table has children.
> I thought we were still a very long way from that though.

Well, if we go off chasing this particular goose then we will set
ourselves back at least one commitfest. I'd rather work towards having a
fully committable patch without inheritance sooner than an even bigger
patch arriving later in the cycle, which could make things difficult for
us.

I cite recent big patch experience as admissible evidence, m'lord.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Development, 24x7 Support, Training and Services



Re: MERGE command for inheritance

From
Robert Haas
Date:
On Wed, Aug 11, 2010 at 11:23 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> Well, if we go off chasing this particular goose then we will set
> ourselves back at least one commitfest. I'd rather work towards having a
> fully committable patch without inheritance sooner than an even bigger
> patch arriving later in the cycle, which could make things difficult for
> us.

Let's give Boxuan a little time to work and see what he comes up with.Maybe it won't be too bad.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company


Re: MERGE command for inheritance

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 11, 2010 at 11:23 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> Well, if we go off chasing this particular goose then we will set
>> ourselves back at least one commitfest. I'd rather work towards having a
>> fully committable patch without inheritance sooner than an even bigger
>> patch arriving later in the cycle, which could make things difficult for
>> us.

> Let's give Boxuan a little time to work and see what he comes up with.
>  Maybe it won't be too bad.

I tend to agree with Simon's argument here: if the patch is near
committable then it'd be better to get it committed and work on
correcting this omission afterwards.  I'm not sure about the truth of
the "if" part, though.
        regards, tom lane


Re: MERGE command for inheritance

From
Greg Smith
Date:
Tom Lane wrote:
> Do we really think this is anywhere near committable now?
>

There's a relatively objective standard for the first thing needed for
commit--does it work?--in the form of the regression tests Simon put
together before development.  I just tried the latest merge_v102.patch
(regression diff attached) to see how that's going.  There are still a
couple of errors in there.  It looks to me like the error handling and
related DO NOTHING support are the next pair of things that patch needs
work on.  I'd rather see that sorted out than to march onward to
inheritance without the fundamentals even nailed down yet.

--
Greg Smith  2ndQuadrant US  Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com   www.2ndQuadrant.us

*** /home/postgres/pgwork/repo/git/postgresql/src/test/regress/expected/merge.out    2010-08-11 12:23:50.000000000
-0400
--- /home/postgres/pgwork/repo/git/postgresql/src/test/regress/results/merge.out    2010-08-11 12:33:27.000000000 -0400
***************
*** 44,57 ****
  WHEN MATCHED THEN
      UPDATE SET balance = t.balance + s.balance
  ;
! SELECT * FROM target;
!  id | balance
! ----+---------
!   1 |      10
!   2 |      25
!   3 |      50
! (3 rows)
!
  ROLLBACK;
  -- do a simple equivalent of an INSERT SELECT
  BEGIN;
--- 44,50 ----
  WHEN MATCHED THEN
      UPDATE SET balance = t.balance + s.balance
  ;
! NOTICE:  one tuple is ERROR
  ROLLBACK;
  -- do a simple equivalent of an INSERT SELECT
  BEGIN;
***************
*** 61,66 ****
--- 54,61 ----
  WHEN NOT MATCHED THEN
      INSERT VALUES (s.id, s.balance)
  ;
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
  SELECT * FROM target;
   id | balance
  ----+---------
***************
*** 102,107 ****
--- 97,103 ----
  WHEN MATCHED THEN
      DELETE
  ;
+ NOTICE:  one tuple is ERROR
  SELECT * FROM target;
   id | balance
  ----+---------
***************
*** 165,176 ****
  ERROR:  multiple actions on single target row

  ROLLBACK;
!
  -- This next SQL statement
  --  fails according to standard
  --  suceeds in PostgreSQL implementation by simply ignoring the second
  --  matching row since it activates no WHEN clause
  BEGIN;
  MERGE into target t
  USING (select * from source) AS s
  ON t.id = s.id
--- 161,175 ----
  ERROR:  multiple actions on single target row

  ROLLBACK;
! ERROR:  syntax error at or near "ERROR"
! LINE 1: ERROR:  multiple actions on single target row
!         ^
  -- This next SQL statement
  --  fails according to standard
  --  suceeds in PostgreSQL implementation by simply ignoring the second
  --  matching row since it activates no WHEN clause
  BEGIN;
+ ERROR:  current transaction is aborted, commands ignored until end of transaction block
  MERGE into target t
  USING (select * from source) AS s
  ON t.id = s.id
***************
*** 179,184 ****
--- 178,184 ----
  WHEN NOT MATCHED THEN
      INSERT VALUES (s.id, s.balance)
  ;
+ ERROR:  current transaction is aborted, commands ignored until end of transaction block
  ROLLBACK;
  -- Now lets prepare the test data to generate 2 non-matching rows
  DELETE FROM source WHERE id = 3 AND balance = 5;
***************
*** 188,195 ****
  ----+---------
    2 |       5
    3 |      20
-   4 |       5
    4 |      40
  (4 rows)

  -- This next SQL statement
--- 188,195 ----
  ----+---------
    2 |       5
    3 |      20
    4 |      40
+   4 |       5
  (4 rows)

  -- This next SQL statement
***************
*** 203,216 ****
  WHEN NOT MATCHED THEN
      INSERT VALUES (s.id, s.balance)
  ;
  SELECT * FROM target;
   id | balance
  ----+---------
    1 |      10
    2 |      20
    3 |      30
-   4 |       5
    4 |      40
  (5 rows)

  ROLLBACK;
--- 203,218 ----
  WHEN NOT MATCHED THEN
      INSERT VALUES (s.id, s.balance)
  ;
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
  SELECT * FROM target;
   id | balance
  ----+---------
    1 |      10
    2 |      20
    3 |      30
    4 |      40
+   4 |       5
  (5 rows)

  ROLLBACK;
***************
*** 225,239 ****
  WHEN NOT MATCHED AND s.balance > 100 THEN
      INSERT VALUES (s.id, s.balance)
  ;
  SELECT * FROM target;
   id | balance
  ----+---------
    1 |      10
    2 |      20
    3 |      30
!     |
!     |
! (5 rows)

  ROLLBACK;
  -- This next SQL statement suceeds, but does nothing since there are
--- 227,243 ----
  WHEN NOT MATCHED AND s.balance > 100 THEN
      INSERT VALUES (s.id, s.balance)
  ;
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
  SELECT * FROM target;
   id | balance
  ----+---------
    1 |      10
    2 |      20
    3 |      30
! (3 rows)

  ROLLBACK;
  -- This next SQL statement suceeds, but does nothing since there are
***************
*** 249,262 ****
  WHEN NOT MATCHED
      DO NOTHING
  ;
  SELECT * FROM target;
!  id | balance
! ----+---------
!   1 |      10
!   2 |      20
!   3 |      30
! (3 rows)
!
  ROLLBACK;
  --
  -- Weirdness
--- 253,263 ----
  WHEN NOT MATCHED
      DO NOTHING
  ;
+ ERROR:  syntax error at or near "DO"
+ LINE 7:  DO NOTHING
+          ^
  SELECT * FROM target;
! ERROR:  current transaction is aborted, commands ignored until end of transaction block
  ROLLBACK;
  --
  -- Weirdness

======================================================================


Re: MERGE command for inheritance

From
Alvaro Herrera
Date:
Excerpts from Heikki Linnakangas's message of mié ago 11 10:52:24 -0400 2010:
> On 11/08/10 17:45, Simon Riggs wrote:

> > We've seen it time and time again
> > that big projects that aim to deliver towards end of a release cycle
> > interfere with dev of other projects and leave loose ends from
> > unforeseen interactions. There's no need for that.
> 
> I don't understand what you're saying, we're not in the end of a release 
> cycle.

This patch still needs a lot of work before it's anywhere close to
committable.  I agree with Simon that it is preferrable to clean it up
to make it committable *without* the burden of extra features.  If
Boxuan continues to add more features, it will be end-of-release before
it is possible to think about committing it.

It seems better to have merge-no-inheritance in 9.1 than nothing.  If we
can get the inheritance case working for 9.1, that's even better, but I
don't think it needs to be a hard requirement.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: MERGE command for inheritance

From
Boxuan Zhai
Date:


On Thu, Aug 12, 2010 at 12:49 AM, Greg Smith <greg@2ndquadrant.com> wrote:
Tom Lane wrote:
Do we really think this is anywhere near committable now?
 

There's a relatively objective standard for the first thing needed for commit--does it work?--in the form of the regression tests Simon put together before development.  I just tried the latest merge_v102.patch (regression diff attached) to see how that's going.  There are still a couple of errors in there.  It looks to me like the error handling and related DO NOTHING support are the next pair of things that patch needs work on.  I'd rather see that sorted out than to march onward to inheritance without the fundamentals even nailed down yet.

 
 
Sorry for the mismatch problem of regress. In fact, I am still unable to make the regression test run on my machine. When I try the command pg_regreess in /src/test/regress/, it always gives a FATAL error:
 
FATAL:  parameter "port" cannot be changed without restarting the server
psql: FATAL:  parameter "port" cannot be changed without restarting the server
command failed: ""C:/msys/1.0/local/pgsql/bin//psql" -X -c "DROP DATABASE IF EXISTS \"regression\"" "postgres""
 
However, I can run this command directly in the MinGW command line interface. I guess this is because the psql_command() function has some check before accept commands. And the MinGW environment  cannot pass these checks.
 
All the SQL commands in the .sql file have been tested by hand. And they are all correct. However, the .out file is not automatic generated by pgsql.
 
I may need to find a linux system to try to generate the correct .out file some other time. Or, would someone help me to generate an .out file through pg_regress?
 
--
Greg Smith  2ndQuadrant US  Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com   www.2ndQuadrant.us


*** /home/postgres/pgwork/repo/git/postgresql/src/test/regress/expected/merge.out       2010-08-11 12:23:50.000000000 -0400
--- /home/postgres/pgwork/repo/git/postgresql/src/test/regress/results/merge.out        2010-08-11 12:33:27.000000000 -0400
***************
*** 44,57 ****
 WHEN MATCHED THEN
       UPDATE SET balance = t.balance + s.balance
 ;
! SELECT * FROM target;
!  id | balance
! ----+---------
!   1 |      10
!   2 |      25
!   3 |      50
! (3 rows)
!
 ROLLBACK;
 -- do a simple equivalent of an INSERT SELECT
 BEGIN;
--- 44,50 ----
 WHEN MATCHED THEN
       UPDATE SET balance = t.balance + s.balance
 ;
! NOTICE:  one tuple is ERROR
 ROLLBACK;
 -- do a simple equivalent of an INSERT SELECT
 BEGIN;
***************
*** 61,66 ****
--- 54,61 ----
 WHEN NOT MATCHED THEN
       INSERT VALUES (s.id, s.balance)
 ;
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
 SELECT * FROM target;
  id | balance
 ----+---------
***************
*** 102,107 ****
--- 97,103 ----
 WHEN MATCHED THEN
       DELETE
 ;
+ NOTICE:  one tuple is ERROR
 SELECT * FROM target;
  id | balance
 ----+---------
***************
*** 165,176 ****
 ERROR:  multiple actions on single target row

 ROLLBACK;
!
 -- This next SQL statement
 --  fails according to standard
 --  suceeds in PostgreSQL implementation by simply ignoring the second
 --  matching row since it activates no WHEN clause
 BEGIN;
 MERGE into target t
 USING (select * from source) AS s
 ON t.id = s.id
--- 161,175 ----
 ERROR:  multiple actions on single target row

 ROLLBACK;
! ERROR:  syntax error at or near "ERROR"
! LINE 1: ERROR:  multiple actions on single target row
!         ^
 -- This next SQL statement
 --  fails according to standard
 --  suceeds in PostgreSQL implementation by simply ignoring the second
 --  matching row since it activates no WHEN clause
 BEGIN;
+ ERROR:  current transaction is aborted, commands ignored until end of transaction block
 MERGE into target t
 USING (select * from source) AS s
 ON t.id = s.id
***************
*** 179,184 ****
--- 178,184 ----
 WHEN NOT MATCHED THEN
       INSERT VALUES (s.id, s.balance)
 ;
+ ERROR:  current transaction is aborted, commands ignored until end of transaction block
 ROLLBACK;
 -- Now lets prepare the test data to generate 2 non-matching rows
 DELETE FROM source WHERE id = 3 AND balance = 5;
***************
*** 188,195 ****
 ----+---------
   2 |       5
   3 |      20
-   4 |       5
   4 |      40
 (4 rows)

 -- This next SQL statement
--- 188,195 ----
 ----+---------
   2 |       5
   3 |      20
   4 |      40
+   4 |       5
 (4 rows)

 -- This next SQL statement
***************
*** 203,216 ****
 WHEN NOT MATCHED THEN
       INSERT VALUES (s.id, s.balance)
 ;
 SELECT * FROM target;
  id | balance
 ----+---------
   1 |      10
   2 |      20
   3 |      30
-   4 |       5
   4 |      40
 (5 rows)

 ROLLBACK;
--- 203,218 ----
 WHEN NOT MATCHED THEN
       INSERT VALUES (s.id, s.balance)
 ;
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
 SELECT * FROM target;
  id | balance
 ----+---------
   1 |      10
   2 |      20
   3 |      30
   4 |      40
+   4 |       5
 (5 rows)

 ROLLBACK;
***************
*** 225,239 ****
 WHEN NOT MATCHED AND s.balance > 100 THEN
       INSERT VALUES (s.id, s.balance)
 ;
 SELECT * FROM target;
  id | balance
 ----+---------
   1 |      10
   2 |      20
   3 |      30
!     |
!     |
! (5 rows)

 ROLLBACK;
 -- This next SQL statement suceeds, but does nothing since there are
--- 227,243 ----
 WHEN NOT MATCHED AND s.balance > 100 THEN
       INSERT VALUES (s.id, s.balance)
 ;
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
+ NOTICE:  one tuple is ERROR
 SELECT * FROM target;
  id | balance
 ----+---------
   1 |      10
   2 |      20
   3 |      30
! (3 rows)

 ROLLBACK;
 -- This next SQL statement suceeds, but does nothing since there are
***************
*** 249,262 ****
 WHEN NOT MATCHED
       DO NOTHING
 ;
 SELECT * FROM target;
!  id | balance
! ----+---------
!   1 |      10
!   2 |      20
!   3 |      30
! (3 rows)
!
 ROLLBACK;
 --
 -- Weirdness
--- 253,263 ----
 WHEN NOT MATCHED
       DO NOTHING
 ;
+ ERROR:  syntax error at or near "DO"
+ LINE 7:  DO NOTHING
+          ^
 SELECT * FROM target;
! ERROR:  current transaction is aborted, commands ignored until end of transaction block
 ROLLBACK;
 --
 -- Weirdness

======================================================================



Re: MERGE command for inheritance

From
Robert Haas
Date:
On Thu, Aug 12, 2010 at 2:24 AM, Boxuan Zhai <bxzhai2010@gmail.com> wrote:
> Sorry for the mismatch problem of regress. In fact, I am still unable to
> make the regression test run on my machine. When I try the command
> pg_regreess in /src/test/regress/, it always gives a FATAL error:

The intention is that you should run "make check" in that directory.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company


Re: MERGE command for inheritance

From
Boxuan Zhai
Date:

I have renewed the merge.sql and merge.out in regress. Please have a look.
Attachment

Re: MERGE command for inheritance

From
Heikki Linnakangas
Date:
On 13/08/10 09:27, Boxuan Zhai wrote:
> I have renewed the merge.sql and merge.out in regress. Please have a look.

Thanks.

Did you change the way DO INSTEAD rules are handled already? 
http://archives.postgresql.org/pgsql-hackers/2010-08/msg00151.php

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: MERGE command for inheritance

From
Boxuan Zhai
Date:


On Fri, Aug 13, 2010 at 2:33 PM, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
On 13/08/10 09:27, Boxuan Zhai wrote:
I have renewed the merge.sql and merge.out in regress. Please have a look.

Thanks.

Did you change the way DO INSTEAD rules are handled already? http://archives.postgresql.org/pgsql-hackers/2010-08/msg00151.php

 
Yes. This mistake has been corrected a few editions ago. Now, the actions replaced by INSTEAD rules will still catch tuples but do nothing for them.
--
 Heikki Linnakangas

 EnterpriseDB   http://www.enterprisedb.com