You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by jinfengni <gi...@git.apache.org> on 2015/10/19 19:24:47 UTC

[GitHub] drill pull request: DRILL-3802: Throw unsupported error for Rollup...

GitHub user jinfengni opened a pull request:

    https://github.com/apache/drill/pull/209

    DRILL-3802: Throw unsupported error for Rollup, Cube, Grouping_Sets, …

    …Grouping, Grouping_ID, Group_ID.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jinfengni/incubator-drill DRILL-3802

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/209.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 #209
    
----
commit 3eeaf9936713d28c3a21c4b8b0787142bd35a44a
Author: Jinfeng Ni <jn...@apache.org>
Date:   2015-10-19T17:19:03Z

    DRILL-3802: Throw unsupported error for Rollup, Cube, Grouping_Sets, Grouping, Grouping_ID, Group_ID.

----


---
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-3802: Throw unsupported error for Rollup...

Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the pull request:

    https://github.com/apache/drill/pull/209#issuecomment-149369397
  
    Gave one minor comment; rest of the changes look good to me.  +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 pull request: DRILL-3802: Throw unsupported error for Rollup...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/drill/pull/209


---
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-3802: Throw unsupported error for Rollup...

Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:

    https://github.com/apache/drill/pull/209#discussion_r42435960
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java ---
    @@ -305,6 +313,105 @@ public SqlNode visit(SqlCall sqlCall) {
         return sqlCall.getOperator().acceptCall(this, sqlCall);
       }
     
    +  private void checkRollupCubeGrpSets(SqlSelect sqlSelect) {
    +    final ExprFinder rollupCubeGrpSetsFinder = new ExprFinder(RollupCubeGrpSets);
    +    sqlSelect.accept(rollupCubeGrpSetsFinder);
    +    if (rollupCubeGrpSetsFinder.find()) {
    +      unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
    --- End diff --
    
    I know that other Unsupported errors have pointed to JIRAs that are marked 'resolved fixed' because they were about throwing such an exception.  Ideally, though, we should create an enhancement JIRA (such as 'Add support for ROLLUP') and point to that in the error message.  


---
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-3802: Throw unsupported error for Rollup...

Posted by hsuanyi <gi...@git.apache.org>.
Github user hsuanyi commented on the pull request:

    https://github.com/apache/drill/pull/209#issuecomment-149294066
  
    +1(non-binding) 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: DRILL-3802: Throw unsupported error for Rollup...

Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on a diff in the pull request:

    https://github.com/apache/drill/pull/209#discussion_r42679515
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java ---
    @@ -305,6 +313,105 @@ public SqlNode visit(SqlCall sqlCall) {
         return sqlCall.getOperator().acceptCall(this, sqlCall);
       }
     
    +  private void checkRollupCubeGrpSets(SqlSelect sqlSelect) {
    +    final ExprFinder rollupCubeGrpSetsFinder = new ExprFinder(RollupCubeGrpSets);
    +    sqlSelect.accept(rollupCubeGrpSetsFinder);
    +    if (rollupCubeGrpSetsFinder.find()) {
    +      unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.FUNCTION,
    --- End diff --
    
    Good point. I'll create an enhancement JIRA and point to the enhancement JIRA in the error msg.



---
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.
---