You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "tkobayas (via GitHub)" <gi...@apache.org> on 2026/10/21 05:29:50 UTC

[PR] [incubator-kie#7209] Ignore @Propagation(IMMEDIATE) in sequential mode [incubator-kie]

tkobayas opened a new pull request, #7220:
URL: https://github.com/apache/incubator-kie/pull/7220

   Data-driven evaluation bypasses the agenda and evaluates paths as soon as segments link.  In sequential mode tuple memory is disabled, so this early evaluation hits null tuple memory causing NPE or incorrect results.
   
   Disable data-driven flag at runtime in PathMemory.isRuleDataDriven when the session is sequential, and log a WARN in ReteooRuleBuilder when a data-driven rule is added to a sequential kbase.
   


-- 
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: commits-unsubscribe@kie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org


Re: [PR] [incubator-kie#7209] Ignore @Propagation(IMMEDIATE) in sequential mode [incubator-kie]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@kie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org


Re: [PR] [incubator-kie#7209] Ignore @Propagation(IMMEDIATE) in sequential mode [incubator-kie]

Posted by "tkobayas (via GitHub)" <gi...@apache.org>.
tkobayas merged PR #7220:
URL: https://github.com/apache/incubator-kie/pull/7220


-- 
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: commits-unsubscribe@kie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org


Re: [PR] [incubator-kie#7209] Ignore @Propagation(IMMEDIATE) in sequential mode [incubator-kie]

Posted by "tkobayas (via GitHub)" <gi...@apache.org>.
tkobayas commented on code in PR #7220:
URL: https://github.com/apache/incubator-kie/pull/7220#discussion_r4356549017


##########
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 is intentional. `ForceEagerActivationFilter` is not related to the eager agenda (`addEagerRuleAgendaItem`, which is already a no-op in sequential mode). It only feeds `PathMemory.dataDriven`, so it triggers exactly the same data-driven path as `@Propagation(IMMEDIATE)` and would hit the same NPE / incorrect results with disabled tuple memory. So it must be disabled in sequential mode too.



-- 
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: commits-unsubscribe@kie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org


Re: [PR] [incubator-kie#7209] Ignore @Propagation(IMMEDIATE) in sequential mode [incubator-kie]

Posted by "tkobayas (via GitHub)" <gi...@apache.org>.
tkobayas commented on code in PR #7220:
URL: https://github.com/apache/incubator-kie/pull/7220#discussion_r4356554774


##########
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:
   `RuleImpl.setDataDriven(true)` is only called for `@Propagation(IMMEDIATE)` (`RuleBuilder` for DRL, `KiePackagesBuilder` for the executable model), so the message is accurate. `ForceEagerActivationOption` is a session option and isn't known at kbase build time, so it isn't covered by this warning.



-- 
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: commits-unsubscribe@kie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org