-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[DROOLS-1175] infer numeric type for sum expression in an accumulate … #827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| import org.drools.compiler.rule.builder.RuleBuildContext; | ||
| import org.drools.compiler.rule.builder.RuleConditionBuilder; | ||
| import org.drools.compiler.rule.builder.dialect.java.parser.JavaLocalDeclarationDescr; | ||
| import org.drools.compiler.rule.builder.dialect.mvel.MVELExprAnalyzer; | ||
| import org.drools.compiler.rule.builder.util.PackageBuilderUtil; | ||
| import org.drools.core.base.accumulators.JavaAccumulatorFunctionExecutor; | ||
| import org.drools.core.base.extractors.ArrayElementReader; | ||
|
|
@@ -47,6 +48,8 @@ | |
| import org.drools.core.util.index.IndexUtil; | ||
| import org.kie.api.runtime.rule.AccumulateFunction; | ||
|
|
||
| import java.math.BigDecimal; | ||
| import java.math.BigInteger; | ||
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
| import java.util.Comparator; | ||
|
|
@@ -124,12 +127,12 @@ public RuleConditionElement build( final RuleBuildContext context, | |
| return accumulate; | ||
| } | ||
|
|
||
| private Accumulate buildExternalFunctionCall( final RuleBuildContext context, | ||
| final AccumulateDescr accumDescr, | ||
| final RuleConditionElement source, | ||
| private Accumulate buildExternalFunctionCall( RuleBuildContext context, | ||
| AccumulateDescr accumDescr, | ||
| RuleConditionElement source, | ||
| Map<String, Declaration> declsInScope, | ||
| Map<String, Class< ? >> declCls, | ||
| final boolean readLocalsFromTuple) { | ||
| boolean readLocalsFromTuple) { | ||
| // list of functions to build | ||
| final List<AccumulateFunctionCallDescr> funcCalls = accumDescr.getFunctions(); | ||
| // list of available source declarations | ||
|
|
@@ -150,7 +153,7 @@ private Accumulate buildExternalFunctionCall( final RuleBuildContext context, | |
|
|
||
| int index = 0; | ||
| for ( AccumulateFunctionCallDescr fc : funcCalls ) { | ||
| AccumulateFunction function = getAccumulateFunction(context, accumDescr, fc); | ||
| AccumulateFunction function = getAccumulateFunction(context, accumDescr, fc, source, declCls); | ||
| if (function == null) { | ||
| return null; | ||
| } | ||
|
|
@@ -164,7 +167,7 @@ private Accumulate buildExternalFunctionCall( final RuleBuildContext context, | |
| accumulators ); | ||
| } else { | ||
| AccumulateFunctionCallDescr fc = accumDescr.getFunctions().get(0); | ||
| AccumulateFunction function = getAccumulateFunction(context, accumDescr, fc); | ||
| AccumulateFunction function = getAccumulateFunction(context, accumDescr, fc, source, declCls); | ||
| if (function == null) { | ||
| return null; | ||
| } | ||
|
|
@@ -211,22 +214,45 @@ private void bindReaderToDeclaration( RuleBuildContext context, AccumulateDescr | |
| } | ||
| } | ||
|
|
||
| private AccumulateFunction getAccumulateFunction(RuleBuildContext context, AccumulateDescr accumDescr, AccumulateFunctionCallDescr fc) { | ||
| private AccumulateFunction getAccumulateFunction(RuleBuildContext context, | ||
| AccumulateDescr accumDescr, | ||
| AccumulateFunctionCallDescr fc, | ||
| RuleConditionElement source, | ||
| Map<String, Class< ? >> declCls) { | ||
| String functionName = getFunctionName( context, fc, source, declCls ); | ||
|
|
||
| // find the corresponding function | ||
| AccumulateFunction function = context.getConfiguration().getAccumulateFunction( fc.getFunction() ); | ||
| AccumulateFunction function = context.getConfiguration().getAccumulateFunction( functionName ); | ||
| if( function == null ) { | ||
| // might have been imported in the package | ||
| function = context.getKnowledgeBuilder().getPackage().getAccumulateFunctions().get(fc.getFunction()); | ||
| function = context.getKnowledgeBuilder().getPackage().getAccumulateFunctions().get( functionName ); | ||
| } | ||
| if ( function == null ) { | ||
| context.addError( new DescrBuildError( accumDescr, | ||
| context.getRuleDescr(), | ||
| null, | ||
| "Unknown accumulate function: '" + fc.getFunction() + "' on rule '" + context.getRuleDescr().getName() + "'. All accumulate functions must be registered before building a resource." ) ); | ||
| "Unknown accumulate function: '" + functionName + "' on rule '" + context.getRuleDescr().getName() + "'. All accumulate functions must be registered before building a resource." ) ); | ||
| } | ||
| return function; | ||
| } | ||
|
|
||
| private String getFunctionName( RuleBuildContext context, AccumulateFunctionCallDescr fc, RuleConditionElement source, Map<String, Class<?>> declCls ) { | ||
| String functionName = fc.getFunction(); | ||
| if (functionName.equals( "sum" )) { | ||
| Class<?> exprClass = MVELExprAnalyzer.getExpressionType( context, declCls, source, fc.getParams()[0] ); | ||
| if (exprClass == int.class || exprClass == Integer.class) { | ||
| functionName = "sumI"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For clarity, I believe they should have been named "sumInteger" etc, but I believe that ship has already sailed, didn't it? |
||
| } else if (exprClass == long.class || exprClass == Long.class) { | ||
| functionName = "sumL"; | ||
| } else if (exprClass == BigInteger.class) { | ||
| functionName = "sumBI"; | ||
| } else if (exprClass == BigDecimal.class) { | ||
| functionName = "sumBD"; | ||
| } | ||
| } | ||
| return functionName; | ||
| } | ||
|
|
||
| private Accumulator buildAccumulator(RuleBuildContext context, AccumulateDescr accumDescr, Map<String, Declaration> declsInScope, Map<String, Class<?>> declCls, boolean readLocalsFromTuple, Declaration[] sourceDeclArr, Set<Declaration> requiredDecl, AccumulateFunctionCallDescr fc, AccumulateFunction function) { | ||
| // analyze the expression | ||
| final JavaAnalysisResult analysis = (JavaAnalysisResult) context.getDialect().analyzeBlock( context, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getFunctionName doesn't deal with averageBD, maxDB, etc, so I suspect that the BigDecimalAverageAccumulateFunction. We or QA will need to build a unit test for very combination of functionName and type, just to be sure that they all work properly.