Repository navigation
[Python/YAML] Accept >=, <=, == and != in generic Filter expressions … - #41044
Conversation
…pache#41043) validate_generic_expression split comparisons with a greedy regex, so a two-character operator lost its first character to the left operand ('age >= 21' became 'age >', '=', ' 21') and the expression was rejected. Only < and > worked. Make the left operand non-greedy and add tests for every operator. Fixes apache#41043.
There was a problem hiding this comment.
📋 Review Summary
This PR fixes a regex greediness bug in validate_generic_expression where two-character comparison operators (>=, <=, ==, !=) were split across the left operand and operator capture groups in Beam YAML generic expressions. The change makes the left operand capture group non-greedy and adds comprehensive unit and pipeline tests covering all supported comparison operators.
🔍 General Feedback
- The fix in
sdks/python/apache_beam/yaml/yaml_mapping.pyis minimal, accurate, and cleanly preserves full multi-character operators during validation. - Unit tests in
ValidateGenericExpressionTestand the end-to-endFiltertest inYamlMappingTestthoroughly cover valid operators, whitespace variations, and invalid expressions.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @derrickaw for label yaml. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
LGTM, lets wait on checks, thanks |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41044 +/- ##
============================================
- Coverage 58.96% 58.95% -0.01%
Complexity 4768 4768
============================================
Files 1464 1464
Lines 185853 185853
Branches 3585 3585
============================================
- Hits 109580 109578 -2
- Misses 72927 72929 +2
Partials 3346 3346
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…(#41043)
validate_generic_expression split comparisons with a greedy regex, so a two-character operator lost its first character to the left operand ('age >= 21' became 'age >', '=', ' 21') and the expression was rejected. Only < and > worked. Make the left operand non-greedy and add tests for every operator.
Fixes #41043.