Skip to content

feat(cbinding,python): dialect-aware exports, lint, security scan, transpile (pygosqlx 0.3.0) - #850

Merged
ajitpratap0 merged 4 commits into
mainfrom
feat/pygosqlx-0.3.0-api
Oct 22, 2026
Merged

ajitpratap0 merged 4 commits into
mainfrom
feat/pygosqlx-0.3.0-api

Conversation

@ajitpratap0

@ajitpratap0 ajitpratap0 commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Summary

Brings the Python bindings up to date as pygosqlx 0.3.0, built on the gosqlx_version double-free fix (#847).

C ABI (pkg/cbinding)

All existing exports are ABI-stable. Their names, signatures and JSON output are unchanged. They now go through shared helpers with an empty dialect, and a new test checks that their output is byte-identical to the *_dialect variants when the dialect is NULL or "". New fields (error_code) are omitempty, so they never appear in the original exports' output.

New exports. Every one returns a caller-owned JSON string that the caller frees with gosqlx_free:

Export Notes
gosqlx_{parse,validate,format,extract_tables,extract_columns,extract_functions,extract_metadata}_dialect(sql, dialect) A NULL or "" dialect gives the same result as the original export. An unknown dialect returns the operation's usual error shape plus "error_code": "E4002" (ErrCodeUnsupportedDialect) and a message listing keywords.DialectNames().
gosqlx_lint(sql, options_json) The gosqlx lint CLI rule set (L001-L010, same configuration as createLinter in cmd/gosqlx/cmd/lint.go, which pkg/mcp also mirrors). Options are {"dialect", "max_line_length"} (default 100, like --max-length). Unknown fields, malformed JSON, non-positive length or trailing data return "error_code": "INVALID_OPTIONS". SQL that does not parse is still linted by the text and token rules.
gosqlx_security_scan(sql) security.Scanner.ScanSQL, the same raw-text scan the MCP security_scan tool uses. Returns findings, per-severity counts, is_clean, has_critical and has_high.
gosqlx_transpile(sql, from, to) Wraps gosqlx.Transpile. Both dialects are required canonical names (missing or unknown ones give E4002).
gosqlx_dialects() {"dialects": keywords.DialectNames()}

Python (python/pygosqlx)

  • parse, validate, validate_detailed, format and extract_* take a keyword-only dialect= argument. With None (the default) they call the original exports, so behaviour does not change.
  • New functions lint(), security_scan(), transpile() and dialects(). New dataclasses LintResult/LintViolation and SecurityScanResult/SecurityFinding. New exceptions UnsupportedDialectError (subclasses both GoSQLXError and ValueError, code="E4002") and TranspileError.
  • Older shared libraries: calls without dialect= keep working. dialect= and the new functions raise GoSQLXError with a "rebuild the shared library" message. Symbols are bound through _bind_optional and checked through _has_func.
  • Version bumped to 0.3.0 in pyproject.toml, setup.py and pygosqlx.__version__. python/README.md API docs updated.

Design decisions (scoped worker, conservative choices)

  1. *_dialect(sql, dialect) variants instead of one options-JSON entry point per operation. A dialect is the only option these operations take, so a second C string keeps the ABI as simple as the original exports, and there is nothing to parse or validate. gosqlx_lint takes options_json because it really has more than one option. This is documented in the package doc comment.
  2. Lint rule set = the CLI's (L001-L010), not gosqlx.Lint (all 30 rules). The brief named gosqlx lint as the source of truth, and the MCP server already mirrors it. The list is a third copy (CLI, MCP, cbinding), and a comment asks maintainers to keep them in sync. I did not extract a shared exported constructor because that would change public API outside this brief.
  3. The security scan uses ScanSQL (raw text) instead of the AST Scan. This matches the MCP tool, and it means injection payloads that do not parse still get scanned.
  4. Transpile accepts canonical dialect names only. The CLI's postgres/mssql aliases are not accepted, which keeps one vocabulary across every dialect argument.
  5. Dialect names are case-sensitive. This matches keywords.IsValidDialect and the CLI.

CLAUDE.md's module dependency line for cbinding is updated to the new imports. build.sh only gets an updated hint line.

Review follow-up (ee7646e)

  • gosqlx_lint options: the trailing-data check now requires EOF, so {} } and {} ] are rejected with INVALID_OPTIONS (dec.More() missed stray closers). Tests added.
  • pygosqlx.transpile: a missing/empty from_dialect/to_dialect now raises UnsupportedDialectError("<arg> is required") up front instead of round-tripping through the library. Conservative choice: kept UnsupportedDialectError (a ValueError) rather than introducing TypeError, so existing behaviour/tests are unchanged.

