You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafodion.apache.org by Dave Birdsall <da...@esgyn.com> on 2015/09/08 23:43:33 UTC
Bug in MERGE/UPSERT transformation
Hi,
I’ve been playing with costing code (specifically, adding costing code for
HBase deletes and updates). While debugging these changes, I ran executor
regressions. TEST015 fails, with some MERGE and UPSERT statements giving
wrong results.
In a correct run (with my changes turned off), I see a plan like this:
>>explain options 'f' merge into t015t1 on a = 1 when matched then update
set b = 01
+> when not matched then insert values (1,2);
LC RC OP OPERATOR OPT DESCRIPTION CARD
---- ---- ---- -------------------- -------- --------------------
---------
9 . 10 root o x
4.00E+000
1 8 9 nested_join
4.00E+000
4 7 8 merge_union
4.00E+000
5 6 7 blocked_union
2.00E+000
. . 6 trafodion_insert T015T1I2
1.00E+000
. . 5 trafodion_vsbb_delet T015T1I2
1.00E+000
2 3 4 blocked_union
2.00E+000
. . 3 trafodion_insert T015T1I1
1.00E+000
. . 2 trafodion_delete T015T1I1
1.00E+000
. . 1 trafodion_merge T015T1
1.00E+000
--- SQL operation complete.
>>log;
In a failed run (with my changes turned on), I see a plan like this:
>>explain options 'f' merge into t015t1 on a=1 when matched then
+> update set b = -1
+> when not matched then insert values (1,2);
LC RC OP OPERATOR OPT DESCRIPTION CARD
---- ---- ---- -------------------- -------- --------------------
---------
11 . 12 root x
4.00E+000
3 10 11 nested_join
4.00E+000
6 9 10 merge_union
4.00E+000
7 8 9 blocked_union
2.00E+000
. . 8 trafodion_insert T015T1I2
1.00E+000
. . 7 trafodion_vsbb_delet T015T1I2
1.00E+000
4 5 6 blocked_union
2.00E+000
. . 5 trafodion_insert T015T1I1
1.00E+000
. . 4 trafodion_delete T015T1I1
1.00E+000
1 2 3 nested_join
1.00E+000
. . 2 trafodion_merge T015T1
1.00E+000
. . 1 trafodion_scan T015T1
1.00E+000
--- SQL operation complete.
In this example, the table T015T1 has two indexes, which is why the
additional operators are present. The index maintenance part looks correct
in both plans.
The thing that looks incorrect is the transformation of a “trafodion merge”
to the base table into a “nested join” of a scan of that table and a
“trafodion merge” of that table. Such a transformation would be correct for
updates and deletes (and indeed we do this for delete in order to take
advantage of the HBase API that allows multiple deletes on one call; that
shows up in explain as “trafodion_vsbb_dele”). But I don’t think it should
ever be correct for a merge/upsert, since part of the action of a
merge/upsert depends on the **absence** of particular keys. A scan + nested
join cannot express absence, only presence.
I think the fix is in TSJFlowRule::topMatch (in optimizer/Transrule.cpp),
we should return false if expr->isMerge() is true.
What do you think?
Dave
Re: Bug in MERGE/UPSERT transformation
Posted by Suresh Subbiah <su...@gmail.com>.
Hi Dave,
Yes MERGE can take an input row from its parent. We have a few test cases
in executor/test015 that get such plans today. I think the second plan you
showed should work, though it is hard to see how it could be more efficient
than the first (current plan).
Will it be useful to look at optimizer with Display on a code base without
your changes (or disabled). The values clause gets eliminated in the
optimizer for merge statement (when it has only one row) and survives with
a flow node (similar to the plan 2 you showed) when the values list has
more than one row.
I have found these queries useful when working on the MERGE transformation.
Hope they help here too. Statement s1 is equivalent to s2 here.
Thanks
Suresh
prepare s1 from upsert into test1 values (1,1) ;
prepare s2 from merge into test1 on a = 1
when matched then update set b = 1
when not matched then insert values (1,1) ;
prepare s1 from upsert into test1 select * from test2 ;
prepare s2 from merge into test1 using (select * from test2) z(a,b) on
test1.a = z.a
when matched then update set b = z.b
when not matched then insert values (z.a,z.b) ;
prepare s1 from upsert into test1 values (1,1),(2,2) ;
prepare s2 from merge into test1 using (values (1,1),(2,2)) z(a,b) on
test1.a = z.a
when matched then update set b = z.b
when not matched then insert values (z.a,z.b) ;
CREATE TABLE TRAFODION.SEABASE.TEST1
(
A INT NO DEFAULT NOT NULL NOT DROPPABLE
, B INT DEFAULT NULL
, PRIMARY KEY (A ASC)
)
;
CREATE INDEX IDX1 ON TRAFODION.SEABASE.TEST1
(
B ASC
)
;
create table test2 like test1 ;
On Wed, Sep 9, 2015 at 6:39 PM, Dave Birdsall <da...@esgyn.com>
wrote:
> Hi,
>
> Well, that wasn't it either. Turns out with that fix, some other MERGE
> statements don't compile. So evidently that TSJRule is essential for some
> plans.
>
> I think I'm guilty of making unfounded assumptions about how MERGE works at
> runtime. So the next avenue of investigation is to get that understanding.
> Perhaps my bug is a run-time bug rather than an optimizer bug. Or it may be
> that the optimizer bug is more subtle than first thought.
>
> In case anyone is interested, I'll keep you posted on what I learn.
>
> Dave
>
> -----Original Message-----
> From: Dave Birdsall [mailto:dave.birdsall@esgyn.com]
> Sent: Wednesday, September 9, 2015 11:26 AM
> To: 'dev@trafodion.incubator.apache.org'
> <de...@trafodion.incubator.apache.org>
> Subject: RE: Bug in MERGE/UPSERT transformation
>
> Hi,
>
> I think I found it. I had the right idea originally, just the wrong rule.
>
> The TSJRule was firing on the MERGE node and transforming it to a TSJ of
> its
> scan to the merge. Which is not semantically correct because the MERGE has
> to do something when the scan returns nothing.
>
> I added a check "if (updateExpr->isMerge()) return FALSE;" to
> TSJRule::topMatch, and I get a correct plan now.
>
> Thanks, Suresh, for suggesting use of the DISPLAY tool. I was able to find
> the culprit rule very quickly by stepping through the tasks for the group
> containing the merge node. It's my first experience finding and fixing an
> Optimizer bug using DISPLAY and it was a very positive experience.
>
> Dave
>
> -----Original Message-----
> From: Suresh Subbiah [mailto:suresh.subbiah60@gmail.com]
> Sent: Tuesday, September 8, 2015 5:10 PM
> To: dev@trafodion.incubator.apache.org
> Subject: Re: Bug in MERGE/UPSERT transformation
>
> Hi Dave,
>
> It is interesting that costing changes caused the second plan to be
> considered and chosen.
> I have been working on transforming UPSERT to MERGE recently and have often
> seen both kind of plans.
> The first kind (which you said works correctly) is what we get for
> something
> like UPSERT into t values (1,1,1) -- if t had 2 indexes
>
> We get a variant of second plan for these types of UPSERT statememts UPSERT
> into t values select * from t1 ; UPSERT into t values (1,1,1),(2,2,2) ;
> However the plans I saw for the second case differ from what you observed
> in
> that the scan drives the rest of the query tree and not just the merge.
> In other words the nested join has scan as its left child and the topmost
> nested in the second plan as its right child.
>
> I don't know what is causing these new plans, but feel it has to be through
> one of the optimizer rules. We can verify that with the display tool.
> Looking forward to learn from your findings.
>
> Thanks
> Suresh
>
> PS All these UPSERT statements can be cast into equivalent MEGE statements
> if necessary
>
> On Tue, Sep 8, 2015 at 5:51 PM, Dave Birdsall <da...@esgyn.com>
> wrote:
>
> > Hi,
> >
> >
> >
> > I found out that it wasn’t that easy. Tried the proposed fix but that
> > didn’t solve the problem. And indeed the TSJFlowRule::topMatch wasn’t
> > firing anyway since expr->getNoFlow() was returning TRUE. So the join
> > transformation was being introduced somewhere else.
> >
> >
> >
> > After some more experimentation I learned that the table in question
> > has two indexes in the test script. And if I get rid of either one,
> > the bug does not occur. Which makes me think some transformation
> > involving multiple indexes is the culprit.
> >
> >
> >
> > I’ll keep debugging…
> >
> >
> >
> > Dave
> >
> >
> >
> > *From:* Dave Birdsall [mailto:dave.birdsall@esgyn.com]
> > *Sent:* Tuesday, September 8, 2015 2:44 PM
> > *To:* 'dev@trafodion.incubator.apache.org' <
> > dev@trafodion.incubator.apache.org>
> > *Subject:* Bug in MERGE/UPSERT transformation
> >
> >
> >
> > Hi,
> >
> >
> >
> > I’ve been playing with costing code (specifically, adding costing code
> > for HBase deletes and updates). While debugging these changes, I ran
> > executor regressions. TEST015 fails, with some MERGE and UPSERT
> > statements giving wrong results.
> >
> >
> >
> > In a correct run (with my changes turned off), I see a plan like this:
> >
> >
> >
> > >>explain options 'f' merge into t015t1 on a = 1 when matched then
> > >>update
> > set b = 01
> >
> > +> when not matched then insert values (1,2);
> >
> >
> >
> > LC RC OP OPERATOR OPT DESCRIPTION CARD
> >
> > ---- ---- ---- -------------------- -------- --------------------
> > ---------
> >
> >
> >
> > 9 . 10 root o x
> > 4.00E+000
> >
> > 1 8 9 nested_join
> > 4.00E+000
> >
> > 4 7 8 merge_union
> > 4.00E+000
> >
> > 5 6 7 blocked_union
> > 2.00E+000
> >
> > . . 6 trafodion_insert T015T1I2
> > 1.00E+000
> >
> > . . 5 trafodion_vsbb_delet T015T1I2
> > 1.00E+000
> >
> > 2 3 4 blocked_union
> > 2.00E+000
> >
> > . . 3 trafodion_insert T015T1I1
> > 1.00E+000
> >
> > . . 2 trafodion_delete T015T1I1
> > 1.00E+000
> >
> > . . 1 trafodion_merge T015T1
> > 1.00E+000
> >
> >
> >
> > --- SQL operation complete.
> >
> > >>log;
> >
> >
> >
> > In a failed run (with my changes turned on), I see a plan like this:
> >
> >
> >
> > >>explain options 'f' merge into t015t1 on a=1 when matched then
> >
> > +> update set b = -1
> >
> > +> when not matched then insert values (1,2);
> >
> >
> >
> > LC RC OP OPERATOR OPT DESCRIPTION CARD
> >
> > ---- ---- ---- -------------------- -------- --------------------
> > ---------
> >
> >
> >
> > 11 . 12 root x
> > 4.00E+000
> >
> > 3 10 11 nested_join
> > 4.00E+000
> >
> > 6 9 10 merge_union
> > 4.00E+000
> >
> > 7 8 9 blocked_union
> > 2.00E+000
> >
> > . . 8 trafodion_insert T015T1I2
> > 1.00E+000
> >
> > . . 7 trafodion_vsbb_delet T015T1I2
> > 1.00E+000
> >
> > 4 5 6 blocked_union
> > 2.00E+000
> >
> > . . 5 trafodion_insert T015T1I1
> > 1.00E+000
> >
> > . . 4 trafodion_delete T015T1I1
> > 1.00E+000
> >
> > 1 2 3 nested_join
> > 1.00E+000
> >
> > . . 2 trafodion_merge T015T1
> > 1.00E+000
> >
> > . . 1 trafodion_scan T015T1
> > 1.00E+000
> >
> >
> >
> > --- SQL operation complete.
> >
> >
> >
> > In this example, the table T015T1 has two indexes, which is why the
> > additional operators are present. The index maintenance part looks
> > correct in both plans.
> >
> >
> >
> > The thing that looks incorrect is the transformation of a “trafodion
> > merge”
> > to the base table into a “nested join” of a scan of that table and a
> > “trafodion merge” of that table. Such a transformation would be
> > correct for updates and deletes (and indeed we do this for delete in
> > order to take advantage of the HBase API that allows multiple deletes
> > on one call; that shows up in explain as “trafodion_vsbb_dele”). But I
> > don’t think it should ever be correct for a merge/upsert, since part
> > of the action of a merge/upsert depends on the **absence** of
> > particular keys. A scan + nested join cannot express absence, only
> > presence.
> >
> >
> >
> > I think the fix is in TSJFlowRule::topMatch (in
> > optimizer/Transrule.cpp), we should return false if expr->isMerge() is
> > true.
> >
> >
> >
> > What do you think?
> >
> >
> >
> > Dave
> >
>
RE: Bug in MERGE/UPSERT transformation
Posted by Dave Birdsall <da...@esgyn.com>.
Hi,
Well, that wasn't it either. Turns out with that fix, some other MERGE
statements don't compile. So evidently that TSJRule is essential for some
plans.
I think I'm guilty of making unfounded assumptions about how MERGE works at
runtime. So the next avenue of investigation is to get that understanding.
Perhaps my bug is a run-time bug rather than an optimizer bug. Or it may be
that the optimizer bug is more subtle than first thought.
In case anyone is interested, I'll keep you posted on what I learn.
Dave
-----Original Message-----
From: Dave Birdsall [mailto:dave.birdsall@esgyn.com]
Sent: Wednesday, September 9, 2015 11:26 AM
To: 'dev@trafodion.incubator.apache.org'
<de...@trafodion.incubator.apache.org>
Subject: RE: Bug in MERGE/UPSERT transformation
Hi,
I think I found it. I had the right idea originally, just the wrong rule.
The TSJRule was firing on the MERGE node and transforming it to a TSJ of its
scan to the merge. Which is not semantically correct because the MERGE has
to do something when the scan returns nothing.
I added a check "if (updateExpr->isMerge()) return FALSE;" to
TSJRule::topMatch, and I get a correct plan now.
Thanks, Suresh, for suggesting use of the DISPLAY tool. I was able to find
the culprit rule very quickly by stepping through the tasks for the group
containing the merge node. It's my first experience finding and fixing an
Optimizer bug using DISPLAY and it was a very positive experience.
Dave
-----Original Message-----
From: Suresh Subbiah [mailto:suresh.subbiah60@gmail.com]
Sent: Tuesday, September 8, 2015 5:10 PM
To: dev@trafodion.incubator.apache.org
Subject: Re: Bug in MERGE/UPSERT transformation
Hi Dave,
It is interesting that costing changes caused the second plan to be
considered and chosen.
I have been working on transforming UPSERT to MERGE recently and have often
seen both kind of plans.
The first kind (which you said works correctly) is what we get for something
like UPSERT into t values (1,1,1) -- if t had 2 indexes
We get a variant of second plan for these types of UPSERT statememts UPSERT
into t values select * from t1 ; UPSERT into t values (1,1,1),(2,2,2) ;
However the plans I saw for the second case differ from what you observed in
that the scan drives the rest of the query tree and not just the merge.
In other words the nested join has scan as its left child and the topmost
nested in the second plan as its right child.
I don't know what is causing these new plans, but feel it has to be through
one of the optimizer rules. We can verify that with the display tool.
Looking forward to learn from your findings.
Thanks
Suresh
PS All these UPSERT statements can be cast into equivalent MEGE statements
if necessary
On Tue, Sep 8, 2015 at 5:51 PM, Dave Birdsall <da...@esgyn.com>
wrote:
> Hi,
>
>
>
> I found out that it wasn’t that easy. Tried the proposed fix but that
> didn’t solve the problem. And indeed the TSJFlowRule::topMatch wasn’t
> firing anyway since expr->getNoFlow() was returning TRUE. So the join
> transformation was being introduced somewhere else.
>
>
>
> After some more experimentation I learned that the table in question
> has two indexes in the test script. And if I get rid of either one,
> the bug does not occur. Which makes me think some transformation
> involving multiple indexes is the culprit.
>
>
>
> I’ll keep debugging…
>
>
>
> Dave
>
>
>
> *From:* Dave Birdsall [mailto:dave.birdsall@esgyn.com]
> *Sent:* Tuesday, September 8, 2015 2:44 PM
> *To:* 'dev@trafodion.incubator.apache.org' <
> dev@trafodion.incubator.apache.org>
> *Subject:* Bug in MERGE/UPSERT transformation
>
>
>
> Hi,
>
>
>
> I’ve been playing with costing code (specifically, adding costing code
> for HBase deletes and updates). While debugging these changes, I ran
> executor regressions. TEST015 fails, with some MERGE and UPSERT
> statements giving wrong results.
>
>
>
> In a correct run (with my changes turned off), I see a plan like this:
>
>
>
> >>explain options 'f' merge into t015t1 on a = 1 when matched then
> >>update
> set b = 01
>
> +> when not matched then insert values (1,2);
>
>
>
> LC RC OP OPERATOR OPT DESCRIPTION CARD
>
> ---- ---- ---- -------------------- -------- --------------------
> ---------
>
>
>
> 9 . 10 root o x
> 4.00E+000
>
> 1 8 9 nested_join
> 4.00E+000
>
> 4 7 8 merge_union
> 4.00E+000
>
> 5 6 7 blocked_union
> 2.00E+000
>
> . . 6 trafodion_insert T015T1I2
> 1.00E+000
>
> . . 5 trafodion_vsbb_delet T015T1I2
> 1.00E+000
>
> 2 3 4 blocked_union
> 2.00E+000
>
> . . 3 trafodion_insert T015T1I1
> 1.00E+000
>
> . . 2 trafodion_delete T015T1I1
> 1.00E+000
>
> . . 1 trafodion_merge T015T1
> 1.00E+000
>
>
>
> --- SQL operation complete.
>
> >>log;
>
>
>
> In a failed run (with my changes turned on), I see a plan like this:
>
>
>
> >>explain options 'f' merge into t015t1 on a=1 when matched then
>
> +> update set b = -1
>
> +> when not matched then insert values (1,2);
>
>
>
> LC RC OP OPERATOR OPT DESCRIPTION CARD
>
> ---- ---- ---- -------------------- -------- --------------------
> ---------
>
>
>
> 11 . 12 root x
> 4.00E+000
>
> 3 10 11 nested_join
> 4.00E+000
>
> 6 9 10 merge_union
> 4.00E+000
>
> 7 8 9 blocked_union
> 2.00E+000
>
> . . 8 trafodion_insert T015T1I2
> 1.00E+000
>
> . . 7 trafodion_vsbb_delet T015T1I2
> 1.00E+000
>
> 4 5 6 blocked_union
> 2.00E+000
>
> . . 5 trafodion_insert T015T1I1
> 1.00E+000
>
> . . 4 trafodion_delete T015T1I1
> 1.00E+000
>
> 1 2 3 nested_join
> 1.00E+000
>
> . . 2 trafodion_merge T015T1
> 1.00E+000
>
> . . 1 trafodion_scan T015T1
> 1.00E+000
>
>
>
> --- SQL operation complete.
>
>
>
> In this example, the table T015T1 has two indexes, which is why the
> additional operators are present. The index maintenance part looks
> correct in both plans.
>
>
>
> The thing that looks incorrect is the transformation of a “trafodion
> merge”
> to the base table into a “nested join” of a scan of that table and a
> “trafodion merge” of that table. Such a transformation would be
> correct for updates and deletes (and indeed we do this for delete in
> order to take advantage of the HBase API that allows multiple deletes
> on one call; that shows up in explain as “trafodion_vsbb_dele”). But I
> don’t think it should ever be correct for a merge/upsert, since part
> of the action of a merge/upsert depends on the **absence** of
> particular keys. A scan + nested join cannot express absence, only
> presence.
>
>
>
> I think the fix is in TSJFlowRule::topMatch (in
> optimizer/Transrule.cpp), we should return false if expr->isMerge() is
> true.
>
>
>
> What do you think?
>
>
>
> Dave
>
RE: Bug in MERGE/UPSERT transformation
Posted by Dave Birdsall <da...@esgyn.com>.
Hi,
I think I found it. I had the right idea originally, just the wrong rule.
The TSJRule was firing on the MERGE node and transforming it to a TSJ of its
scan to the merge. Which is not semantically correct because the MERGE has
to do something when the scan returns nothing.
I added a check "if (updateExpr->isMerge()) return FALSE;" to
TSJRule::topMatch, and I get a correct plan now.
Thanks, Suresh, for suggesting use of the DISPLAY tool. I was able to find
the culprit rule very quickly by stepping through the tasks for the group
containing the merge node. It's my first experience finding and fixing an
Optimizer bug using DISPLAY and it was a very positive experience.
Dave
-----Original Message-----
From: Suresh Subbiah [mailto:suresh.subbiah60@gmail.com]
Sent: Tuesday, September 8, 2015 5:10 PM
To: dev@trafodion.incubator.apache.org
Subject: Re: Bug in MERGE/UPSERT transformation
Hi Dave,
It is interesting that costing changes caused the second plan to be
considered and chosen.
I have been working on transforming UPSERT to MERGE recently and have often
seen both kind of plans.
The first kind (which you said works correctly) is what we get for something
like UPSERT into t values (1,1,1) -- if t had 2 indexes
We get a variant of second plan for these types of UPSERT statememts UPSERT
into t values select * from t1 ; UPSERT into t values (1,1,1),(2,2,2) ;
However the plans I saw for the second case differ from what you observed in
that the scan drives the rest of the query tree and not just the merge.
In other words the nested join has scan as its left child and the topmost
nested in the second plan as its right child.
I don't know what is causing these new plans, but feel it has to be through
one of the optimizer rules. We can verify that with the display tool.
Looking forward to learn from your findings.
Thanks
Suresh
PS All these UPSERT statements can be cast into equivalent MEGE statements
if necessary
On Tue, Sep 8, 2015 at 5:51 PM, Dave Birdsall <da...@esgyn.com>
wrote:
> Hi,
>
>
>
> I found out that it wasn’t that easy. Tried the proposed fix but that
> didn’t solve the problem. And indeed the TSJFlowRule::topMatch wasn’t
> firing anyway since expr->getNoFlow() was returning TRUE. So the join
> transformation was being introduced somewhere else.
>
>
>
> After some more experimentation I learned that the table in question
> has two indexes in the test script. And if I get rid of either one,
> the bug does not occur. Which makes me think some transformation
> involving multiple indexes is the culprit.
>
>
>
> I’ll keep debugging…
>
>
>
> Dave
>
>
>
> *From:* Dave Birdsall [mailto:dave.birdsall@esgyn.com]
> *Sent:* Tuesday, September 8, 2015 2:44 PM
> *To:* 'dev@trafodion.incubator.apache.org' <
> dev@trafodion.incubator.apache.org>
> *Subject:* Bug in MERGE/UPSERT transformation
>
>
>
> Hi,
>
>
>
> I’ve been playing with costing code (specifically, adding costing code
> for HBase deletes and updates). While debugging these changes, I ran
> executor regressions. TEST015 fails, with some MERGE and UPSERT
> statements giving wrong results.
>
>
>
> In a correct run (with my changes turned off), I see a plan like this:
>
>
>
> >>explain options 'f' merge into t015t1 on a = 1 when matched then
> >>update
> set b = 01
>
> +> when not matched then insert values (1,2);
>
>
>
> LC RC OP OPERATOR OPT DESCRIPTION CARD
>
> ---- ---- ---- -------------------- -------- --------------------
> ---------
>
>
>
> 9 . 10 root o x
> 4.00E+000
>
> 1 8 9 nested_join
> 4.00E+000
>
> 4 7 8 merge_union
> 4.00E+000
>
> 5 6 7 blocked_union
> 2.00E+000
>
> . . 6 trafodion_insert T015T1I2
> 1.00E+000
>
> . . 5 trafodion_vsbb_delet T015T1I2
> 1.00E+000
>
> 2 3 4 blocked_union
> 2.00E+000
>
> . . 3 trafodion_insert T015T1I1
> 1.00E+000
>
> . . 2 trafodion_delete T015T1I1
> 1.00E+000
>
> . . 1 trafodion_merge T015T1
> 1.00E+000
>
>
>
> --- SQL operation complete.
>
> >>log;
>
>
>
> In a failed run (with my changes turned on), I see a plan like this:
>
>
>
> >>explain options 'f' merge into t015t1 on a=1 when matched then
>
> +> update set b = -1
>
> +> when not matched then insert values (1,2);
>
>
>
> LC RC OP OPERATOR OPT DESCRIPTION CARD
>
> ---- ---- ---- -------------------- -------- --------------------
> ---------
>
>
>
> 11 . 12 root x
> 4.00E+000
>
> 3 10 11 nested_join
> 4.00E+000
>
> 6 9 10 merge_union
> 4.00E+000
>
> 7 8 9 blocked_union
> 2.00E+000
>
> . . 8 trafodion_insert T015T1I2
> 1.00E+000
>
> . . 7 trafodion_vsbb_delet T015T1I2
> 1.00E+000
>
> 4 5 6 blocked_union
> 2.00E+000
>
> . . 5 trafodion_insert T015T1I1
> 1.00E+000
>
> . . 4 trafodion_delete T015T1I1
> 1.00E+000
>
> 1 2 3 nested_join
> 1.00E+000
>
> . . 2 trafodion_merge T015T1
> 1.00E+000
>
> . . 1 trafodion_scan T015T1
> 1.00E+000
>
>
>
> --- SQL operation complete.
>
>
>
> In this example, the table T015T1 has two indexes, which is why the
> additional operators are present. The index maintenance part looks
> correct in both plans.
>
>
>
> The thing that looks incorrect is the transformation of a “trafodion
> merge”
> to the base table into a “nested join” of a scan of that table and a
> “trafodion merge” of that table. Such a transformation would be
> correct for updates and deletes (and indeed we do this for delete in
> order to take advantage of the HBase API that allows multiple deletes
> on one call; that shows up in explain as “trafodion_vsbb_dele”). But I
> don’t think it should ever be correct for a merge/upsert, since part
> of the action of a merge/upsert depends on the **absence** of
> particular keys. A scan + nested join cannot express absence, only
> presence.
>
>
>
> I think the fix is in TSJFlowRule::topMatch (in
> optimizer/Transrule.cpp), we should return false if expr->isMerge() is
> true.
>
>
>
> What do you think?
>
>
>
> Dave
>
Re: Bug in MERGE/UPSERT transformation
Posted by Suresh Subbiah <su...@gmail.com>.
Hi Dave,
It is interesting that costing changes caused the second plan to be
considered and chosen.
I have been working on transforming UPSERT to MERGE recently and have often
seen both kind of plans.
The first kind (which you said works correctly) is what we get for
something like
UPSERT into t values (1,1,1) -- if t had 2 indexes
We get a variant of second plan for these types of UPSERT statememts
UPSERT into t values select * from t1 ;
UPSERT into t values (1,1,1),(2,2,2) ;
However the plans I saw for the second case differ from what you observed
in that the scan drives the rest of the query tree and not just the merge.
In other words the nested join has scan as its left child and the topmost
nested in the second plan as its right child.
I don't know what is causing these new plans, but feel it has to be through
one of the optimizer rules. We can verify that with the display tool.
Looking forward to learn from your findings.
Thanks
Suresh
PS All these UPSERT statements can be cast into equivalent MEGE statements
if necessary
On Tue, Sep 8, 2015 at 5:51 PM, Dave Birdsall <da...@esgyn.com>
wrote:
> Hi,
>
>
>
> I found out that it wasn’t that easy. Tried the proposed fix but that
> didn’t solve the problem. And indeed the TSJFlowRule::topMatch wasn’t
> firing anyway since expr->getNoFlow() was returning TRUE. So the join
> transformation was being introduced somewhere else.
>
>
>
> After some more experimentation I learned that the table in question has
> two indexes in the test script. And if I get rid of either one, the bug
> does not occur. Which makes me think some transformation involving multiple
> indexes is the culprit.
>
>
>
> I’ll keep debugging…
>
>
>
> Dave
>
>
>
> *From:* Dave Birdsall [mailto:dave.birdsall@esgyn.com]
> *Sent:* Tuesday, September 8, 2015 2:44 PM
> *To:* 'dev@trafodion.incubator.apache.org' <
> dev@trafodion.incubator.apache.org>
> *Subject:* Bug in MERGE/UPSERT transformation
>
>
>
> Hi,
>
>
>
> I’ve been playing with costing code (specifically, adding costing code for
> HBase deletes and updates). While debugging these changes, I ran executor
> regressions. TEST015 fails, with some MERGE and UPSERT statements giving
> wrong results.
>
>
>
> In a correct run (with my changes turned off), I see a plan like this:
>
>
>
> >>explain options 'f' merge into t015t1 on a = 1 when matched then update
> set b = 01
>
> +> when not matched then insert values (1,2);
>
>
>
> LC RC OP OPERATOR OPT DESCRIPTION CARD
>
> ---- ---- ---- -------------------- -------- --------------------
> ---------
>
>
>
> 9 . 10 root o x
> 4.00E+000
>
> 1 8 9 nested_join
> 4.00E+000
>
> 4 7 8 merge_union
> 4.00E+000
>
> 5 6 7 blocked_union
> 2.00E+000
>
> . . 6 trafodion_insert T015T1I2
> 1.00E+000
>
> . . 5 trafodion_vsbb_delet T015T1I2
> 1.00E+000
>
> 2 3 4 blocked_union
> 2.00E+000
>
> . . 3 trafodion_insert T015T1I1
> 1.00E+000
>
> . . 2 trafodion_delete T015T1I1
> 1.00E+000
>
> . . 1 trafodion_merge T015T1
> 1.00E+000
>
>
>
> --- SQL operation complete.
>
> >>log;
>
>
>
> In a failed run (with my changes turned on), I see a plan like this:
>
>
>
> >>explain options 'f' merge into t015t1 on a=1 when matched then
>
> +> update set b = -1
>
> +> when not matched then insert values (1,2);
>
>
>
> LC RC OP OPERATOR OPT DESCRIPTION CARD
>
> ---- ---- ---- -------------------- -------- --------------------
> ---------
>
>
>
> 11 . 12 root x
> 4.00E+000
>
> 3 10 11 nested_join
> 4.00E+000
>
> 6 9 10 merge_union
> 4.00E+000
>
> 7 8 9 blocked_union
> 2.00E+000
>
> . . 8 trafodion_insert T015T1I2
> 1.00E+000
>
> . . 7 trafodion_vsbb_delet T015T1I2
> 1.00E+000
>
> 4 5 6 blocked_union
> 2.00E+000
>
> . . 5 trafodion_insert T015T1I1
> 1.00E+000
>
> . . 4 trafodion_delete T015T1I1
> 1.00E+000
>
> 1 2 3 nested_join
> 1.00E+000
>
> . . 2 trafodion_merge T015T1
> 1.00E+000
>
> . . 1 trafodion_scan T015T1
> 1.00E+000
>
>
>
> --- SQL operation complete.
>
>
>
> In this example, the table T015T1 has two indexes, which is why the
> additional operators are present. The index maintenance part looks correct
> in both plans.
>
>
>
> The thing that looks incorrect is the transformation of a “trafodion merge”
> to the base table into a “nested join” of a scan of that table and a
> “trafodion merge” of that table. Such a transformation would be correct for
> updates and deletes (and indeed we do this for delete in order to take
> advantage of the HBase API that allows multiple deletes on one call; that
> shows up in explain as “trafodion_vsbb_dele”). But I don’t think it should
> ever be correct for a merge/upsert, since part of the action of a
> merge/upsert depends on the **absence** of particular keys. A scan + nested
> join cannot express absence, only presence.
>
>
>
> I think the fix is in TSJFlowRule::topMatch (in optimizer/Transrule.cpp),
> we should return false if expr->isMerge() is true.
>
>
>
> What do you think?
>
>
>
> Dave
>
RE: Bug in MERGE/UPSERT transformation
Posted by Dave Birdsall <da...@esgyn.com>.
Hi,
I found out that it wasn’t that easy. Tried the proposed fix but that
didn’t solve the problem. And indeed the TSJFlowRule::topMatch wasn’t
firing anyway since expr->getNoFlow() was returning TRUE. So the join
transformation was being introduced somewhere else.
After some more experimentation I learned that the table in question has
two indexes in the test script. And if I get rid of either one, the bug
does not occur. Which makes me think some transformation involving multiple
indexes is the culprit.
I’ll keep debugging…
Dave
*From:* Dave Birdsall [mailto:dave.birdsall@esgyn.com]
*Sent:* Tuesday, September 8, 2015 2:44 PM
*To:* 'dev@trafodion.incubator.apache.org' <
dev@trafodion.incubator.apache.org>
*Subject:* Bug in MERGE/UPSERT transformation
Hi,
I’ve been playing with costing code (specifically, adding costing code for
HBase deletes and updates). While debugging these changes, I ran executor
regressions. TEST015 fails, with some MERGE and UPSERT statements giving
wrong results.
In a correct run (with my changes turned off), I see a plan like this:
>>explain options 'f' merge into t015t1 on a = 1 when matched then update
set b = 01
+> when not matched then insert values (1,2);
LC RC OP OPERATOR OPT DESCRIPTION CARD
---- ---- ---- -------------------- -------- --------------------
---------
9 . 10 root o x
4.00E+000
1 8 9 nested_join
4.00E+000
4 7 8 merge_union
4.00E+000
5 6 7 blocked_union
2.00E+000
. . 6 trafodion_insert T015T1I2
1.00E+000
. . 5 trafodion_vsbb_delet T015T1I2
1.00E+000
2 3 4 blocked_union
2.00E+000
. . 3 trafodion_insert T015T1I1
1.00E+000
. . 2 trafodion_delete T015T1I1
1.00E+000
. . 1 trafodion_merge T015T1
1.00E+000
--- SQL operation complete.
>>log;
In a failed run (with my changes turned on), I see a plan like this:
>>explain options 'f' merge into t015t1 on a=1 when matched then
+> update set b = -1
+> when not matched then insert values (1,2);
LC RC OP OPERATOR OPT DESCRIPTION CARD
---- ---- ---- -------------------- -------- --------------------
---------
11 . 12 root x
4.00E+000
3 10 11 nested_join
4.00E+000
6 9 10 merge_union
4.00E+000
7 8 9 blocked_union
2.00E+000
. . 8 trafodion_insert T015T1I2
1.00E+000
. . 7 trafodion_vsbb_delet T015T1I2
1.00E+000
4 5 6 blocked_union
2.00E+000
. . 5 trafodion_insert T015T1I1
1.00E+000
. . 4 trafodion_delete T015T1I1
1.00E+000
1 2 3 nested_join
1.00E+000
. . 2 trafodion_merge T015T1
1.00E+000
. . 1 trafodion_scan T015T1
1.00E+000
--- SQL operation complete.
In this example, the table T015T1 has two indexes, which is why the
additional operators are present. The index maintenance part looks correct
in both plans.
The thing that looks incorrect is the transformation of a “trafodion merge”
to the base table into a “nested join” of a scan of that table and a
“trafodion merge” of that table. Such a transformation would be correct for
updates and deletes (and indeed we do this for delete in order to take
advantage of the HBase API that allows multiple deletes on one call; that
shows up in explain as “trafodion_vsbb_dele”). But I don’t think it should
ever be correct for a merge/upsert, since part of the action of a
merge/upsert depends on the **absence** of particular keys. A scan + nested
join cannot express absence, only presence.
I think the fix is in TSJFlowRule::topMatch (in optimizer/Transrule.cpp),
we should return false if expr->isMerge() is true.
What do you think?
Dave