Repository navigation
feat(sqlserver): adapt to derived DDL with typed options and strict DDL rejection - #4
Merged
Merged
Conversation
Introduce the SQL Server-specific column definition (and options) derived from the generic ColumnDefinition / ColumnOptions, carrying typed SPARSE / ROWGUIDCOL attributes with no dialect_options bag. The SQL Server format_column_definition accepts both the generic and SQL Server column definitions and renders the SQL Server-only attributes. Export the new classes and cover them with tests.
Introduce SQLServerCreateTableOptions (deriving the generic CreateTableOptions) carrying the SQL Server-only MEMORY_OPTIMIZED / DURABILITY options. The SQL Server CREATE TABLE formatter reads these typed fields instead of the dialect_options bag. Graph table kind / edge constraints remain for a later CreateTableExpression-specific pass.
…leOptions The SQL Server formatter now reads typed memory_optimized/durability instead of the dialect_options bag.
…ex fields Move hash_index/bucket_count off the dialect_options bag onto a typed SQLServerIndexDefinition subclass read by format_index_definition.
…ssion Add SQLServerAlterColumn (data_type/collate/not_null/sparse/masked_function/ drop_masked) and SQLServerCreateTableExpression (graph_table_kind/ edge_constraints), replacing the dialect_options bag reads.
Add SQLServerMergeExpression (output/output_action/holdlock) and SQLServerSelectIntoExpression (select_into_table); give SQLServerPartitionByRangeClause a typed partition_scheme field.
…roundtrip The subclass used *args/**kwargs, so introspection-based get_params() dropped 'select' and serialization round-trip failed to reconstruct the query. Mirror QueryExpression's explicit parameter list so get_params() reflects every constructor argument.
…e-table subclass The generic CreateTableExpression no longer carries on_commit_delete / external_file; stop threading them through the SQL Server subclass.
Core DataType no longer carries a dialect_options bag; backend type subclasses stop forwarding it. Update the type-protocol tests.
…heck_option The view formatter rendered WITH CHECK OPTION unconditionally, leaving the capability bit inert.
It called a non-existent super().format_cte and had no callers; CTE formatting goes through format_cte_expression now.
Remove identity/identity_start/identity_increment/identity_clause from XxxColumnDefinition and XxxColumnOptions to match core's column-attribute migration; XxxColumnDefinition now forwards attributes.
Render the dialect-free column attributes channel in each backend's format_column_definition (it was silently dropped), and give backends whose identity syntax differs from the SQL-standard form a format_identity_clause override (MySQL/MariaDB AUTO_INCREMENT, SQL Server IDENTITY(seed, inc)). Oracle gates column COLLATE to 12.2+; MariaDB drops the duplicate dialect formatter so the attribute-aware mixin is authoritative.
Core added the ColumnAttributeSupport protocol and renamed the deriver plan entry to creation_plan(); update protocol conformance and derived-DDL tests.
…lently dropping SQL Server has no inline comment mechanism (its native mechanism is sp_addextendedproperty, not implemented); a declared comment raises UnsupportedFeatureError instead of being silently dropped.
- CREATE INDEX IF NOT EXISTS / TABLESPACE now raise - CREATE SEQUENCE IF NOT EXISTS / OWNED BY now raise - unsupported inline-index options now raise - multi-action ALTER TABLE emits one statement per action - CREATE TABLE IF NOT EXISTS renders the native IF OBJECT_ID guard - advertise the gated ALTER action capabilities
Verify SQL Server DDLSource declarations, backend types, column options, and dialect rendering.
Expression classes moved under backend.expression/ and several were renamed during the derived-DDL refactor, leaving relative-import depths and forward references in TYPE_CHECKING blocks pointing at modules and classes that no longer exist. Type checkers and any runtime evaluation of those blocks fail to resolve them. Verified statically: every TYPE_CHECKING import in the touched files now resolves to an existing module and an existing top-level symbol, and no string annotation references a name absent from the workspace.
The suite applies markers that were never declared in [tool.pytest.ini_options] markers. pytest only warns for an unknown marker, so the warning scrolls past unnoticed and a dropped or misspelt registration is silent. Registers the ones in use, with descriptions that say what they do: collection grouping, not ordering. `feature` labels feature-area tests and `benchmark_relation` marks relation benchmarks; neither affects scheduling, so neither substitutes for the xdist_group marker that actually serialises a suite. usefixtures needs no entry and is not added: pytest provides it, so it was a false positive in the isolation audit rather than a missing registration.
… revisions The installed hook exec'd scripts/preflight.sh unconditionally. That hook is a local, untracked artifact, so it survives a checkout of any commit that predates the script, and on such a commit exec failed with a bare "No such file or directory" — blocking the commit with nothing to act on. In this repository scripts/preflight.sh first appears at 8e12401, so every earlier commit is affected. The failure is also silent in a dangerous way: it swallowed a `git commit --amend`, so a commit-message rewrite reported as applied when it had not been, and had to be redone. The hook now checks the script is executable and exits 0 otherwise, so a revision without the pre-flight simply skips it. Normal operation is unchanged. Also honours core.hooksPath. git ignores .git/hooks entirely once that is set, so the hook was being written to a location that would never run while this script still reported success. No repository sets it today, so that part is latent.
Both dependencies were pinned to feature/activerecord-derived-ddl, a branch that exists only to carry the derived-DDL work. Now that the core change has landed in release/v1.0.0.dev30 and the matching testsuite change in release/v1.0.0.dev17, pinning to the feature branch means CI keeps resolving a branch that will be deleted once the backend PRs merge. This is a prerequisite for merging: leaving the old ref in place means the first run after that branch disappears fails to resolve, not the code.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/v1.0.0.dev3 #4 +/- ##
=======================================================
- Coverage 73.98% 73.36% -0.63%
=======================================================
Files 124 133 +9
Lines 6715 7332 +617
Branches 1019 1171 +152
=======================================================
+ Hits 4968 5379 +411
- Misses 1432 1548 +116
- Partials 315 405 +90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adapts the SQL Server backend to the derived-DDL model in
rhosocial/python-activerecord#121, with the emphasis on refusing to emit SQL the server
would reject.
Previously the backend accepted a declared option and then dropped it, so a schema the
application asked for could silently differ from what was created. That is now an error:
Alongside that, options become typed rather than free-form:
SQLServerColumnDefinitionwith typed column options and backend identity syntax.SQLServerCreateTableOptions, with the memory-optimized tests migrated onto it.SQLServerIndexDefinitionwith typed hash-index fields.ALTER COLUMNoptions, a graph table expression, and typedMERGE/SELECT INTO/ partition scheme, plus an explicit signature on
SQLServerSelectIntoExpressionforround-tripping.
CREATE VIEW ... WITH CHECK OPTIONis gated onsupports_view_check_option, type DDLsupport is added, and
CommentSupportis classified for SQL Server conformance.SQLServerDatabaseMixinis wired into the dialect, which retires the need to construct thedialect directly in tests. Dead code goes: a vestigial
format_cte, the reserveddialect_optionsbag on data-type value objects and partition clauses, and inheritedFirebird clause parameters that no longer belong on the create-table subclass.
Type of change
feat: A new featuredocs: Documentation only changes (if related to the new feature)test: Adding missing tests or correcting existing tests (for the new feature)chore: Changes to the build process or auxiliary tools and libraries (if related to the new feature)Breaking Change
No commit carries a
!marker. The public model and query APIs are unchanged.Impact on End-Users:
No backward-incompatible API changes. One deliberate behaviour change does affect
applications: a declared table or column comment, or an unsupported table declaration, now
raises instead of being silently ignored. Code that relied on the option being dropped will
start failing — which is the point, since the created schema was not what was asked for.
Impact on Backend Developers (Custom Implementations):
dialect_optionsbag is removed from data-type value objects and partitionclauses, and inherited Firebird clause parameters are gone from the create-table subclass.
Anything reading or writing them must move to a declared option or capability bit.
nodes from core.
SQLServerDatabaseMixinis now wired into the dialect, so tests constructSQLServerDialectdirectly instead of assembling it from mixins.format_cteis removed as dead code.split were repaired so type checkers resolve them.
Related Repositories/Packages
The core half is rhosocial/python-activerecord#121 (merged into
release/v1.0.0.dev30) andthe shared contract half is rhosocial/python-activerecord-testsuite#21 (merged into
release/v1.0.0.dev17).This PR also repoints this repository's workflow at those release branches. Both dependencies
were pinned to
feature/activerecord-derived-ddl, a branch that only exists to carry thederived-DDL work; leaving the old ref in place would have made the first run after that
branch is deleted fail to resolve.
Merged or in flight, and still to merge in the same sequence:
91b89e0f7dbc394b2ca8aff98cf00870faf03c6d8c0c573289e7e2824d2e9f0Testing
Local full-suite execution was skipped at the user's instruction. Verification relied on
remote CI, which ran on this exact tip
ff98cf0and covers SQL Server 2017, 2019, 2022 and2025 on Python 3.9 through 3.14, including free-threaded builds on 2019 and 2022. All nine
jobs pass.
That run also covers the workflow change in this PR: it is the first execution against
release/v1.0.0.dev30andrelease/v1.0.0.dev17rather than the feature branches, so thenew dependency refs are verified to resolve.
Checklist
changelog.d/(to be added after this PR isopened, named by the actual PR number)
Additional Notes
Two commits are housekeeping rather than adapter work and are called out so a reviewer is
not surprised:
ci: install core and testsuite from release branches— the workflow repoint describedabove, a prerequisite for merging.
fix(scripts): stop the pre-commit hook from blocking commits on older revisions,fix(backend): repair unresolvable TYPE_CHECKING importsandchore(test): register the pytest markers the suite already uses— mechanical repairs required for the hook, typechecking and lint to run at all after the namespace split.
chore(sqlserver): align pull request templateanddocs(sqlserver): drop empty dialect_options from examplesare documentation-only and affect no runtime behaviour.