Test Plan

  • gofmt -l pkg/cbinding is clean. go vet ./pkg/cbinding/ passes. golangci-lint run ./pkg/cbinding/... reports 0 issues.
  • task test:cbinding (CGO, -race) passes. New cbinding_ext_test.go covers:
    • byte-identical output of the original exports vs NULL/"" dialect, for each export and each of NULL, empty, whitespace, valid, multi-statement and invalid inputs
    • every dialect accepting portable SQL
    • SQL Server TOP/brackets parsing only with the dialect
    • unknown dialects (fakesql, MySQL, postgresql) giving E4002 for every export
    • empty, NULL and invalid input, and error positions
    • lint: rule set, clean input, empty input, unparseable input, max_line_length, dialect, and 8 option error cases
    • security scan: detection, clean, empty and NULL input, unparseable payload
    • transpile: MySQL to PostgreSQL, a passthrough pair, and 7 error cases (missing, unknown, NULL dialects; invalid, empty, NULL SQL)
    • gosqlx_dialects
    • a 16-goroutine concurrency test over all new exports
      Every result is freed via gosqlx_free in the helpers.
  • pytest python/tests against a freshly built libgosqlx.dylib: 184 passed. Without the library: 77 passed, the rest skipped. New test_dialect_and_analysis.py covers every new function and dialect= path, older-library graceful errors (monkeypatched _has_func), argument validation, the result types and a threaded run.
  • pkg/cbinding/tests/test_ctypes.py (raw ABI harness): 25 passed.
  • CI: the Python workflow from fix(cbinding): return caller-owned string from gosqlx_version; add Python CI #847 and the cbinding race job

Breaking Changes

None. Existing C exports and their JSON output are unchanged. Existing Python calls behave the same. dialect is a new keyword-only argument with a None default.

Release note

pygosqlx 0.3.0 / C binding:

  • Dialects: parse, validate, validate_detailed, format and extract_* take an optional dialect= argument (for example dialect="sqlserver"). pygosqlx.dialects() lists the supported names. An unknown dialect raises UnsupportedDialectError, which is also a ValueError.
  • New functions:
    • pygosqlx.lint() runs the gosqlx lint rule set (L001-L010, configurable max_line_length).
    • pygosqlx.security_scan() detects SQL injection patterns and returns typed findings with severity counts.
    • pygosqlx.transpile(sql, from_dialect, to_dialect) converts SQL between dialects.
  • libgosqlx exports: new gosqlx_*_dialect, gosqlx_lint, gosqlx_security_scan, gosqlx_transpile and gosqlx_dialects. Each returns a JSON string that the caller frees with gosqlx_free. The existing exports are unchanged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

…anspile; pygosqlx 0.3.0

C ABI (pkg/cbinding): every existing export keeps its name, signature and
JSON output. New exports, all JSON-out and freed with gosqlx_free:

- gosqlx_{parse,validate,format,extract_tables,extract_columns,
  extract_functions,extract_metadata}_dialect(sql, dialect): NULL/"" behaves
  exactly like the original export; an unknown dialect returns the normal
  error shape with error_code "E4002" and the list of valid names.
- gosqlx_lint(sql, options_json): the `gosqlx lint` CLI rule set
  (L001-L010), options {"dialect", "max_line_length"}; unknown or invalid
  options return error_code "INVALID_OPTIONS".
