> 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.
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. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31871/#review75852 ----------------------------------------------------------- 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 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/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 > f8d1803 > > 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 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java > 4ab3cc0 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestLocalExchange.java > fc1c6b9 > > 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 > >
