xlstream-eval: support text comparison operators in conditional criteria#188
Merged
Conversation
3 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes: #175
Fixes: #174
Problem
Two related bugs in conditional aggregate criteria:
COUNTIF text operator criteria returns 0 #175: SUMIF/COUNTIF with text comparison operators (
">b",">=banana","<banana") always returned 0. The criteria parser only handled numeric operands — non-numeric text fell through toCriteria::Equalswith the full operator string as a literal, matching nothing.COUNTIF/COUNTIFS operator criteria counts unique matching values, not all rows #174: COUNTIF/COUNTIFS with any operator or wildcard criteria counted matching buckets instead of matching rows.
try_operator_criteriare-folded already-finished per-bucket values usingAggKind::Count, which counts feeds (one per bucket) instead of summing stored row counts.Fix
Text comparison variants: Add
GreaterText,GreaterOrEqText,LessText,LessOrEqTextto theCriteriaenum. When an operator operand failsparse::<f64>(), the parser creates the text variant (lowercased) instead of falling through toEquals. Matching uses case-insensitive alphabetical comparison via a newtext_for_comparehelper, applied only toValue::Textcells.COUNT re-fold fix:
try_operator_criterianow finishes withAggKind::SumforCountkind — matching how the prelude builder stores row counts (each bucket's value is already the sum of 1.0-per-row feeds). This makes COUNTIF sum matching buckets' row counts instead of counting the number of matching buckets.Changes
crates/xlstream-eval/src/criteria.rs: add 4 text-comparison enum variants; updateparse()fallbacks; addtext_for_comparehelper; add match arms; update rustdoc.crates/xlstream-eval/src/prelude.rs:try_operator_criteriausesAggKind::Sumfinish forCountkind.docs/architecture/aggregates.md: syncCriteriaenum listing with new variants.Test plan
cargo test -p xlstream-evalpasses (2292 lib, no regressions)make checkpassesCloses #175
Closes #174