- gosqlx_security_scan(sql): pkg/sql/security ScanSQL with severity counts.
- gosqlx_transpile(sql, from, to): gosqlx.Transpile.
- gosqlx_dialects(): keywords.DialectNames().

The original exports now route through the same helpers with an empty
dialect, and a test pins their output byte-for-byte against the dialect
variants.

Python (pygosqlx 0.3.0): keyword-only dialect= on parse, validate,
validate_detailed, format and extract_*; new lint(), security_scan(),
transpile() and dialects() with LintResult/LintViolation and
SecurityScanResult/SecurityFinding dataclasses; UnsupportedDialectError
(GoSQLXError + ValueError) and TranspileError. An older shared library
keeps working without dialect= and raises GoSQLXError for the new calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@vercel

vercel Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
gosqlx-website Ignored Ignored Preview Oct 22, 2026 7:37am UTC

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 32s —— View job


Review of PR #850 (pygosqlx 0.3.0 / cbinding dialect exports)

I read pkg/cbinding/cbinding_ext.go in full. I did not read the Python code, the tests or the cbinding.go refactor in detail, and I did not run any tests.

Overall: The design is clean. The *For(sql, dialect) helpers are shared by the original and the new exports, which is a sound way to keep the output byte-identical. omitempty on error_code and the "older shared library" fallback in Python are both sensible. security.Finding has JSON tags, so its serialization is stable.

Issues

  1. Trailing-data check in parseLintOptions is incomplete (cbinding_ext.go:408).

    • dec.More() returns false when the next non-space byte is ] or }. Input like {"dialect":"mysql"} } or {} ] therefore passes without error.
    • Proper check: if _, err := dec.Token(); err != io.EOF { return ..., errors.New("trailing data ...") }. Decoding a second value into json.RawMessage and requiring io.EOF also works.
    • Add a test case for {} }.
  2. gosqlx_transpile with an empty dialect returns E4002 but no "unknown dialect" wording. This is intentional and documented, but it differs from the *_dialect exports, where an empty dialect means "default". The Python wrapper should reject None or "" before the call so users get a clear TypeError or ValueError instead of UnsupportedDialectError. Please check that it does.

  3. validateFor and formatFor re-implement logic from gosqlx.Validate and gosqlx.Format (cbinding_ext.go:125-185).

    • The option mapping (LineWidth: opts.SingleLineLimit under //nolint:staticcheck, NewlinePerClause: opts.IndentSize > 0) is copied from gosqlx.Format. If that mapping changes upstream, the dialect and non-dialect paths will drift apart silently.
    • The parity test only covers the NULL/"" dialect path, which goes through gosqlx.Format. It does not compare against a non-empty dialect.
    • Consider adding a test that formats portable SQL with each dialect and compares the result to the no-dialect output where they should match, or a gosqlx.FormatWithDialect helper upstream.
  4. Lint rule set is duplicated a third time (CLI, MCP, cbinding), as you noted. A comment alone won't catch drift. A small test that compares the rule IDs from newLinter against the CLI's createLinter is cheap, if the CLI package is importable. If it isn't, exporting a constructor in a follow-up PR would fix this properly.

  5. Minor: unsupportedDialectError is detected with errors.As, but the errors reach errorCodeFor unwrapped (parse and validate wrap only other errors). This works today. If someone later wraps with %w, it still works, so no change is needed.

Security and performance

  • No memory-safety concerns in the new exports. Every export returns a toJSON string that the caller frees with gosqlx_free, and C.GoString(NULL) returns "", which the NULL-input tests cover.
  • ScanSQL runs on raw input with no length cap. This is the same as the MCP tool, but a hosted Python service might want a size guard on the caller side. Consider documenting it.
  • newLinter builds a new rule set on every call. That is cheap and race-free, so I would not pool it.

Tests

Coverage is strong: parity, unknown dialects on every export, option-error cases, a concurrency test and the older-library paths in Python. The gap is the trailing-data case from item 1 and the dialect-versus-no-dialect format comparison from item 3.

Nothing here blocks the merge. Item 1 is the only real bug, and it is small. I made no code changes.
· branch feat/pygosqlx-0.3.0-api

@github-actions github-actions Bot added documentation Improvements or additions to documentation python labels Oct 21, 2026
Standalone staticcheck (SA1019) ignores //nolint, so spell out the default
line width instead of reading the deprecated field, and pin the generic
dialect format output to gosqlx_format's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 15s —— View job


Review of PR #850 (pygosqlx 0.3.0 / cbinding dialect exports)

I checked parseLintOptions, transpileFor and the Python transpile/lint wrappers. I did not run any tests or change any code.

Overall: The design is clean. The shared *For(sql, dialect) helpers keep the original and new exports byte-identical. omitempty on error_code and the older-library fallback in Python are sensible.

Issues

  1. Trailing-data check is incomplete (pkg/cbinding/cbinding_ext.go:412). This is the only real bug.

    • dec.More() returns false when the next non-space byte is ] or }. Input like {} } or {"dialect":"mysql"} ] therefore passes without error.
    • Fix: after decoding, require dec.Token() to return io.EOF.
    • Add a test case for {} }.
  2. Python transpile accepts None (python/pygosqlx/core.py:768).

    • _check_dialect_arg allows None, so transpile(sql, None, "postgresql") reaches the C library and fails with UnsupportedDialectError.
    • Both dialects are required here. Consider rejecting None or "" up front with a clear TypeError or ValueError.
    • If you keep the current behaviour, document it. UnsupportedDialectError is already a ValueError, so callers catching ValueError still work.
  3. validateFor and formatFor copy logic from gosqlx.Validate and gosqlx.Format.

    • The option mapping (SingleLineLimit to LineWidth, IndentSize > 0 to NewlinePerClause) can drift silently from upstream.
    • The parity test only covers the NULL/"" dialect path. A test that formats portable SQL with each dialect and compares it to the no-dialect output would cover the dialect path.
  4. The lint rule list now exists in three places (CLI, MCP, cbinding).

    • A comment will not catch drift. A small test comparing rule IDs against the CLI's createLinter would, if that package is importable.
    • Otherwise, export a shared constructor in a follow-up PR.

Security and performance

  • I saw no memory-safety problems. Every export returns a caller-owned string freed with gosqlx_free, and NULL inputs are tested.
  • ScanSQL has no input length cap, same as the MCP tool. A hosted Python service may want a size guard on the caller side, so consider documenting that.
  • newLinter builds its rule set on every call. That is cheap and race-free, so I would not pool it.

Tests

Coverage is strong: parity, unknown dialects on every export, option error cases, concurrency, and the older-library paths. The gaps are the {} } case (item 1) and the dialect-versus-no-dialect format comparison (item 3).

Nothing here blocks the merge, but I'd fix item 1 first.
· branch feat/pygosqlx-0.3.0-api

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 16s —— View job


Review of PR #850 (pygosqlx 0.3.0 / cbinding dialect exports)

I re-checked the two code paths the earlier reviews flagged (parseLintOptions and Python transpile). I did not run tests or change any code.

Overall: The design is sound. The shared *For(sql, dialect) helpers keep the original and _dialect exports consistent, error_code is omitempty, and the older-library fallback is sensible.

Confirmed issues

  1. Trailing-data check is incomplete (pkg/cbinding/cbinding_ext.go:412). This is the only real bug.

    • dec.More() returns false when the next non-space byte is ] or }. Input like {} } or {"dialect":"mysql"} ] therefore passes without INVALID_OPTIONS.
    • Fix: replace the More() check with if _, err := dec.Token(); err != io.EOF { return opts, errors.New("invalid options_json: trailing data after the options object") }, and add {} } and {} ] to the option-error test cases.
  2. Python transpile accepts None (python/pygosqlx/core.py:768-777).

    • _check_dialect_arg allows None, and (from_dialect or "") turns None into "". The call then reaches the library and fails with UnsupportedDialectError.
    • This is safe, because that error is also a ValueError. Still, both dialects are required here, so a clear up-front ValueError("from_dialect is required") would be friendlier. Alternatively, mention in the docstring that None and "" raise UnsupportedDialectError, which it already partly says.

