[GitHub] drill pull request #1198: DRILL-6294: Changes to support Calcite 1.16.0
GitHub user vvysotskyi opened a pull request: https://github.com/apache/drill/pull/1198 DRILL-6294: Changes to support Calcite 1.16.0 You can merge this pull request into a Git repository by running: $ git pull https://github.com/vvysotskyi/drill DRILL-6294 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1198.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 #1198 commit a79d6586033b618c95462368520237aab84f47bf Author: Volodymyr Vysotskyi Date: 2018-02-07T14:24:50Z DRILL-6294: Changes to support Calcite 1.16.0 commit 48880eb80d60a15e4ffdfcd6c729bfc75cf5d2da Author: Volodymyr Vysotskyi Date: 2018-03-11T10:11:45Z DRILL-6294: Remove deprecated API usage ---
[GitHub] drill pull request #1198: DRILL-6294: Changes to support Calcite 1.16.0
Github user chunhui-shi commented on a diff in the pull request: https://github.com/apache/drill/pull/1198#discussion_r178364263 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java --- @@ -218,7 +218,8 @@ private void reduceAggs( RelOptUtil.createProject( newAggRel, projList, -oldAggRel.getRowType().getFieldNames()); +oldAggRel.getRowType().getFieldNames(), +DrillRelFactories.LOGICAL_BUILDER); --- End diff -- Could you explain why we are using DrillRelFactories.LOGICAL_BUILDER but not relBuilderFactory that was used in line 211? And could you point me to this 4 param createProject method with Factory as the last param? ---
[GitHub] drill pull request #1198: DRILL-6294: Changes to support Calcite 1.16.0
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/1198#discussion_r178371341 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java --- @@ -218,7 +218,8 @@ private void reduceAggs( RelOptUtil.createProject( newAggRel, projList, -oldAggRel.getRowType().getFieldNames()); +oldAggRel.getRowType().getFieldNames(), +DrillRelFactories.LOGICAL_BUILDER); --- End diff -- In the second commit it was fixed and used relBuilderFactory to create builder and project. ---
[GitHub] drill pull request #1198: DRILL-6294: Changes to support Calcite 1.16.0
Github user chunhui-shi commented on a diff in the pull request: https://github.com/apache/drill/pull/1198#discussion_r178395314 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java --- @@ -1142,16 +1142,8 @@ public void moveToCurrentRow() throws SQLException { } @Override - public AvaticaStatement getStatement() { -try { - throwIfClosed(); -} catch (AlreadyClosedSqlException e) { - // Can't throw any SQLException because AvaticaConnection's - // getStatement() is missing "throws SQLException". - throw new RuntimeException(e.getMessage(), e); -} catch (SQLException e) { - throw new RuntimeException(e.getMessage(), e); -} + public AvaticaStatement getStatement() throws SQLException { +throwIfClosed(); --- End diff -- Since you are touching this file. You might want to remove not needed Exceptions for throwIfClosed() method that are derives of SqlException. ---
[GitHub] drill pull request #1198: DRILL-6294: Changes to support Calcite 1.16.0
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/1198#discussion_r178404654 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java --- @@ -1142,16 +1142,8 @@ public void moveToCurrentRow() throws SQLException { } @Override - public AvaticaStatement getStatement() { -try { - throwIfClosed(); -} catch (AlreadyClosedSqlException e) { - // Can't throw any SQLException because AvaticaConnection's - // getStatement() is missing "throws SQLException". - throw new RuntimeException(e.getMessage(), e); -} catch (SQLException e) { - throw new RuntimeException(e.getMessage(), e); -} + public AvaticaStatement getStatement() throws SQLException { +throwIfClosed(); --- End diff -- Thanks, it looks clearer now. ---
[GitHub] drill pull request #1198: DRILL-6294: Changes to support Calcite 1.16.0
Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/1198 ---