You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by chunhui-shi <gi...@git.apache.org> on 2016/05/26 07:00:49 UTC
[GitHub] drill pull request: DRILL-4618: Fix hive function loader not corre...
GitHub user chunhui-shi opened a pull request:
https://github.com/apache/drill/pull/509
DRILL-4618: Fix hive function loader not correctly take random flag; \u2026
\u2026and function visitor should not use previous function holder if this function is nondeterministic
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/chunhui-shi/drill DRILL-4618-random
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/509.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 #509
----
commit 81fe40f9890f1aa7a8a5893bff41b9d8dc0f9399
Author: chunhui-shi <cs...@maprtech.com>
Date: 2016-05-26T06:22:01Z
DRILL-4618: Fix hive function loader not correctly take random flag; and function visitor should not use previous function holder if this function is nondeterministic
----
---
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] drill issue #509: DRILL-4618: Fix hive function loader not correctly take ra...
Posted by Ben-Zvi <gi...@git.apache.org>.
Github user Ben-Zvi commented on the issue:
https://github.com/apache/drill/pull/509
Tested and checked ... LGTM
---
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] drill pull request #509: DRILL-4618: Fix hive function loader not correctly ...
Posted by sudheeshkatkam <gi...@git.apache.org>.
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/509#discussion_r82657875
--- Diff: contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java ---
@@ -84,7 +84,7 @@ public void testGetJsonObject() throws Exception {
.go();
}
- @Test // DRILL-3272
+ @Test //DRILL-4618
--- End diff --
How is this test related to 4618?
---
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] drill pull request #509: DRILL-4618: Fix hive function loader not correctly ...
Posted by sudheeshkatkam <gi...@git.apache.org>.
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/509#discussion_r82645626
--- Diff: contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java ---
@@ -84,7 +84,6 @@ public void testGetJsonObject() throws Exception {
.go();
}
- @Test // DRILL-3272
public void testIf() throws Exception {
--- End diff --
Why is this test being ignored? Is this test not relevant anymore?
---
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] drill issue #509: DRILL-4618: Fix hive function loader not correctly take ra...
Posted by sudheeshkatkam <gi...@git.apache.org>.
Github user sudheeshkatkam commented on the issue:
https://github.com/apache/drill/pull/509
+1
---
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] drill issue #509: DRILL-4618: Fix hive function loader not correctly take ra...
Posted by chunhui-shi <gi...@git.apache.org>.
Github user chunhui-shi commented on the issue:
https://github.com/apache/drill/pull/509
@StevenMPhillips Hi Steven, is this fix addressing your concern now? Thanks.
---
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] drill pull request #509: DRILL-4618: Fix hive function loader not correctly ...
Posted by chunhui-shi <gi...@git.apache.org>.
Github user chunhui-shi commented on a diff in the pull request:
https://github.com/apache/drill/pull/509#discussion_r82657521
--- Diff: contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java ---
@@ -84,7 +84,6 @@ public void testGetJsonObject() throws Exception {
.go();
}
- @Test // DRILL-3272
public void testIf() throws Exception {
--- End diff --
It was a merge error. Fixed.
---
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] drill pull request #509: DRILL-4618: Fix hive function loader not correctly ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/509
---
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] drill pull request: DRILL-4618: Fix hive function loader not corre...
Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:
https://github.com/apache/drill/pull/509#discussion_r64940411
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java ---
@@ -811,7 +811,7 @@ public HoldingContainer visitFunctionCall(FunctionCall call, ClassGenerator<?> g
@Override
public HoldingContainer visitFunctionHolderExpression(FunctionHolderExpression holder, ClassGenerator<?> generator) throws RuntimeException {
HoldingContainer hc = getPrevious(holder, generator.getMappingSet());
- if (hc == null) {
+ if (hc == null || holder.isRandom()) {
--- End diff --
I think this change is not sufficient to cover all cases. If random() is just part of the expression, then this won't help. For example, the expression 2 * random().
I think a better way to fix this would be to modify EqualityVisitor.visitFunctionHolderExpression() to return false if the function is random. That way any expression which contains a random function will never be considered equal to another expression.
---
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] drill pull request #509: DRILL-4618: Fix hive function loader not correctly ...
Posted by sudheeshkatkam <gi...@git.apache.org>.
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/509#discussion_r82645633
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/EqualityVisitor.java ---
@@ -75,6 +75,9 @@ public Boolean visitFunctionHolderExpression(FunctionHolderExpression holder, Lo
if (!holder.getName().equals(((FunctionHolderExpression) value).getName())) {
return false;
}
+ if(holder.isRandom()) {
--- End diff --
spacing: `if (`
---
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.
---