Repository navigation
feat(firebird): adapt to derived DDL with complete domain and column support - #5
Merged
Merged
Conversation
Introduce the Firebird-specific column definition (and options) derived from the generic ColumnDefinition / ColumnOptions, carrying typed COMPUTED BY / CHARACTER SET / COLLATE attributes with no dialect_options bag. The Firebird format_column_definition accepts both the generic and Firebird column definitions and renders the Firebird-only attributes. Export the new classes and cover them with tests.
…e/external_file Carry the Firebird-only CREATE TABLE clauses on a dedicated subclass deriving the generic statement; the Firebird formatter already reads them via getattr and thus accepts both the generic and the specific instance.
Core DataType no longer carries a dialect_options bag; backend type subclasses stop forwarding it. Update the type-protocol tests.
…capabilities Firebird's DROP VIEW has no CASCADE clause and computed indexes use COMPUTED BY syntax not emitted by the generic renderer. Declare both capabilities False (fail-fast) and cover them with tests.
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.
… before TableMixin format_create_table_statement / format_column_definition raise UnsupportedFeatureError for declared comments instead of silently dropping them (Firebird annotates through standalone COMMENT ON). FirebirdCommentMixin is composed before TableMixin so supports_comment_on/format_comment_statement are not shadowed by the generic ones; format_comment_statement accepts any comment expression.
…ions - render a generic GeneratedColumnExpression as COMPUTED BY (<expr>) - raise on declared DEFERRABLE / FOREIGN KEY MATCH (Firebird cannot render them)
Two stale references left by the derived-DDL refactor: - mixins/transaction.py imported from backend.transaction; the expression module it needs now lives at backend.expression.transaction. - mixins/window.py imported WindowFunctionCall, but the parameter annotation on format_window_function_call still named WindowFunctionCallExpression, which no longer exists. Type checkers fail to resolve the forward reference. Verified statically: every TYPE_CHECKING import in the touched files 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 818cc9b, 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.dev2 #5 +/- ##
=======================================================
+ Coverage 71.67% 72.19% +0.51%
=======================================================
Files 92 94 +2
Lines 4908 5323 +415
Branches 736 834 +98
=======================================================
+ Hits 3518 3843 +325
- Misses 1184 1230 +46
- Partials 206 250 +44
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 Firebird backend to the derived-DDL model in
rhosocial/python-activerecord#121.
The largest item is domain DDL, which was broken rather than merely incomplete: domain DDL
rendering is restored and domain support is completed, including
ALTER DOMAINconstraintrendering and the DDL type protocol alias. Generated columns and constraint options are no
longer silently dropped, and declared comments are no longer dropped either — the comment
mixin is composed before
TableMixinso the declaration actually reaches the renderer.Capability corrections matter here more than elsewhere, because Firebird's dialect is built
by mixin composition and the wrong bit produces DDL the server rejects:
Adds
FirebirdCreateTableExpressionforon_commit_delete/external_fileandFirebirdColumnDefinitionwith typed column options, and drops an index by bare name withoutON <table>. Comments use the core typed clause nodes, and rendering moves to direct DDLexpressions.
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. Declared comments, generated column definitions and
constraint options now raise or render instead of being discarded, and cascade-view and
functional-index support is reported accurately. Applications that were relying on the old
silently-dropping behaviour will now fail — the created schema was not what was requested.
Impact on Backend Developers (Custom Implementations):
TableMixin. A custom composition that reorders these silently loses comments again.unique-index support. A conformance suite asserting the old values will fail.
dialect_optionsis removed from data-type value objects; column classes drop the legacyidentity fields.
tests keyed to the old shapes need updating.
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, a prerequisite for
merging: the dependencies were pinned to
feature/activerecord-derived-ddl, a branch thatonly exists to carry this work.
Merged or in flight: mysql #56, postgres #58, mariadb #5, sqlserver #4, oracle. Still to
merge: clickhouse, snowflake, bigquery.
Testing
Local full-suite execution was skipped at the user's instruction. Verification relied on
remote CI on this exact tip
0c57328, which is also the first execution against the releasebranches rather than the feature branch, so the new dependency refs are verified to resolve.
Checklist
changelog.d/(to be added after this PR isopened, named by the actual PR number)
Additional Notes
fix(firebird): stop silently dropping comments; compose comment mixin before TableMixinisthe one commit here with a non-obvious cause. Because the dialect is assembled from mixins,
the comment declaration was reaching a mixin that had already had its chance to render, and
the comment was dropped without an error. The fix is ordering, not a new code path — which is
why it is paired with a conformance classification rather than left implicit.