Skip to content

fix: use regex-based pyformat substitution in cursor.execute()#61

Merged
ClayMav merged 5 commits intowherobots:mainfrom
james-willis:fix/cursor-percent-in-like-queries
Mar 17, 2026
Merged

fix: use regex-based pyformat substitution in cursor.execute()#61
ClayMav merged 5 commits intowherobots:mainfrom
james-willis:fix/cursor-percent-in-like-queries

Conversation

@james-willis
Copy link
Contributor

@james-willis james-willis commented Mar 16, 2026

Summary

  • Replaces the raw operation % (parameters or {}) call in cursor.execute() with a regex-based _substitute_parameters() helper that only matches %(name)s pyformat tokens.
  • Literal % characters in SQL (e.g. LIKE '%good%') are left untouched, even when parameters are provided. No %% escaping is needed.
  • Unknown parameter keys now raise ProgrammingError with a clear message.
  • Adds a PEP 249 compliance section to the README documenting the module-level globals (apilevel, threadsafety, paramstyle) and parameterized query usage with examples.

Test plan

18 new tests in tests/test_cursor.py across two classes:

TestCursorExecuteParameterSubstitution (11 tests) — end-to-end through cursor.execute():

  • LIKE queries with % wildcards (leading, trailing, both sides, multiple clauses)
  • parameters=None and parameters={} with percent-containing queries
  • Named parameter substitution, multiple parameters
  • LIKE wildcards combined with named parameters (no %% escaping)
  • Unknown parameter key raises ProgrammingError

TestSubstituteParameters (7 tests) — unit tests for the helper directly:

  • None/empty params return operation unchanged
  • Named param substitution
  • Literal % preserved alongside params
  • Unknown key raises ProgrammingError
  • Repeated param name substituted everywhere
  • Bare %s (format-style) is not treated as a pyformat param

@james-willis james-willis requested a review from a team as a code owner March 16, 2026 18:55
@james-willis james-willis requested review from peterfoldes and removed request for a team March 16, 2026 18:55
Copy link

@salty-hambot salty-hambot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Salty Hambot 🤖🧂

Fix works for the empty-params case but leaves a landmine for non-empty params with literal % — a comment noting the limitation and a negative regression test would make this much more defensible.

2 finding(s) posted.
💬 To request a re-review, comment @salty-hambot review

@james-willis james-willis marked this pull request as draft March 16, 2026 19:40
Replace Python's % string formatting with a regex that only matches
%(name)s tokens, leaving literal percent characters (e.g. LIKE '%good')
untouched. This fixes TypeError/ValueError when SQL contains % wildcards.

Also adds a PEP 249 compliance section to the README documenting the
module-level globals (apilevel, threadsafety, paramstyle) and
parameterized query usage.
@james-willis james-willis force-pushed the fix/cursor-percent-in-like-queries branch from bf944d8 to 73599bd Compare March 16, 2026 21:08
@james-willis james-willis changed the title fix: skip %-formatting in cursor.execute() when no parameters are provided fix: use regex-based pyformat substitution in cursor.execute() Mar 16, 2026
@james-willis james-willis marked this pull request as ready for review March 16, 2026 21:21
Copy link

@salty-hambot salty-hambot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Salty Hambot 🤖🧂

The regex rewrite elegantly dodges the %-formatting trap, but str() substitution without quoting means every string parameter produces invalid SQL — and the README's own examples are broken by it.

2 finding(s) posted.
💬 To request a re-review, comment @salty-hambot review

@sfishel18 sfishel18 self-requested a review March 16, 2026 21:29
String values are now single-quoted with internal quotes escaped,
None becomes NULL, booleans become TRUE/FALSE, bytes become X'hex',
and numeric types pass through unquoted. This complies with the
PEP 249 requirement that the driver handle proper quoting for
client-side parameter interpolation.
Copy link

@salty-hambot salty-hambot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Salty Hambot 🤖🧂

Looking good on the third pass — the quoting and regex rewrite are solid. One edge case left: float('inf') and float('nan') sneak through _quote_value and produce invalid SQL literals that'll blow up server-side.

1 finding(s) posted.
💬 To request a re-review, comment @salty-hambot review

@notion-workspace
Copy link

ClayMav
ClayMav previously approved these changes Mar 16, 2026
@sfishel18
Copy link
Contributor

is there no third-party library we can use to render a parameterized sql statement?

@james-willis
Copy link
Contributor Author

Not that I could find Simon

@ClayMav ClayMav merged commit 93f496d into wherobots:main Mar 17, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants