Skip to content

Expose sql_mode connection parameter - #142

Merged
kesmit13 merged 1 commit into
mainfrom
sql-mode-param
Oct 14, 2026
Merged

kesmit13 merged 1 commit into
mainfrom
sql-mode-param

Conversation

@kesmit13

@kesmit13 kesmit13 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add sql_mode to singlestoredb.connect() and register a matching sql_mode config option (SINGLESTOREDB_SQL_MODE). It can also be set in a connection URL (...?sql_mode=ANSI_QUOTES).
  • MySQL protocol: the low-level driver already ran SET sql_mode=%s on connect; this change exposes that through the public API.
  • HTTP Data API: the Data API is stateless and has no session-variable field. This client also rejects multi-statement queries, so sql_mode can't be applied server-side. The HTTP connection accepts the parameter for API parity and emits a RuntimeWarning saying it is ignored.
  • vectorstore.connect() is unchanged: it forwards to the external VectorDB package, which may not accept sql_mode.

Test plan

  • New test_sql_mode / test_sql_mode_param in test_connection.py pass over MySQL (checks @@sql_mode)
  • Same tests pass with USE_DATA_API=1 (checks the warning)
  • test_connection.py + test_config.py pass over MySQL (84 passed)
  • pre-commit clean

🤖 Generated with Claude Code


Note

Low Risk
Additive optional connection setting with documented HTTP no-op; MySQL path only changes session SQL semantics when callers opt in.

Overview
Adds a public sql_mode connection option so callers can set the session sql_mode when opening a connection via singlestoredb.connect(), the SINGLESTOREDB_SQL_MODE env var, or a URL query parameter (e.g. ?sql_mode=ANSI_QUOTES).

On MySQL protocol connections, the value flows through existing build_params / config wiring into the driver (tests assert @@sql_mode reflects the requested mode). On HTTP Data API connections, the parameter is accepted for API parity but is not applied; the HTTP client emits a RuntimeWarning that it is ignored.

Tests cover live session behavior (MySQL) vs warning (HTTP) and URL/keyword param parsing.

Reviewed by Cursor Bugbot for commit ec061c3. Bugbot is set up for automated code reviews on this repo. Configure here.

Add sql_mode to singlestoredb.connect() and register it as a config
option (SINGLESTOREDB_SQL_MODE). The MySQL driver already applied
sql_mode on connect; it is now reachable via kwargs, URLs, and env.

The HTTP Data API is stateless and has no session-variable support, so
the HTTP connection accepts sql_mode for API parity but emits a
RuntimeWarning that it is ignored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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.

🟢 Approval recommended

The implementation consistently exposes, routes, documents, and tests the new connection option.

0 open findings

What changed in this PR

Exposes sql_mode through public connection APIs while preserving protocol-specific behavior.

Changes:

  • Adds sql_mode configuration and URL support.
  • Applies it to MySQL sessions and warns when HTTP ignores it.
  • Adds MySQL and HTTP coverage.
File Description
singlestoredb/​config.py Registers the sql_mode option and environment variable.
singlestoredb/​connection.py Adds the public connection parameter and documentation.
singlestoredb/​http/​connection.py Accepts but warns about unsupported sql_mode.
singlestoredb/​tests/​test_connection.py Tests parameter parsing and protocol behavior.

🧠 Review effort: Balanced


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

@kesmit13
kesmit13 marked this pull request as ready for review October 13, 2026 18:01
@kesmit13
kesmit13 merged commit 9b8ec6e into main Oct 14, 2026
15 checks passed
@kesmit13
kesmit13 deleted the sql-mode-param branch October 14, 2026 16:57

This branch was successfully deployed

1 active deployment
Base — ec061c37 Deployed Oct 13, 2026 by kesmit13 via test-coverage #545
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.

2 participants