Copilot commented on code in PR #7220:
URL: https://github.com/apache/incubator-kie/pull/7220#discussion_r4335119350
##########
drools-core/src/main/java/org/drools/core/reteoo/PathMemory.java:
##########
@@ -62,6 +62,10 @@ protected boolean initDataDriven(ReteEvaluator
reteEvaluator) {
}
protected boolean isRuleDataDriven(ReteEvaluator reteEvaluator, RuleImpl
rule) {
+ // Data-driven evaluation in sequential mode causes NPE and incorrect
results (incubator-kie#7209)
+ if (reteEvaluator != null && reteEvaluator.isSequential()) {
+ return false;
+ }
return rule != null &&
(rule.isDataDriven() || (reteEvaluator != null &&
reteEvaluator.getRuleSessionConfiguration()
.getForceEagerActivationFilter().accept(rule)));
Review Comment:
This early return disables not only `rule.isDataDriven()` (IMMEDIATE) but
also the `ForceEagerActivationFilter` path, which can unintentionally change
semantics for rules/configurations relying on forced eager activation in
sequential sessions. If the intent is strictly to ignore
`@Propagation(IMMEDIATE)` (data-driven) while keeping other eager mechanisms
available, refactor the boolean expression to only gate `rule.isDataDriven()`
behind `!reteEvaluator.isSequential()` and still allow
`getForceEagerActivationFilter().accept(rule)` to return true.
##########
drools-core/src/main/java/org/drools/core/reteoo/builder/ReteooRuleBuilder.java:
##########
@@ -120,6 +124,10 @@ public List<TerminalNode> addRule(InternalRuleBase kBase,
Collection<InternalWor
// transform rule and gets the array of subrules
final GroupElement[] subrules = rule.getTransformedLhs(
LogicTransformer.getInstance(), kBase.getGlobals() );
+ if (kBase.getRuleBaseConfiguration().isSequential() &&
rule.isDataDriven()) {
+ log.warn("@Propagation(IMMEDIATE) on rule '{}' is ignored in
sequential mode", rule.getName());
+ }
Review Comment:
The WARN message states `@Propagation(IMMEDIATE)`, but the condition is
`rule.isDataDriven()`. If `dataDriven` can be enabled via mechanisms other than
the IMMEDIATE annotation, this message may be misleading. Consider wording it
in terms of 'data-driven propagation' (and optionally mention IMMEDIATE as a
common cause) unless you can reliably detect that the rule was marked
data-driven specifically by `@Propagation(IMMEDIATE)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