Suggestions (non-blocking)

  1. validateFor and formatFor copy the option mapping from gosqlx.Validate and gosqlx.Format. The parity test only covers the NULL/"" dialect path. A test that formats portable SQL with a few dialects and compares the result to the no-dialect output would catch drift.
  2. The lint rule list now exists in three places (CLI, MCP, cbinding). A comment will not catch drift. If the CLI package can't be imported, a follow-up PR that exports a shared constructor would fix it. Otherwise a rule-ID comparison test would do.
  3. ScanSQL has no input length cap, same as the MCP tool. A hosted Python service may want a size guard on the caller side, so consider documenting it.

Security and performance

  • I saw no memory-safety concerns. Every new export returns a caller-owned string freed with gosqlx_free, and NULL inputs are covered by tests.
  • newLinter builds its rule set on every call. That is cheap and race-free, so I would not pool it.

Tests

Coverage is strong: parity, unknown dialects on every export, option-error cases, concurrency and the older-library paths. The gaps are the {} } case (item 1) and the dialect-path format comparison (item 3).

Nothing here blocks the merge, but I'd fix item 1 first.
· branch feat/pygosqlx-0.3.0-api

…anspile dialects

dec.More() returns false before a stray ']' or '}', so input like `{} }`
slipped past the trailing-data check. Require dec.Token() to hit io.EOF.
pygosqlx.transpile now rejects a missing from_dialect/to_dialect up front
with UnsupportedDialectError("<arg> is required").

Addresses claude-review findings on #850.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 13s —— View job


Review of PR #850 (pygosqlx 0.3.0 / cbinding dialect exports)

I checked the two items the earlier reviews flagged against the current branch head (ee7646e). I did not run tests or change any code.

Both earlier blocking items are already fixed in ee7646e:

  1. Trailing-data check in parseLintOptions (pkg/cbinding/cbinding_ext.go:413-415). It now requires dec.Token() to return io.EOF, with a comment explaining why dec.More() was not enough. The option-error tests cover {} }, {"dialect":"mysql"} ], {} {} and {} 1 (cbinding_ext_test.go:305-308).
  2. Python transpile with None or "" (python/pygosqlx/core.py:768-777). It now rejects a missing dialect up front with UnsupportedDialectError("<arg> is required") before the library call. That error is also a ValueError, and the docstring says so.

Remaining suggestions (non-blocking):

  • validateFor and formatFor copy the option mapping from gosqlx.Validate and gosqlx.Format. The parity test only covers the NULL/"" dialect path. A test that formats portable SQL with a few dialects and compares it to the no-dialect output would catch drift.
  • The lint rule list now exists in three places (CLI, MCP, cbinding), and a comment will not catch drift. A rule-ID comparison test would, if the CLI package is importable. Otherwise, export a shared constructor in a follow-up PR.
  • ScanSQL has no input length cap, same as the MCP tool. A hosted Python service may want a size guard on the caller side, so consider documenting it.

Security and performance: I saw no memory-safety concerns. Every new export returns a caller-owned string freed with gosqlx_free, and NULL inputs are tested. newLinter builds its rule set on every call. That is cheap and race-free, so I would not pool it.

Tests: Coverage is strong: parity, unknown dialects on every export, option-error cases, concurrency, and the older-library paths in Python. The one gap is the dialect-path format comparison above.

Nothing blocks the merge. The remaining checks are the CI jobs listed as unchecked in the test plan.
· branch feat/pygosqlx-0.3.0-api

@ajitpratap0
ajitpratap0 merged commit a08d7b7 into main Oct 22, 2026
34 of 35 checks passed
@ajitpratap0
ajitpratap0 deleted the feat/pygosqlx-0.3.0-api branch October 22, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant