drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacques Nadeau" <jacques.dr...@gmail.com>
Subject Re: Review Request 31871: 2406 - part 2 - (subset of 2060 part 2) enable interpreted expression evaluation at planning time
Date Tue, 10 Mar 2015 04:52:26 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31871/#review75852
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/ops/BufferManager.java
<https://reviews.apache.org/r/31871/#comment123171>

    Give short description of purpose



exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
<https://reviews.apache.org/r/31871/#comment123172>

    Disagree with TODO.  Please remove.  Our goal is to not expose outer context to inner
contexts.



exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
<https://reviews.apache.org/r/31871/#comment123173>

    This comment seems like it could easily become wrong.  I would remove.  People can simply
find search code for references to getAllocator which would be a better indiciator of why
this exists.



exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
<https://reviews.apache.org/r/31871/#comment123174>

    This should be fixed.  Otherwise, you're simply introducing a bug the moment that we reduce
an expression containing current date in a constant expression.



exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
<https://reviews.apache.org/r/31871/#comment123175>

    Can you update this interface to be isDeterministic?  Update func holder as well.  Simply
keep the old interface of isRandom() and mark as deprecated.  Additionally, please provide
a default constructor that is isDeterministic = true so that we don't have to change all the
places were we assumed that behavior.



exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
<https://reviews.apache.org/r/31871/#comment123176>

    Can you update this interface to be isDeterministic?  Update func holder as well.  Simply
keep the old interface of isRandom() and mark as deprecated.  Additionally, please provide
a default constructor that is isDeterministic = true so that we don't have to change all the
places were we assumed that behavior.


- Jacques Nadeau


On March 9, 2015, 10:07 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31871/
> -----------------------------------------------------------
> 
> (Updated March 9, 2015, 10:07 p.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Bugs: DRILL-2406
>     https://issues.apache.org/jira/browse/DRILL-2406
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> To enable planning rules to take advantage of constant expressions in queries, or to
allow them to run queries against small datasets, such as partition information we need to
e able to evaluate expressions at planning time. Expression evaluation in the regular expecuation
path is relatively expensive, as it invovles java code generation, compilation and JITing
expression trees. An interpreted expression system was added recently to allow evaluating
an expression without generating java code at runtime. This patch exposes that work (after
some refactorings in 2143 and the first part of 2406) to the planning context for use in optimizer
rules.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
00aaec6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java 2a28bcb

>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/BufferManager.java PRE-CREATION

>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java aa1dffd

>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java c881432 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java
93fff35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java
907fcb1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
6b54c43 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 409450f

> 
> Diff: https://reviews.apache.org/r/31871/diff/
> 
> 
> Testing
> -------
> 
> Cluster tests completed, waiting on unit tests.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message