Fix Python SDKs lowercasing output expressions (corrupts string literals) - #3480
Harsh23Kashyap wants to merge 1 commit into
Conversation
…erals Both Python SDK query builders lowercased every output column string before parsing it, to match the special tokens (*, _row_id, _score, ...) case-insensitively. That also rewrites string literals inside real expressions: output(["'ACTIVE'"]) selects 'active', and once json_extract(...) parses through the SDKs, json_extract(data, '$.UserName') would silently query key '$.username'. Match the special tokens against a lowercased copy and parse the original string. Plain identifiers are still lowercased by the Column arm, so name handling is unchanged. Regression test: test_output_preserves_string_literal_case.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe Python embedded and Thrift query builders now preserve string literal case in ChangesOutput expression case preservation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Output expressions now preserve string-literal case while special output tokens remain case-insensitive, with regression coverage for both Python query builders. No current merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit keeps case safe, Comment |
1 participant
Summary
Both Python SDK query builders lowercased each
output()column string before parsing it, to match the special output tokens case-insensitively. That also rewrote string literals inside real expressions:output(["'ACTIVE'"])silently selected'active', andjson_extract(data, '$.UserName')(once it parses through the SDKs, see #3463) would query the key$.username. The HTTP API forwards the strings untouched, so the same query returned different data per client.The special tokens are now matched against a lowercased copy while the original string is parsed. Plain identifiers are still lowercased by the
exp.Columnarm, so column-name handling is unchanged; only string literals keep their case.Fixes #3479
Testing
'active'; after the fix they hold'ACTIVE'._SCOREstill maps toscore,MyColstill normalizes tomycol,*still works.test_output_preserves_string_literal_casetopython/test_pysdk/test_condition.py(thrift builder directly, embedded builder in embedded mode).output()methods.