drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Altekruse" <altekruseja...@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 20:49:20 GMT


> On March 10, 2015, 8:01 p.m., Chris Westin wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java, line
165
> > <https://reviews.apache.org/r/31871/diff/1/?file=889813#file889813line165>
> >
> >     After my patch, QueryContext no longer needs to be closed -- so there's no possibility
of leaking an exception here, because there can't be one. So I wouldn't worry about this for
now -- you can leave it as-is if it's working for you, but beware of merging if this goes
in after my patch.

After a discussion with Chris I will be making the following changes.

I will make the QueryContext class properly return Exception, not IOException. I had written
this originally to implement the Closable interface (which throws IOException) and ended up
changing it to AutoClosable during the time I had rebased this on top of Chris' work for testing.
This did not cause an issue with compilation or runtime, as IOExcption inherits from Exception,
but it did unnecesarily create confusion as I had changed the signature from AutoCloseable.

I will also add a return statement after the call to move to the FAILED state, this will prevent
sending the message that was provided to the call to cleanup originally.

As the moveToState method calls cleanup to send the failure message to the client, I will
add a flag to QueryContext to make the close method idempotent as encouraged by the AutoClosable
documentation. http://docs.oracle.com/javase/7/docs/api/java/lang/AutoCloseable.html


- Jason


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


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