You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/05/25 17:41:54 UTC

[GitHub] [incubator-pinot] GSharayu opened a new pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

GSharayu opened a new pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973


   For general select queries
   
   e.g SELECT * FROM Foo ..... LIMIT N
   
   e.g SELECT col1, col2 ..... FROM Foo .... LIMIT N
   
   the columns are identifier expressions and not some transform functions. So, we should have a PassThroughTransformOperator that avoids the if conditional check repeatedly in TransformBlock.
   
   If the columns selected in the query are identifier expressions then no need to make the check again and again for every block fetch of 10K records. Since during query planning time, we have the information available in TransformPlanNode if selected expressions are identifiers or not, we can make an optimization to have a specialized TransformOperator and TransformBlock that simply does a pass through and avoids the repeated if check at query runtime.
   
   This is something that would have been done in pinot if there was run time query specific code generation. Since we currently don't have it, we can handwrite the specialized operator.
   
   Issue #6972 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] siddharthteotia commented on pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#issuecomment-848087338


   @GSharayu , you may want to update the PR description to state if existing query tests cover both scenarios and may be point to the test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] siddharthteotia edited a comment on pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
siddharthteotia edited a comment on pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#issuecomment-849032571


   @Jackie-Jiang 
   
   A query on a 50 million row dataset that was taking 7secs repeatedly was taking 5.5secs on multiple runs. It was a select * reporting style query. 
   
   So a reasonable optimization. Another reason for doing that is to get better at leveraging information already available and extracted at query planning time later during query runtime and avoid branches if possible.
   
   Ideally I would like to replace transform operator from the plan completely with just project operator for non transform queries. If Pinot had run time query specific code generation, it would have been done anyway. But in the absence of that, we should use handwritten specialized operator  meant for the query as much as possible.  We have done some changes on replacing transform operator with project and the changes are quite invasive. Let's see how that goes. 
   
   Dynamic dispatch due to runtime polymorphism can be expensive sometimes but hotspot jvm does a pretty good job of optimizing it once the initial code gets compiled to native code. What additional thing I used to do for inheritance is to cache the super class final variables in a local reference.  But I think that's overkill and not needed or we can do a follow-up.
   
   cc @GSharayu 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#discussion_r640037165



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/PassThroughTransformOperator.java
##########
@@ -0,0 +1,52 @@
+/**
+ * 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.pinot.core.operator.transform;
+
+import java.util.Collection;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.core.operator.ProjectionOperator;
+import org.apache.pinot.core.operator.blocks.PassThroughTransformBlock;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+
+
+/**
+ * Class for evaluating pass through transform expressions.
+ */
+public class PassThroughTransformOperator extends TransformOperator {
+  private static final String OPERATOR_NAME = "PassThroughTransformOperator";

Review comment:
       Sorry we missed this. @GSharayu , can you please address this in a follow-up PR ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter commented on pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#issuecomment-848129143


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6973?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > :exclamation: No coverage uploaded for pull request base (`master@8878df5`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
   > The diff coverage is `74.59%`.
   
   > :exclamation: Current head 8c6af0c differs from pull request most recent head 6d9938e. Consider uploading reports for the commit 6d9938e to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6973/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6973?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #6973   +/-   ##
   =========================================
     Coverage          ?   65.36%           
     Complexity        ?       12           
   =========================================
     Files             ?     1439           
     Lines             ?    71333           
     Branches          ?    10334           
   =========================================
     Hits              ?    46624           
     Misses            ?    21337           
     Partials          ?     3372           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `65.36% <74.59%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6973?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...pinot/broker/pruner/PartitionZKMetadataPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcHJ1bmVyL1BhcnRpdGlvblpLTWV0YWRhdGFQcnVuZXIuamF2YQ==) | `33.33% <ø> (ø)` | |
   | [.../routing/segmentpruner/PartitionSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1BhcnRpdGlvblNlZ21lbnRQcnVuZXIuamF2YQ==) | `61.71% <ø> (ø)` | |
   | [...mon/metadata/segment/SegmentPartitionMetadata.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0YWRhdGEvc2VnbWVudC9TZWdtZW50UGFydGl0aW9uTWV0YWRhdGEuamF2YQ==) | `65.00% <ø> (ø)` | |
   | [...va/org/apache/pinot/common/utils/SegmentUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU2VnbWVudFV0aWxzLmphdmE=) | `68.75% <ø> (ø)` | |
   | [...er/api/resources/LLCSegmentCompletionHandlers.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==) | `0.00% <ø> (ø)` | |
   | [...e/assignment/segment/OfflineSegmentAssignment.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9PZmZsaW5lU2VnbWVudEFzc2lnbm1lbnQuamF2YQ==) | `92.12% <ø> (ø)` | |
   | [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `67.87% <ø> (ø)` | |
   | [.../realtime/segment/CommittingSegmentDescriptor.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvQ29tbWl0dGluZ1NlZ21lbnREZXNjcmlwdG9yLmphdmE=) | `73.33% <ø> (ø)` | |
   | [...ot/controller/helix/core/util/ZKMetadataUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3V0aWwvWktNZXRhZGF0YVV0aWxzLmphdmE=) | `90.62% <ø> (ø)` | |
   | [...mmender/realtime/provisioning/MemoryEstimator.java](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9yZWFsdGltZS9wcm92aXNpb25pbmcvTWVtb3J5RXN0aW1hdG9yLmphdmE=) | `86.00% <ø> (ø)` | |
   | ... and [170 more](https://codecov.io/gh/apache/incubator-pinot/pull/6973/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6973?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6973?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8878df5...6d9938e](https://codecov.io/gh/apache/incubator-pinot/pull/6973?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#discussion_r639430374



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/PassThroughTransformOperator.java
##########
@@ -0,0 +1,52 @@
+/**
+ * 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.pinot.core.operator.transform;
+
+import java.util.Collection;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.core.operator.ProjectionOperator;
+import org.apache.pinot.core.operator.blocks.PassThroughTransformBlock;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+
+
+/**
+ * Class for evaluating pass through transform expressions.
+ */
+public class PassThroughTransformOperator extends TransformOperator {
+  private static final String OPERATOR_NAME = "PassThroughTransformOperator";

Review comment:
       This is unused. You should override the `getOperatorName()`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] siddharthteotia merged pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
siddharthteotia merged pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] GSharayu commented on a change in pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
GSharayu commented on a change in pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#discussion_r639038817



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/plan/TransformPlanNode.java
##########
@@ -33,13 +34,15 @@
 public class TransformPlanNode implements PlanNode {
   private final Collection<ExpressionContext> _expressions;
   private final ProjectionPlanNode _projectionPlanNode;
+  private boolean _areIdentifiers = true;

Review comment:
       updated!
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#discussion_r639032664



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/PassThroughTransformOperator.java
##########
@@ -0,0 +1,51 @@
+/**
+ * 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.pinot.core.operator.transform;
+
+import java.util.Collection;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.core.operator.ProjectionOperator;
+import org.apache.pinot.core.operator.blocks.PassThroughTransformBlock;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+
+
+/**
+ * Class for evaluating pass through transform expressions.
+ */
+public class PassThroughTransformOperator extends TransformOperator {
+  /**

Review comment:
       Just like TransformOperator has operator name, have a private static here
   
   `private static final String OPERATOR_NAME = "PassThroughTransformOperator";`

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/plan/TransformPlanNode.java
##########
@@ -33,13 +34,15 @@
 public class TransformPlanNode implements PlanNode {
   private final Collection<ExpressionContext> _expressions;
   private final ProjectionPlanNode _projectionPlanNode;
+  private boolean _areIdentifiers = true;

Review comment:
       (nit) suggest renaming to `_allExpressionsIdentifiers` or something similar 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] siddharthteotia commented on pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#issuecomment-849032571


   @Jackie-Jiang 
   
   A query on a 50 million row dataset that was taking 7secs repeatedly was taking 5.5secs on multiple runs. It was a select * reporting style query. 
   
   So a reasonable optimization. Another reason for doing that is to get better at leveraging information already available and extracted at query planning time later during query runtime and avoid branches if possible.
   
   Ideally I would like to replace transform operator from the plan completely with just project operator for non transform queries. If Pinot had run time query specific code generation, it would have been done anyway. But in the absence of that, we should use handwritten specialized operator  meant for the query as much as possible.  We have done some changes on replacing transform operator with project and the changes are quite invasive. Let's see how that goes. 
   
   Dynamic dispatch due to runtime polymorphism can be expensive sometimes but hotspot jvm does a pretty good job of optimizing it once the initial code gets compiled to native code. What additional thing I used to do for inheritance is to cache the super class final variables in a local reference to avoid the  But I think that's overkill and not needed or we can do a follow-up.
   
   cc @GSharayu 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] GSharayu commented on a change in pull request #6973: Implement PassThroughTransformOperator to optimize select queries(#6972)

Posted by GitBox <gi...@apache.org>.
GSharayu commented on a change in pull request #6973:
URL: https://github.com/apache/incubator-pinot/pull/6973#discussion_r640091769



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/PassThroughTransformOperator.java
##########
@@ -0,0 +1,52 @@
+/**
+ * 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.pinot.core.operator.transform;
+
+import java.util.Collection;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.core.operator.ProjectionOperator;
+import org.apache.pinot.core.operator.blocks.PassThroughTransformBlock;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+
+
+/**
+ * Class for evaluating pass through transform expressions.
+ */
+public class PassThroughTransformOperator extends TransformOperator {
+  private static final String OPERATOR_NAME = "PassThroughTransformOperator";

Review comment:
       will update!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org