Skip to content

Match SQLite callback keyword parsing - #9737

Draft
youknowdot wants to merge 3 commits into
RustPython:mainfrom
youknowdot:fix/sqlite-callback-binding-batch-20261107
Draft

youknowdot wants to merge 3 commits into
RustPython:mainfrom
youknowdot:fix/sqlite-callback-binding-batch-20261107

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Retire the remaining expected failures in test_sqlite3/test_userfunctions.py and test_sqlite3/test_hooks.py as one file-level batch.

SQLite's defining-class Clinic wrappers use _PyArg_UnpackKeywords, even when every argument is positional-only. The generic native calling convention previously rejected keywords before that parser could report a missing positional argument or an excess total argument count.

  • Add a typed UnpackKeywords<T> adapter. Its FORCE_UNPACK_KEYWORDS compile-time metadata selects CPython's _PyArg_UnpackKeywords calling and validation conventions through the existing binder, preserving argument conversion and displayed signatures. Positional-only parameters still cannot be passed by keyword.
  • Keep default binding behavior and its successful-call fast path unchanged. Opaque or malformed argument shapes cannot use the adapter.
  • Apply the same convention to aggregate, collation, window-function, authorizer, and trace callback registration.
  • Remove only the three obsolete expected-failure decorators through scripts/update_lib; existing skips, assertions, test logic, and data are unchanged.
  • Add shared-binder coverage for scalar, receiver, and flattened forms, and compile-time dispatch/signature coverage. No duplicate Python snippets.

This is independent of the other open expected-failure batches and is based directly on main ea5fbcc00.

Reference: CPython 3.15 SQLite generated wrappers and _PyArg_UnpackKeywords.

Validation

Behavioral validation before the identifier/documentation-only rename (binding and conversion logic unchanged):

  • SQLite-enabled threaded release build passed.
  • The two complete target modules passed: 106 tests, five existing skips, zero remaining expected failures.
  • Full SQLite, builtin, call, inspect, and descriptor regression modules passed: 1,407 tests, 271 existing skips, four unrelated expected failures.
  • 212 focused binding checks, five text signatures, and a control for ordinary positional-only dispatch passed.
  • Strict workspace all-target Clippy, including SQLite, passed with -D warnings.
  • Formatting, diff checks, normal commit hooks, and independent source review passed.

Before the naming-only revision, workspace tests passed: 1,520 passed, zero failed, 20 ignored across unit, integration, and doc tests. Separate C-API tests passed: 149 passed, zero failed, two ignored, and separate C-API all-target Clippy passed with -D warnings.

On naming revision 1a834ea436, normal commit hooks, formatting, diff checks, and strict workspace all-target Clippy including SQLite have passed again. Workspace tests and separate C-API checks are still running; earlier interrupted compilations are not counted as passing.

CPython 3.15 behavior is checked against the fixed source above; no CPython 3.15 runtime oracle is claimed. A clean merge was verified against current main 9670d1b364, including the subsequent #9706, #9668, and #9718 changes. No unrelated module expansion is included.

Normal hosted CI is enabled. Full hosted and cross-platform validation remains outstanding; this is not a merge-ready claim.

AI assistance: OpenAI Codex assisted implementation, source review, and validation. The model version is not exposed. Commits include Assisted-by trailers.

Preserve typed positional-only signatures while selecting the defining-class Clinic argument parser for callback registration. Derive dispatch and binding policy at compile time; leave the successful-call validation path unchanged.

Assisted-by: OpenAI Codex:model version not exposed
@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/sqlite3
[x] test: cpython/Lib/test/test_sqlite3 (TODO: 8)

dependencies:

  • sqlite3

dependent tests: (2 tests)

  • sqlite3: test_dbm_sqlite3 test_sqlite3

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

Use scripts/update_lib on test_userfunctions.py and test_hooks.py after fixing the native defining-class argument parser. Remove their three remaining decorators, covering six concrete test cases, without changing assertions, logic, or existing skips.

Assisted-by: OpenAI Codex:model version not exposed
@youknowdot
youknowdot force-pushed the fix/sqlite-callback-binding-batch-20261107 branch from 04ea01a to 7a758a4 Compare November 7, 2026 07:07
Rename the typed adapter and force-selection metadata consistently. Clarify that it reuses the ordinary binder, preserves positional-only semantics, and selects the CPython _PyArg_UnpackKeywords conventions without adding a second parser.

Assisted-by: OpenAI Codex:model version not exposed
@codspeed

codspeed Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing youknowdot:fix/sqlite-callback-binding-batch-20261107 (1a834ea) with main (9670d1b)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

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.

1 participant