Skip to content

Fix SQL f-string placeholder causing parsing errors in interval expressions#7881

Merged
mscolnick merged 3 commits intomarimo-team:mainfrom
VedantMadane:fix-sql-fstring-placeholder
Jan 20, 2026
Merged

Fix SQL f-string placeholder causing parsing errors in interval expressions#7881
mscolnick merged 3 commits intomarimo-team:mainfrom
VedantMadane:fix-sql-fstring-placeholder

Conversation

@VedantMadane
Copy link
Contributor

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

  • Added test_normalize_sql_f_string_with_interval regression test
  • Updated existing tests to expect "1" instead of "null"

Fixes #7717

@vercel
Copy link

vercel bot commented Jan 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
marimo-docs Ready Ready Preview, Comment Jan 20, 2026 5:15pm

Request Review

@Light2Dark Light2Dark added the bug Something isn't working label Jan 19, 2026
@mscolnick
Copy link
Contributor

"1" is valid in more SQL contexts (e.g., interval '1 days' parses correctly)

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.

# WHERE clause 
SELECT * FROM table WHERE {var} = 10
SELECT * FROM table WHERE col = {var}
SELECT * FROM table WHERE {col_name} = {value}
SELECT * FROM table WHERE {var} > 100
SELECT * FROM table WHERE {var} BETWEEN 10 AND 20
SELECT * FROM table WHERE {var} IN (1, 2, 3)
SELECT * FROM table WHERE {var} LIKE '%search%'
SELECT * FROM table WHERE {var} IS NULL
SELECT * FROM table WHERE {var} IS NOT NULL

# Table name
SELECT * FROM {table_name} WHERE col = 10
SELECT * FROM {schema}.{table} WHERE col = 10
SELECT * FROM {database}.{schema}.{table} WHERE col = 10

# JOIN clauses:
SELECT * FROM table1 JOIN {table2} ON table1.id = {table2}.id
SELECT * FROM {table1} JOIN {table2} ON {join_col} = {join_col}
SELECT * FROM table1 {join_type} JOIN table2 ON {condition}

# ORDER BY and LIMIT:
SELECT * FROM table ORDER BY {column}
SELECT * FROM table ORDER BY {column} {direction}
SELECT * FROM table LIMIT {limit}
SELECT * FROM table LIMIT {limit} OFFSET {offset}
SELECT * FROM {table} ORDER BY {col} DESC LIMIT {n}

# Misc:
SELECT * FROM table WHERE {col} = {val} OR {col2} = {val2}
SELECT * FROM table WHERE ({condition1}) AND ({condition2})
SELECT * FROM table WHERE {col} IN ({value_list})
SELECT * FROM table WHERE {date_col} > {date_value}

@VedantMadane
Copy link
Contributor Author

VedantMadane commented Jan 20, 2026

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.

@mscolnick mscolnick merged commit 7e4a36b into marimo-team:main Jan 20, 2026
38 of 41 checks passed
@mscolnick
Copy link
Contributor

thank you @VedantMadane for the improvement!

@github-actions
Copy link

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.19.5-dev24

botterYosuke pushed a commit to botterYosuke/marimo that referenced this pull request Jan 21, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spurious SQL parsing error logged when f-string variable is used in interval expression

3 participants