You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Jinfeng Ni <jn...@maprtech.com> on 2014/02/04 20:17:42 UTC
Re: Review Request 17333: Drill-346 : CodeGenerator generate evaluation code
in doSetup method for constant expression.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17333/
-----------------------------------------------------------
(Updated Feb. 4, 2014, 11:17 a.m.)
Review request for drill.
Changes
-------
Additional changes :
1. EvaluationVisitor will check if the current scope is within Constant scope, by call MappingSet.isWithinConstant(). If the current scope is within Constant scope, then the HoldingContainer for the expression will be marked as constant. (This implies the constant HoldingContainer will be accessible in doSetup() method)
2. MappingSet is modified to make sure the constant GeneratorMapping is different from other GeneratorMapping, if not so.
3. Add Param annotation, to indicate some parameters are required to be constant. If the argument turns out to not a constant in code generation time, a runtime exception will be thrown.
Repository: drill-git
Description
-------
In this patch, the following code changes are made:
1. CodeGenerator.java : 1) use DEFAULT_CONSTANT_MAP for constant scope when generates code.
2) Add a field isConst to HoldingContainer, to indicate if a HoldingContainer corresponds to a constant expression.
3) use the constant boundaries which are determined by ConstantExpressionIdentifier.
2. ConstantExpressionIdentifier.java : fix bugs to correct identify constant boundaries for an expression.
3. MappingSet.java: Switch the GeneratorMapping when enter/exit a constant scope.
4. EvaluationVisitor.java: In ConstantFilter, when visits a constant expression, make sure the HoldingContainer would declare a classField, in stead of local var, and set isConst.
5. DrillSimpleFuncHolder : pass in the array of inputVariables when generated SETUP block, making them accessible if the input is a constant in SETUP block.
Other related changes:
1. Add a new type of FunctionDefinition, to model non-deterministic functions, such as randomBigInt(3), alternator(), etc.
2. Make sure GeneratorMapping is static var; MappingSet is instance variable; Make sure the naming convention for the change is consistent.
Diffs (updated)
-----
common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java f99150e
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/MappingSet.java 1cb2380
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java e674eab
exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3
exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java 20c0746
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 3193c76
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0
Diff: https://reviews.apache.org/r/17333/diff/
Testing
-------
Test is mainly done through running the existing unit testcases; no new unit testcase is added.
In addition, I verified from the log that the generated java codes indeed put the evaluation logic in doSetup for the following expressions:
1+2
cast(123 as varchar(10))
substring(varcharcol, 1+1, 1+2)
Thanks,
Jinfeng Ni
Re: Review Request 17333: Drill-346 : CodeGenerator generate evaluation code
in doSetup method for constant expression.
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17333/
-----------------------------------------------------------
(Updated March 11, 2014, 4:53 p.m.)
Review request for drill.
Changes
-------
Rebase on latest master branch.
Repository: drill-git
Description
-------
In this patch, the following code changes are made:
1. CodeGenerator.java : 1) use DEFAULT_CONSTANT_MAP for constant scope when generates code.
2) Add a field isConst to HoldingContainer, to indicate if a HoldingContainer corresponds to a constant expression.
3) use the constant boundaries which are determined by ConstantExpressionIdentifier.
2. ConstantExpressionIdentifier.java : fix bugs to correct identify constant boundaries for an expression.
3. MappingSet.java: Switch the GeneratorMapping when enter/exit a constant scope.
4. EvaluationVisitor.java: In ConstantFilter, when visits a constant expression, make sure the HoldingContainer would declare a classField, in stead of local var, and set isConst.
5. DrillSimpleFuncHolder : pass in the array of inputVariables when generated SETUP block, making them accessible if the input is a constant in SETUP block.
Other related changes:
1. Add a new type of FunctionDefinition, to model non-deterministic functions, such as randomBigInt(3), alternator(), etc.
2. Make sure GeneratorMapping is static var; MappingSet is instance variable; Make sure the naming convention for the change is consistent.
Diffs (updated)
-----
common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 7a48112
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java f99150e
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/MappingSet.java 1cb2380
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java e674eab
exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3
exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java 20c0746
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java bd668e7
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 3193c76
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 72f1ad9
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 381fbe2
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 5fd12c0
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0
Diff: https://reviews.apache.org/r/17333/diff/
Testing
-------
Test is mainly done through running the existing unit testcases; no new unit testcase is added.
In addition, I verified from the log that the generated java codes indeed put the evaluation logic in doSetup for the following expressions:
1+2
cast(123 as varchar(10))
substring(varcharcol, 1+1, 1+2)
Thanks,
Jinfeng Ni
Re: Review Request 17333: Drill-346 : CodeGenerator generate evaluation code
in doSetup method for constant expression.
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17333/
-----------------------------------------------------------
(Updated Feb. 19, 2014, 4:30 p.m.)
Review request for drill.
Changes
-------
Code cleanup and refactoring.
Repository: drill-git
Description
-------
In this patch, the following code changes are made:
1. CodeGenerator.java : 1) use DEFAULT_CONSTANT_MAP for constant scope when generates code.
2) Add a field isConst to HoldingContainer, to indicate if a HoldingContainer corresponds to a constant expression.
3) use the constant boundaries which are determined by ConstantExpressionIdentifier.
2. ConstantExpressionIdentifier.java : fix bugs to correct identify constant boundaries for an expression.
3. MappingSet.java: Switch the GeneratorMapping when enter/exit a constant scope.
4. EvaluationVisitor.java: In ConstantFilter, when visits a constant expression, make sure the HoldingContainer would declare a classField, in stead of local var, and set isConst.
5. DrillSimpleFuncHolder : pass in the array of inputVariables when generated SETUP block, making them accessible if the input is a constant in SETUP block.
Other related changes:
1. Add a new type of FunctionDefinition, to model non-deterministic functions, such as randomBigInt(3), alternator(), etc.
2. Make sure GeneratorMapping is static var; MappingSet is instance variable; Make sure the naming convention for the change is consistent.
Diffs (updated)
-----
common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java f99150e
exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/MappingSet.java 1cb2380
exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java e674eab
exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3
exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java 20c0746
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 3193c76
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0
Diff: https://reviews.apache.org/r/17333/diff/
Testing
-------
Test is mainly done through running the existing unit testcases; no new unit testcase is added.
In addition, I verified from the log that the generated java codes indeed put the evaluation logic in doSetup for the following expressions:
1+2
cast(123 as varchar(10))
substring(varcharcol, 1+1, 1+2)
Thanks,
Jinfeng Ni