You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by vdiravka <gi...@git.apache.org> on 2017/03/22 16:57:04 UTC

[GitHub] drill pull request #792: DRILL-4971: query encounters system error: Statemen...

GitHub user vdiravka opened a pull request:

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

    DRILL-4971: query encounters system error: Statement "break AndOP3" i\u2026

    \u2026s not enclosed by a breakable statement with label "AndOP3"
    
    - New evaluated blocks for boolean operators should be with braces always, since they use labels.

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

    $ git pull https://github.com/vdiravka/drill DRILL-4971

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

    https://github.com/apache/drill/pull/792.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 #792
    
----
commit b8950f88ee0ffda6ae2f6dd31a29dedda08e0189
Author: Vitalii Diravka <vi...@gmail.com>
Date:   2017-03-17T11:41:46Z

    DRILL-4971: query encounters system error: Statement "break AndOP3" is not enclosed by a breakable statement with label "AndOP3"
    - New evaluated blocks for boolean operators should be with braces always, since they use labels.

----


---
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 #792: DRILL-4971: Query encounters system error, when the...

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

    https://github.com/apache/drill/pull/792#discussion_r107901014
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java ---
    @@ -671,8 +674,9 @@ private HoldingContainer visitBooleanAnd(BooleanOperator op,
           HoldingContainer out = generator.declare(op.getMajorType());
     
           JLabel label = generator.getEvalBlockLabel("AndOP");
    -      JBlock eval = generator.getEvalBlock().block();  // enter into nested block
    -      generator.nestEvalBlock(eval);
    +      JBlock eval = new JBlock();
    --- End diff --
    
    It makes sense. 
    Added. 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 #792: DRILL-4971: Query encounters system error, when the...

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

    https://github.com/apache/drill/pull/792#discussion_r107922769
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestEvaluationVisitor.java ---
    @@ -72,6 +73,37 @@ public void x() throws Exception{
         logger.debug(g.generateAndGet());
       }
     
    +  @Test
    +  public void testVisitBooleanOrWithoutFunctionsEvaluation() throws Exception {
    --- End diff --
    
    How about move the new testcase into TestBugFixes? The new testcases uses SQL, while the existing one in TestEvaluationVisitor actually tests in a lower level.  Probably better to not mix them together. 


---
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 #792: DRILL-4971: Query encounters system error, when the...

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

    https://github.com/apache/drill/pull/792#discussion_r107943909
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestEvaluationVisitor.java ---
    @@ -72,6 +73,37 @@ public void x() throws Exception{
         logger.debug(g.generateAndGet());
       }
     
    +  @Test
    +  public void testVisitBooleanOrWithoutFunctionsEvaluation() throws Exception {
    --- End diff --
    
    I agree that new test cases use higher level, SQL. The first thought was TestExampleQueries class, but this class is overloaded.
    TestBugFixes will be better. 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 #792: DRILL-4971: Query encounters system error, when the...

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

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


---
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 #792: DRILL-4971: query encounters system error: Statemen...

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

    https://github.com/apache/drill/pull/792#discussion_r107808909
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java ---
    @@ -671,8 +674,9 @@ private HoldingContainer visitBooleanAnd(BooleanOperator op,
           HoldingContainer out = generator.declare(op.getMajorType());
     
           JLabel label = generator.getEvalBlockLabel("AndOP");
    -      JBlock eval = generator.getEvalBlock().block();  // enter into nested block
    -      generator.nestEvalBlock(eval);
    +      JBlock eval = new JBlock();
    --- End diff --
    
    How about we add the following two methods in ClassGenerator.java: 
    
    ```java
      private JBlock createInnerBlock(BlockType type) {
        final JBlock currBlock = getBlock(type);
        final JBlock innerBlock = new JBlock();
        currBlock.add(innerBlock);
        return innerBlock;
      }
    
      protected JBlock createInnerEvalBlock() {
        return createInnerBlock(BlockType.EVAL);
      }
    ```
    
    Then, replace `generator.getEvalBlock().block()` with `generator.createInnerEvalBlock()`  ?



---
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 #792: DRILL-4971: Query encounters system error, when there aren...

Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on the issue:

    https://github.com/apache/drill/pull/792
  
    +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.
---