Обсуждение: MERGE command for inheritance
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.
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.
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
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.
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
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
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
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
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
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: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?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.
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.
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
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
On Wed, Aug 11, 2010 at 4:45 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
The support for UPDATE and SELECT of partitioned cases is very differentOn 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.
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
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
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
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
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
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
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
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
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
======================================================================
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
On Thu, Aug 12, 2010 at 12:49 AM, Greg Smith <greg@2ndquadrant.com> wrote:
Tom Lane wrote: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.Do we really think this is anywhere near committable now?
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""
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
======================================================================
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
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
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:Thanks.I have renewed the merge.sql and merge.out in regress. Please have a look.
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