[jira] [Commented] (DRILL-5538) Create TopProject with validatedNodeType after PHYSICAL phase

2017-06-27 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/DRILL-5538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16064840#comment-16064840
 ] 

ASF GitHub Bot commented on DRILL-5538:
---

Github user asfgit closed the pull request at:

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


>  Create TopProject with validatedNodeType after PHYSICAL phase
> --
>
> Key: DRILL-5538
> URL: https://issues.apache.org/jira/browse/DRILL-5538
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Query Planning & Optimization
>Affects Versions: 1.10.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>  Labels: ready-to-commit
> Fix For: 1.11.0
>
>
> When [RDBMS storage 
> plugin|https://drill.apache.org/docs/rdbms-storage-plugin/]  is enabled, 
> during query execution certain JDBC rules are added.
> One of the rules is 
> [ProjectRemoveRule|https://github.com/apache/drill/blob/master/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java#L140].
>  Drill also uses this rule but during phases when it considers it useful, for 
> example, during LOGICAL and JOIN_PLANNING. On the contrary, storage plugin 
> rules are added to any phase of query planning. Thus it results to project 
> stage to be removed when actually it is needed.
> Sometimes when ProjectRemoveRule decides that project is trivial and removes 
> it, during this stage Drill added column alias or removed implicit columns.
> For example, with RDBMS plugin enabled, alias is not displayed for simple 
> query:
> {noformat}
> 0: jdbc:drill:zk=local> create temporary table t as select * from sys.version;
> SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
> SLF4J: Defaulting to no-operation (NOP) logger implementation
> SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further 
> details.
> +---++
> | Fragment  | Number of records written  |
> +---++
> | 0_0   | 1  |
> +---++
> 1 row selected (0.623 seconds)
> 0: jdbc:drill:zk=local> select version as current_version from t;
> +--+
> | version  |
> +--+
> | 1.11.0-SNAPSHOT  |
> +--+
> 1 row selected (0.28 seconds)
> {noformat}
> Proposed fix is to move the creating of TopProject with validatedNodeType 
> after physical planning is done.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (DRILL-5538) Create TopProject with validatedNodeType after PHYSICAL phase

2017-06-23 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/DRILL-5538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16060753#comment-16060753
 ] 

ASF GitHub Bot commented on DRILL-5538:
---

Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/844#discussion_r123726213
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/TopProjectVisitor.java
 ---
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner.physical.visitor;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.drill.exec.planner.common.DrillRelOptUtil;
+import org.apache.drill.exec.planner.physical.Prel;
+import org.apache.drill.exec.planner.physical.ProjectPrel;
+import org.apache.drill.exec.planner.physical.ScreenPrel;
+import org.apache.drill.exec.planner.physical.WriterPrel;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Adds non-trivial top project to ensure the final output field names are 
preserved.
+ * Is added under the screen or writer operators.
--- End diff --

Added comments and put ready-to-commit label.


>  Create TopProject with validatedNodeType after PHYSICAL phase
> --
>
> Key: DRILL-5538
> URL: https://issues.apache.org/jira/browse/DRILL-5538
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Query Planning & Optimization
>Affects Versions: 1.10.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>  Labels: ready-to-commit
> Fix For: 1.11.0
>
>
> When [RDBMS storage 
> plugin|https://drill.apache.org/docs/rdbms-storage-plugin/]  is enabled, 
> during query execution certain JDBC rules are added.
> One of the rules is 
> [ProjectRemoveRule|https://github.com/apache/drill/blob/master/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java#L140].
>  Drill also uses this rule but during phases when it considers it useful, for 
> example, during LOGICAL and JOIN_PLANNING. On the contrary, storage plugin 
> rules are added to any phase of query planning. Thus it results to project 
> stage to be removed when actually it is needed.
> Sometimes when ProjectRemoveRule decides that project is trivial and removes 
> it, during this stage Drill added column alias or removed implicit columns.
> For example, with RDBMS plugin enabled, alias is not displayed for simple 
> query:
> {noformat}
> 0: jdbc:drill:zk=local> create temporary table t as select * from sys.version;
> SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
> SLF4J: Defaulting to no-operation (NOP) logger implementation
> SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further 
> details.
> +---++
> | Fragment  | Number of records written  |
> +---++
> | 0_0   | 1  |
> +---++
> 1 row selected (0.623 seconds)
> 0: jdbc:drill:zk=local> select version as current_version from t;
> +--+
> | version  |
> +--+
> | 1.11.0-SNAPSHOT  |
> +--+
> 1 row selected (0.28 seconds)
> {noformat}
> Proposed fix is to move the creating of TopProject with validatedNodeType 
> after physical planning is done.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (DRILL-5538) Create TopProject with validatedNodeType after PHYSICAL phase

2017-06-22 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/DRILL-5538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16060081#comment-16060081
 ] 

ASF GitHub Bot commented on DRILL-5538:
---

Github user jinfengni commented on a diff in the pull request:

https://github.com/apache/drill/pull/844#discussion_r123631660
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/TopProjectVisitor.java
 ---
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner.physical.visitor;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.drill.exec.planner.common.DrillRelOptUtil;
+import org.apache.drill.exec.planner.physical.Prel;
+import org.apache.drill.exec.planner.physical.ProjectPrel;
+import org.apache.drill.exec.planner.physical.ScreenPrel;
+import org.apache.drill.exec.planner.physical.WriterPrel;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Adds non-trivial top project to ensure the final output field names are 
preserved.
+ * Is added under the screen or writer operators.
--- End diff --

You may add comments why we need such non-trivial project, due to Calcite's 
behavior of ProjectRemoveRule. Also, it might be worth mentioning that the 
added Project may not be sitting right under Screen/Writer operator in the 
physical plan, as we would add additional Projects, for case likes * column 
expansion, or partition by column processing.


>  Create TopProject with validatedNodeType after PHYSICAL phase
> --
>
> Key: DRILL-5538
> URL: https://issues.apache.org/jira/browse/DRILL-5538
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Query Planning & Optimization
>Affects Versions: 1.10.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>
> When [RDBMS storage 
> plugin|https://drill.apache.org/docs/rdbms-storage-plugin/]  is enabled, 
> during query execution certain JDBC rules are added.
> One of the rules is 
> [ProjectRemoveRule|https://github.com/apache/drill/blob/master/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java#L140].
>  Drill also uses this rule but during phases when it considers it useful, for 
> example, during LOGICAL and JOIN_PLANNING. On the contrary, storage plugin 
> rules are added to any phase of query planning. Thus it results to project 
> stage to be removed when actually it is needed.
> Sometimes when ProjectRemoveRule decides that project is trivial and removes 
> it, during this stage Drill added column alias or removed implicit columns.
> For example, with RDBMS plugin enabled, alias is not displayed for simple 
> query:
> {noformat}
> 0: jdbc:drill:zk=local> create temporary table t as select * from sys.version;
> SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
> SLF4J: Defaulting to no-operation (NOP) logger implementation
> SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further 
> details.
> +---++
> | Fragment  | Number of records written  |
> +---++
> | 0_0   | 1  |
> +---++
> 1 row selected (0.623 seconds)
> 0: jdbc:drill:zk=local> select version as current_version from t;
> +--+
> | version  |
> +--+
> | 1.11.0-SNAPSHOT  |
> +--+
> 1 row selected (0.28 seconds)
> {noformat}
> Proposed fix is to move the creating of TopProject with validatedNodeType 
> after

[jira] [Commented] (DRILL-5538) Create TopProject with validatedNodeType after PHYSICAL phase

2017-06-22 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/DRILL-5538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16060080#comment-16060080
 ] 

ASF GitHub Bot commented on DRILL-5538:
---

Github user jinfengni commented on a diff in the pull request:

https://github.com/apache/drill/pull/844#discussion_r123632779
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
 ---
@@ -459,7 +468,13 @@ protected Prel convertToPrel(RelNode drel) throws 
RelConversionException, SqlUns
 /* The order of the following transformations is important */
 
 /*
- * 0.) For select * from join query, we need insert project on top of 
scan and a top project just
+ * 0.)
+ * Add top project before screen operator or writer to ensure that 
final output column names are preserved.
+ */
+phyRelNode = TopProjectVisitor.insertTopProject(phyRelNode, 
validatedRowType);
--- End diff --

Make sense to me that you put this Visitor handling first, after physical 
transform() call, before these existing visitor handling.  


>  Create TopProject with validatedNodeType after PHYSICAL phase
> --
>
> Key: DRILL-5538
> URL: https://issues.apache.org/jira/browse/DRILL-5538
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Query Planning & Optimization
>Affects Versions: 1.10.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>
> When [RDBMS storage 
> plugin|https://drill.apache.org/docs/rdbms-storage-plugin/]  is enabled, 
> during query execution certain JDBC rules are added.
> One of the rules is 
> [ProjectRemoveRule|https://github.com/apache/drill/blob/master/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java#L140].
>  Drill also uses this rule but during phases when it considers it useful, for 
> example, during LOGICAL and JOIN_PLANNING. On the contrary, storage plugin 
> rules are added to any phase of query planning. Thus it results to project 
> stage to be removed when actually it is needed.
> Sometimes when ProjectRemoveRule decides that project is trivial and removes 
> it, during this stage Drill added column alias or removed implicit columns.
> For example, with RDBMS plugin enabled, alias is not displayed for simple 
> query:
> {noformat}
> 0: jdbc:drill:zk=local> create temporary table t as select * from sys.version;
> SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
> SLF4J: Defaulting to no-operation (NOP) logger implementation
> SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further 
> details.
> +---++
> | Fragment  | Number of records written  |
> +---++
> | 0_0   | 1  |
> +---++
> 1 row selected (0.623 seconds)
> 0: jdbc:drill:zk=local> select version as current_version from t;
> +--+
> | version  |
> +--+
> | 1.11.0-SNAPSHOT  |
> +--+
> 1 row selected (0.28 seconds)
> {noformat}
> Proposed fix is to move the creating of TopProject with validatedNodeType 
> after physical planning is done.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (DRILL-5538) Create TopProject with validatedNodeType after PHYSICAL phase

2017-06-15 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/DRILL-5538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16050454#comment-16050454
 ] 

ASF GitHub Bot commented on DRILL-5538:
---

Github user arina-ielchiieva commented on the issue:

https://github.com/apache/drill/pull/844
  
@jinfengni updated to PR to create TopProject with validatedNodeType after 
PHYSICAL phase instead of removing ProjectRemoveRule completely.


>  Create TopProject with validatedNodeType after PHYSICAL phase
> --
>
> Key: DRILL-5538
> URL: https://issues.apache.org/jira/browse/DRILL-5538
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Query Planning & Optimization
>Affects Versions: 1.10.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>
> When [RDBMS storage 
> plugin|https://drill.apache.org/docs/rdbms-storage-plugin/]  is enabled, 
> during query execution certain JDBC rules are added.
> One of the rules is 
> [ProjectRemoveRule|https://github.com/apache/drill/blob/master/contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java#L140].
>  Drill also uses this rule but during phases when it considers it useful, for 
> example, during LOGICAL and JOIN_PLANNING. On the contrary, storage plugin 
> rules are added to any phase of query planning. Thus it results to project 
> stage to be removed when actually it is needed.
> Sometimes when ProjectRemoveRule decides that project is trivial and removes 
> it, during this stage Drill added column alias or removed implicit columns.
> For example, with RDBMS plugin enabled, alias is not displayed for simple 
> query:
> {noformat}
> 0: jdbc:drill:zk=local> create temporary table t as select * from sys.version;
> SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
> SLF4J: Defaulting to no-operation (NOP) logger implementation
> SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further 
> details.
> +---++
> | Fragment  | Number of records written  |
> +---++
> | 0_0   | 1  |
> +---++
> 1 row selected (0.623 seconds)
> 0: jdbc:drill:zk=local> select version as current_version from t;
> +--+
> | version  |
> +--+
> | 1.11.0-SNAPSHOT  |
> +--+
> 1 row selected (0.28 seconds)
> {noformat}
> Proposed fix is to move the creating of TopProject with validatedNodeType 
> after physical planning is done.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)