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 Wed, 11 Mar 2015 00:21:13 GMT

> On March 10, 2015, 4:52 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java,
line 43
> > <https://reviews.apache.org/r/31871/diff/1/?file=889812#file889812line43>
> >
> >     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.
> Jason Altekruse wrote:
>     I will start this work, but I was trying to avoid confusion within Drill as much
as possible, as well as major refactoring. If we want to change this pattern everywhere, that
would include where we define all of the non-deterministic UDFs (although admittedly there
aren't too many of them). I had chosen this point as it was exactly where we meet with the
Calcite API.
> Jason Altekruse wrote:
>     I'm nervous about this being inconsistent within Drill. I believe that if we are
going to try to be consistent with Calcite, we should change how we refer to this property
everywhere. I have a patch that does the renaming, UDF updates and change the interface to
Hive UDFS that I think is necessary to make this complete change. I would advocate for splitting
this into a separate JIRA if we want to make the change.
>     I have updated all current references to this modified constructor in the patch already
posted, so there is no need for a default constructor that does not provide determinism. While
indeterministic functions are rare, I think it would be best to leave this required so any
new function types defined will be explicitly created to fill this field appropriately.

This has not been tested, but this is what I believe the changeset for tracking determinism
would require. https://github.com/jaltekruse/incubator-drill/commit/40d475324255df5b3313ca877b678c3796051fb8

- Jason

This is an automatically generated e-mail. To reply, visit:

On March 10, 2015, 11:21 p.m., Jason Altekruse wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31871/
> -----------------------------------------------------------
> (Updated March 10, 2015, 11:21 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
> Diffs
> -----
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 30b905f

>   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 e413921

>   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/fragment/SimpleParallelizer.java
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 409450f

>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestLocalExchange.java
>   exec/java-exec/src/test/java/org/apache/drill/exec/pop/TestFragmentChecker.java 07d310a

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

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