----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30697/#review77612 -----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java <https://reviews.apache.org/r/30697/#comment125749> I saw VARBINARY, DECIMAL9 etc in the switch statement in the reduce() method. Why are those types listed as NON_REDUCIBLE_TYPES? exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java <https://reviews.apache.org/r/30697/#comment125759> Is it better to put this number into a named static const var, and add comment to explain why you choose this magic number? exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java <https://reviews.apache.org/r/30697/#comment125748> Is it better to pass in isNullable, in stead of passing RexNode node? Seems the only information obtained from node is the nullability infor. exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java <https://reviews.apache.org/r/30697/#comment125757> Will it better to put the mapping of Drill's minorType and Calcite's SqlTypeName into a map, so that other codes may also leverage the map, in stead of switch statement? exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java <https://reviews.apache.org/r/30697/#comment125752> Is better to throw exception or other way to inform there is error, in stead of returing the original expression, for those currently unsupported types? If the type is not supported, that seems to indicate there is a bug in the caller if it tries to call this method in the first place. - Jinfeng Ni On March 24, 2015, 9:53 a.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30697/ > ----------------------------------------------------------- > > (Updated March 24, 2015, 9:53 a.m.) > > > Review request for drill, Aditya Kishore, Aman Sinha, Jacques Nadeau, Jinfeng > Ni, Mehant Baid, and Parth Chandra. > > > Bugs: DRILL-2060 > https://issues.apache.org/jira/browse/DRILL-2060 > > > Repository: drill-git > > > Description > ------- > > THIS REQUEST HAS BEEN BROKEN INTO TWO REVIEWS: > - I had to make a fairly substandial change to the interpreteed expression > evaluation to make the tests work. > > Use a small modification of a rule in optiq and hook up the interpreted > expression evaluator to fold constant expressions (including those with Drill > UDFs) into literals. Fragment memory limits have been disabled, a more > complete refactoring of memory management is planned, the changes made here > were to allow the creation of a childAllocator without access to a fragment > context. I have added two comments next to the modifiations I made to the > optiq rule for our use case. > > > Diffs > ----- > > > contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HiveUDFOperator.java > 7524690 > exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java d87fb76 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java > 32cf362 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctionHelpers.java > 3b1d7ef > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java > 35c35ec > exec/java-exec/src/main/java/org/apache/drill/exec/ops/BufferManager.java > 536f6fd > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRule.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > b1a7189 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java > f320157 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java > 710418b > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java > 608fac7 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyGroupScan.java > 54cad56 > exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java e673230 > exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java > cbb5c6e > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java > 72ad31a > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/TestConstantFolding.java > PRE-CREATION > exec/java-exec/src/test/resources/parquet/alltypes.json PRE-CREATION > pom.xml 05573a1 > > Diff: https://reviews.apache.org/r/30697/diff/ > > > Testing > ------- > > This is a work in progress, some testing has been done, no full unit test run > yet > > > Thanks, > > Jason Altekruse > >
