You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by zellerh <gi...@git.apache.org> on 2016/03/01 00:17:49 UTC
[GitHub] incubator-trafodion pull request: [TRAFODION-1850] First N operato...
GitHub user zellerh opened a pull request:
https://github.com/apache/incubator-trafodion/pull/351
[TRAFODION-1850] First N operator no longer returns extra rows.
This also fixes [TRAFODION-908].
This is the entire or a partial fix to the problem where
a select [first n] * from udf(...) can return more than n rows.
The problem was that neither FirstN nor the UDF operator fully
implemented the GET_N protocol. While that may be ok for the UDF,
the FirstN needed a fix.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zellerh/incubator-trafodion bug/1581
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/351.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 #351
----
commit f614456a56d665587b706da2fb6bb82b5dcc0000
Author: Hans Zeller <hz...@apache.org>
Date: 2016-02-29T23:12:58Z
[TRAFODION-1850] First N operator no longer returns extra rows.
This is the entire or a partial fix to the problem where
a select [first n] * from udf(...) can return more than n rows.
The problem was that neither FirstN nor the UDF operator fully
implemented the GET_N protocol. While that may be ok for the UDF,
the FirstN needed a fix.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1850] First N operato...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/351#discussion_r54513625
--- Diff: core/sql/executor/ExFirstN.cpp ---
@@ -228,11 +232,11 @@ short ExFirstNTcb::work()
// GET_N request value and firstNRows_ to my child.
if ((pentry_down->downState.request != ex_queue::GET_N) ||
(pentry_down->downState.requestValue == firstnTdb().firstNRows()))
- centry->downState.requestValue = firstNVal;
+ centry->downState.requestValue = effectiveFirstN_;
--- End diff --
Actually, that made me realize that on new line 234 the code also needs to be changed to effectiveFirstN_.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1850] First N operato...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/351
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1850] First N operato...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/351#discussion_r54507978
--- Diff: core/sql/executor/ExFirstN.cpp ---
@@ -228,11 +232,11 @@ short ExFirstNTcb::work()
// GET_N request value and firstNRows_ to my child.
if ((pentry_down->downState.request != ex_queue::GET_N) ||
(pentry_down->downState.requestValue == firstnTdb().firstNRows()))
- centry->downState.requestValue = firstNVal;
+ centry->downState.requestValue = effectiveFirstN_;
--- End diff --
In the comment above this (line 230 to be precise) should we change "firstNRows_" to "effectiveFirstN_"?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1850] First N operato...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/351#discussion_r54513552
--- Diff: core/sql/executor/ExFirstN.cpp ---
@@ -228,11 +232,11 @@ short ExFirstNTcb::work()
// GET_N request value and firstNRows_ to my child.
if ((pentry_down->downState.request != ex_queue::GET_N) ||
(pentry_down->downState.requestValue == firstnTdb().firstNRows()))
- centry->downState.requestValue = firstNVal;
+ centry->downState.requestValue = effectiveFirstN_;
--- End diff --
Thanks, I'll fix that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---