Fix handling of aggregate literals with only a right-hand comparison term (Issue #311)

[madmike200590]

Fixed in AggregateOperatorNormalization: Aggregates of form #aggr{...} OP TERM are rewritten into TERM inv(OP) #aggr{...} and then processed as before. inv(OP) refers to the inverse operator, i.e. inv(<) = >= etc. For operators = and != no inverse operator is used, i.e. rewritten form is TERM OP #aggr{...}.

Closes #311

[codecov[bot]]

Codecov Report

Merging #318 (3848c87) into master (fd5e5ba) will increase coverage by 0.02%. The diff coverage is 64.70%.

@@             Coverage Diff              @@
##             master     #318      +/-   ##
============================================
+ Coverage     69.51%   69.53%   +0.02%     
- Complexity     2092     2101       +9     
============================================
  Files           182      182              
  Lines          8006     8023      +17     
  Branches       1416     1423       +7     
============================================
+ Hits           5565     5579      +14     
+ Misses         2086     2084       -2     
- Partials        355      360       +5     

Impacted Files	Coverage Δ
...ion/aggregates/AggregateOperatorNormalization.java	81.13% <64.70%> (-7.76%)	:arrow_down:
.../tuwien/kr/alpha/core/parser/ParseTreeVisitor.java	85.18% <0.00%> (+0.46%)	:arrow_up:
...ormation/aggregates/AggregateLiteralSplitting.java	80.32% <0.00%> (+1.63%)	:arrow_up:
...a/at/ac/tuwien/kr/alpha/core/atoms/ChoiceAtom.java	42.85% <0.00%> (+2.38%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update fd5e5ba...3848c87. Read the comment docs.

[AntoniusW]

Fixed in AggregateOperatorNormalization: Aggregates of form #aggr{...} OP TERM are rewritten into TERM inv(OP) #aggr{...} and then processed as before. inv(OP) refers to the inverse operator, i.e. inv(<) = >= etc. For operators = and != no inverse operator is used, i.e. rewritten form is TERM OP #aggr{...}.

Uhm, I beg to disagree: #agg {..} > 2 should be equal to 2 < #agg {..}, i.e., what is needed here is flipping of operators, not negation.

Consider for example X < Y which is flipped Y > X and not Y >= X, to see that consider the case with X=2,Y=2 and we have 2 < 2 is false as is 2 > 2 while 2 >= 2 becomes true. The unit tests currently do not consider such a case and hence cannot show that the implementation is bugged at the moment.

Please change that.

[madmike200590]

Fixed in AggregateOperatorNormalization: Aggregates of form #aggr{...} OP TERM are rewritten into TERM inv(OP) #aggr{...} and then processed as before. inv(OP) refers to the inverse operator, i.e. inv(<) = >= etc. For operators = and != no inverse operator is used, i.e. rewritten form is TERM OP #aggr{...}.

Uhm, I beg to disagree: #agg {..} > 2 should be equal to 2 < #agg {..}, i.e., what is needed here is flipping of operators, not negation.

Consider for example X < Y which is flipped Y > X and not Y >= X, to see that consider the case with X=2,Y=2 and we have 2 < 2 is false as is 2 > 2 while 2 >= 2 becomes true. The unit tests currently do not consider such a case and hence cannot show that the implementation is bugged at the moment.

Please change that.

Thanks for catching this! Since the actual rewriting in AggregateOperatorNormalization uses operator negation in quite a few places, I mentally tripped myself up here :sweat_smile:

This should now be fixed.