Fix SQL f-string placeholder causing parsing errors in interval expressions#7881
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Is this true? Could we add a parameterized test with various contexts which this gets replaced. im sure we can't get 100%, but would be good to document the coverage for it. e..g. |
|
Added a new pytest.mark.parametrize parameterized coverage test in tests/_ast/test_visitor.py for normalize_sql_f_string() across several common SQL contexts (WHERE comparisons, BETWEEN, IN, LIKE with quoted pattern, LIMIT/OFFSET, VALUES). For the subset that should produce valid SQL after normalization, the test also runs a best effort sqlglot parse (when sqlglot is installed) to document the intended coverage. |
|
thank you @VedantMadane for the improvement! |
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.19.5-dev24 |
…ssions (marimo-team#7881) ## Summary Fix spurious SQL parsing errors when f-string variables are used in interval expressions. When using f-strings in `mo.sql()` like `interval '{days_ago} days'`, the static analysis would replace the variable with "null", creating `interval 'null days'` which is invalid SQL. This caused alarming ERROR log messages even though the query executed successfully. ## Changes - Changed the f-string placeholder from "null" to "1" in `normalize_sql_f_string()` - "1" is valid in more SQL contexts (e.g., `interval '1 days'` parses correctly) - Updated tests to reflect the new placeholder - Added regression test for interval expression case ## Test plan - [x] Added `test_normalize_sql_f_string_with_interval` regression test - [x] Updated existing tests to expect "1" instead of "null" Fixes marimo-team#7717
Summary
Fix spurious SQL parsing errors when f-string variables are used in interval expressions.
When using f-strings in
mo.sql()likeinterval '{days_ago} days', the static analysis would replace the variable with "null", creatinginterval 'null days'which is invalid SQL. This caused alarming ERROR log messages even though the query executed successfully.Changes
normalize_sql_f_string()interval '1 days'parses correctly)Test plan
test_normalize_sql_f_string_with_intervalregression testFixes #7717