You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by DaveBirdsall <gi...@git.apache.org> on 2018/02/27 00:27:48 UTC
[GitHub] trafodion pull request #1454: [TRAFODION-2969] Fix interaction of [first n] ...
GitHub user DaveBirdsall opened a pull request:
https://github.com/apache/trafodion/pull/1454
[TRAFODION-2969] Fix interaction of [first n] etc. with subqueries
Formerly, a [first n] in a subquery in a SELECT list resulted in an internal error in the Normalizer. The problem was that the Normalizer method Subquery::transformToRelExpr was not expecting a FirstN node at the top of the subquery tree.
Besides the fact that the Normalizer doesn't like it, having FirstN above the GroupByAgg node that enforces the one-row cardinality rule doesn't make sense.
In this set of changes, the FirstN node is placed underneath the GroupByAgg node when it matters. For [first n] / [any n] where n > 1, we can simply throw the [first n] / [any n] away, since the subquery already enforces at most one row semantics. For [first 1] and [last 1], we need to insert the FirstN node below the GroupByAgg node so that we narrow the result set down to one (or zero) rows before GroupByAgg enforces the cardinality rule. So, adding [first 1] to a subquery avoids cardinality violations. For [last 0] there is a subtlety. [last 0] by definition returns no rows, so we want the subquery to return null. The way we chose to do this is to force the creation of a one-row GroupByAgg node always, and put the FirstN node under that. In this way, GroupByAgg will see zero rows and produce a null result.
Regression test core/TEST002 has had many [first n] - subquery interaction tests added.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/DaveBirdsall/trafodion Trafodion2969
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1454.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1454
----
commit 2f2714d8fd719b92476e96642cd3fc4389523d92
Author: Dave Birdsall <db...@...>
Date: 2018-02-27T00:18:39Z
[TRAFODION-2969] Fix interaction of [first n] etc. with subqueries
----
---
[GitHub] trafodion pull request #1454: [TRAFODION-2969] Fix interaction of [first n] ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1454#discussion_r171021699
--- Diff: core/sql/optimizer/RelExpr.cpp ---
@@ -11388,8 +11388,9 @@ void RelRoot::setMvBindContext(MvBindContext * pMvBindContext)
pMvBindContextForScope_ = pMvBindContext;
}
-void RelRoot::addOneRowAggregates(BindWA* bindWA)
+NABoolean RelRoot::addOneRowAggregates(BindWA* bindWA, NABoolean forceGroupByAgg)
{
+ NABoolean GroupByAggNodeAdded = FALSE;
--- End diff --
Done; changed it to start with lower case letter
---
[GitHub] trafodion pull request #1454: [TRAFODION-2969] Fix interaction of [first n] ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1454
---
[GitHub] trafodion pull request #1454: [TRAFODION-2969] Fix interaction of [first n] ...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1454#discussion_r171012886
--- Diff: core/sql/regress/core/EXPECTED002.LINUX ---
@@ -464,22 +478,16 @@ A AMAX B BMAX C C
>>
>>
>>------------------------------------------------------------------------
->>?section Genesis_10_000222_6892_p1
->>SELECT 1 FROM T002T3 T1
-+>GROUP BY T1.A
-+>HAVING T1.A >ANY
-+> ( SELECT 2 FROM T002T1 T2
-+> WHERE T2.C >SOME
-+> ( SELECT AVG (T1.A) FROM T002T1 T3 )
-+> );
-
-(EXPR)
-------
-
- 1
- 1
-
---- 2 row(s) selected.
+>>-- This test disabled since it is non -deterministic.
--- End diff --
Unless you plan to work on this right away it might be good to file a JIRA to keep track of this.
---
[GitHub] trafodion pull request #1454: [TRAFODION-2969] Fix interaction of [first n] ...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1454#discussion_r171012343
--- Diff: core/sql/optimizer/RelExpr.cpp ---
@@ -11388,8 +11388,9 @@ void RelRoot::setMvBindContext(MvBindContext * pMvBindContext)
pMvBindContextForScope_ = pMvBindContext;
}
-void RelRoot::addOneRowAggregates(BindWA* bindWA)
+NABoolean RelRoot::addOneRowAggregates(BindWA* bindWA, NABoolean forceGroupByAgg)
{
+ NABoolean GroupByAggNodeAdded = FALSE;
--- End diff --
A small nit: This variable should start with a lower case letter.
---
[GitHub] trafodion pull request #1454: [TRAFODION-2969] Fix interaction of [first n] ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1454#discussion_r171015233
--- Diff: core/sql/regress/core/EXPECTED002.LINUX ---
@@ -464,22 +478,16 @@ A AMAX B BMAX C C
>>
>>
>>------------------------------------------------------------------------
->>?section Genesis_10_000222_6892_p1
->>SELECT 1 FROM T002T3 T1
-+>GROUP BY T1.A
-+>HAVING T1.A >ANY
-+> ( SELECT 2 FROM T002T1 T2
-+> WHERE T2.C >SOME
-+> ( SELECT AVG (T1.A) FROM T002T1 T3 )
-+> );
-
-(EXPR)
-------
-
- 1
- 1
-
---- 2 row(s) selected.
+>>-- This test disabled since it is non -deterministic.
--- End diff --
Sure, I can do that.
---
[GitHub] trafodion pull request #1454: [TRAFODION-2969] Fix interaction of [first n] ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1454#discussion_r171019092
--- Diff: core/sql/regress/core/EXPECTED002.LINUX ---
@@ -464,22 +478,16 @@ A AMAX B BMAX C C
>>
>>
>>------------------------------------------------------------------------
->>?section Genesis_10_000222_6892_p1
->>SELECT 1 FROM T002T3 T1
-+>GROUP BY T1.A
-+>HAVING T1.A >ANY
-+> ( SELECT 2 FROM T002T1 T2
-+> WHERE T2.C >SOME
-+> ( SELECT AVG (T1.A) FROM T002T1 T3 )
-+> );
-
-(EXPR)
-------
-
- 1
- 1
-
---- 2 row(s) selected.
+>>-- This test disabled since it is non -deterministic.
--- End diff --
JIRA filed. See https://issues.apache.org/jira/browse/TRAFODION-2972
---