callproc: escape procname#1225
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1225 +/- ##
==========================================
- Coverage 86.24% 84.43% -1.82%
==========================================
Files 17 17
Lines 2436 2441 +5
Branches 258 241 -17
==========================================
- Hits 2101 2061 -40
- Misses 249 295 +46
+ Partials 86 85 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes issue #1206 where Cursor.callproc() fails for stored procedures whose identifier requires quoting (e.g., contains a dot), especially when arguments are provided (due to server variable naming).
Changes:
- Quote/escape
procnameinCursor.callproc()and also quote the generated@_procname_nuser variables. - Add a regression test that creates and calls a procedure named with a dot (
`foo.bar`).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
pymysql/cursors.py |
Changes callproc() to backtick-quote/escape procedure names and parameter user variables. |
pymysql/tests/test_issues.py |
Adds regression coverage for calling a procedure whose name requires quoting. |
Comments suppressed due to low confidence (1)
pymysql/cursors.py:267
- The
SETstatement still uses old-style%formatting (fmt % (...)) even thoughfmtnow embedsprocname_escaped. Ifprocnamecontains a%character (legal in a backquoted identifier), this will raise a Python formatting error because%will be treated as a format specifier. Consider building these fragments without%-formatting (e.g., using f-strings for the whole assignment) so procedure names with%can’t breakcallproc().
fmt = f"@`_{procname_escaped}_%d`=%s"
self._query(
"SET %s"
% ",".join(
fmt % (index, conn.escape(arg)) for index, arg in enumerate(args)
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| q = "CALL `{}`({})".format( | ||
| procname_escaped, | ||
| ",".join([f"@`_{procname_escaped}_{i}`" for i in range(len(args))]), |
There was a problem hiding this comment.
callproc() now always wraps procname in backticks (e.g., CALL db.proc``), which changes behavior for callers who previously passed schema-qualified names like db.proc (this will now look for a procedure literally named `db.proc` in the current schema). Consider preserving the ability to call qualified procedures (e.g., by detecting and quoting each identifier part separately or by documenting/handling qualified names explicitly) so this fix doesn’t introduce a breaking change.
| fmt = f"@`_{procname_escaped}_%d`=%s" | ||
| self._query( |
There was a problem hiding this comment.
callproc() now uses backtick-quoted user variables (e.g., @_proc_0``). The docstring above still states variables are named @_procname_n and suggests selecting them without quoting; please update the docstring to reflect the new quoting requirement so OUT/INOUT retrieval guidance stays correct.
| cur.execute( | ||
| dedent("""\ | ||
| create procedure `foo.bar` (arg1 int) | ||
| begin | ||
| select arg1*2; | ||
| end | ||
| """) | ||
| ) | ||
|
|
||
| cur.callproc("foo.bar", args=(123,)) | ||
| self.assertEqual(cur.fetchone()[0], 246) |
There was a problem hiding this comment.
After callproc(), MySQL/MariaDB produce an extra empty result set for the CALL itself. This test fetches one row but doesn’t advance through remaining result sets (or close the cursor/connection), which can cause “commands out of sync”/disconnect issues in some environments. Consider consuming nextset() until it returns None (as the callproc docstring recommends) and closing the cursor/connection via try/finally or context managers.
| cur.execute( | |
| dedent("""\ | |
| create procedure `foo.bar` (arg1 int) | |
| begin | |
| select arg1*2; | |
| end | |
| """) | |
| ) | |
| cur.callproc("foo.bar", args=(123,)) | |
| self.assertEqual(cur.fetchone()[0], 246) | |
| try: | |
| cur.execute( | |
| dedent("""\ | |
| create procedure `foo.bar` (arg1 int) | |
| begin | |
| select arg1*2; | |
| end | |
| """) | |
| ) | |
| cur.callproc("foo.bar", args=(123,)) | |
| row = cur.fetchone() | |
| self.assertEqual(row[0], 246) | |
| while cur.nextset() is not None: | |
| pass | |
| finally: | |
| cur.close() | |
| conn.close() |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
fix #1206