[jira] [Commented] (DRILL-5538) Create TopProject with validatedNodeType after PHYSICAL phase
[ 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
[ 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
[ 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
[ 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
[ 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)