Skip to content

fix(c/driver/sqlite): accept unprefixed named parameters - #4773

Open
emecii wants to merge 3 commits into
apache:mainfrom
emecii:fix/sqlite-unprefixed-named-parameters
Open

emecii wants to merge 3 commits into
apache:mainfrom
emecii:fix/sqlite-unprefixed-named-parameters

Conversation

@emecii

@emecii emecii commented Sep 10, 2026

Copy link
Copy Markdown

Summary

Accept unprefixed names for SQLite's :, @, and $ named parameters when the match is unique. Exact prefixed names keep their existing behavior; ambiguous unprefixed names produce an explicit error. Positional binding and parameter-count validation are unchanged.

AI disclosure: OpenAI Codex generated this implementation, tests, and PR description; automated validation is listed above.

Closes #3520

Preserve exact named bindings and reject ambiguous prefix-free matches. Add native and Python regression coverage and document the matching rules.

Generated-by: OpenAI Codex

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Alias and exact input names can resolve to one index while another SQL parameter remains silently unbound.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Enables SQLite named binding without :, @, or $ prefixes when uniquely matched.

Changes:

  • Adds unique unprefixed-name resolution and ambiguity errors.
  • Adds native and Python regression coverage.
  • Documents matching and precedence rules.
File summaries
File Description
c/driver/sqlite/statement_reader.c Implements unprefixed matching.
c/driver/sqlite/sqlite_test.cc Adds native binding tests.
python/adbc_driver_sqlite/tests/test_dbapi.py Adds DB-API regressions.
docs/source/driver/sqlite.rst Documents named parameters.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread c/driver/sqlite/statement_reader.c
@lidavidm lidavidm added this to the ADBC Libraries 25 milestone Sep 21, 2026

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems reasonable overall. One question though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have we verified this against what the stdlib sqlite does?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Verified against Python 3.12.12’s sqlite3. Common unprefixed bindings agree, including repeated parameters and executemany. Differences: stdlib reuses a for both :a/@a and ignores extra keys; ADBC preserves exact-prefixed keys, enforces field count, and rejects ambiguous aliases. Numbered ?1 uses "1" in stdlib versus "?1" here. Added direct comparisons and documentation in bafb27e; all 12 named-parameter tests pass. Production behavior is unchanged.

AI-generated reply (OpenAI Codex).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Named parameter binding not consistent with DBAPI (stdlib sqlite)

3 participants