Repository navigation
feat(snowflake): adapt to derived DDL with typed options and comment support - #3
Merged
Merged
Conversation
…al forms Snowflake has no declarative RANGE/LIST/HASH table partitioning, so the previous SnowflakePartitionClause (RANGE/LIST + VALUES LESS THAN/IN) produced invalid Snowflake SQL. Correct the model: - Capability bits are honest: supports_table_partitioning / supports_partitioned_table_creation / range / list / hash now return False. - The generic PartitionClause is rejected (UnsupportedFeatureError); standard tables use CLUSTER BY. - Add SnowflakeClusterByClause + format_cluster_by_clause (wired into SnowflakeTableModifierMixin/protocol) for standard-table clustering keys. - Add SnowflakeExternalPartitionClause + format_external_partition_clause for the external-table PARTITION BY (cols) form (no method, no VALUES). - Update tests to the corrected semantics. (cherry picked from commit 9dbc506) (cherry picked from commit f409122)
Introduce SnowflakeCreateTableOptions (deriving the generic CreateTableOptions) carrying the Snowflake-only TRANSIENT header modifier. The generic format_create_table_options no longer renders TRANSIENT; Snowflake overrides it to add TRANSIENT and accepts both the generic and Snowflake options. (cherry picked from commit bbd0fac) (cherry picked from commit 8046777)
Snowflake has no SQL-standard GENERATED ... AS IDENTITY column syntax; render IDENTITY(start, step) (synonym for AUTOINCREMENT). Column collation must be a quoted specification (COLLATE 'en-ci'), so override format_column_attribute. (cherry picked from commit 55cc826) (cherry picked from commit 7366f44)
… inline Snowflake natively supports both the inline COMMENT = 'text' table option and the standalone COMMENT ON statement; declare supports_table_comment() / supports_column_comment() as True so the capability bits match the actual rendering behavior. format_table_comment renders the Snowflake table-level form (COMMENT = 'text'), the column level keeps the bare form. (cherry picked from commit 304fc44) (cherry picked from commit 6b9f701)
…n hierarchy
Snowflake declares supports_materialized_view() = True, but its formatter only
read Snowflake-specific fields (as_query / column_list / name), so passing a
core expression raised:
AttributeError: 'CreateMaterializedViewExpression' object has no
attribute 'as_query'
A backend that advertises MV support yet cannot use the generic MV API is
exactly the defect the new backend contract catches
(python-activerecord-testsuite feature/basic/ddl/test_materialized_view_contract.py).
Make SnowflakeCreateMaterializedViewExpression inherit the core
CreateMaterializedViewExpression and normalise the field names (view_name /
query / column_aliases), while keeping the existing Snowflake spellings
name= / as_query= / column_list= as aliases, and or_replace / if_not_exists /
cluster_by / comment as extensions. query now accepts either a QueryExpression
or a raw SQL string.
Also fixes two silent discards:
- TABLESPACE / storage_options do not exist in Snowflake and now raise
UnsupportedFeatureError instead of being quietly ignored
- or_replace together with if_not_exists is rejected up front, since Snowflake
forbids the combination, rather than emitting SQL the server will reject
Snowflake has no WITH [NO] DATA (a view is always created empty and filled in
the background), so the inherited with_data is not rendered, and a test pins
that behaviour.
Tests: the 15 existing MV cases stay green; 11 new cases cover generic
expression interop, field aliases, the mutual-exclusion check, the
TABLESPACE/storage_options rejection, and with_data not being rendered.
(cherry picked from commit 26f409f)
(cherry picked from commit cbf9fd9)
… 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 33ce3d1, 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.
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 Snowflake backend to the derived-DDL model in
rhosocial/python-activerecord#121.
Comment handling is the main addition: standalone
COMMENT ONis supported, commentcapabilities are advertised deliberately, and a table comment renders inline the way Snowflake
expects rather than as a separate statement. Column options are typed —
SnowflakeCreateTableOptionsrendersTRANSIENT, andIDENTITY(start, step)and quotedcolumn collation are supported and pinned by tests.
Two capability corrections prevent the backend from claiming or emitting DDL it cannot
honour: capabilities for
ALTER SCHEMAoperations that cannot be rendered are no longeradvertised, and partition semantics are corrected to the clustering and external forms
Snowflake actually has.
SnowflakeDatabaseMixinis wired into the dialect, materialized-viewexpressions are wired into the core expression hierarchy, and an index is dropped by bare
name without
ON <table>.Also included: type DDL support,
CommentSupportclassification, typed database andinsert-overwrite expressions, and coverage for
DDLSourcecollection, table inheritance andtablespace.
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. Unrenderable
ALTER SCHEMAcapabilities are no longeradvertised, so code that relied on them will now fail rather than emit SQL the server rejects.
Impact on Backend Developers (Custom Implementations):
dialect_optionsis removed from data-type value objects and from partition clauses.Partition semantics are corrected to clustering/external forms, so custom partition
implementations keyed to the previous behaviour need updating.
SnowflakeCreateTableOptionsis the new entry point for typed create-table options andreplaces ad-hoc
TRANSIENThandling; the transient tests were migrated onto it.SnowflakeDatabaseMixinis now wired into the dialect, so tests constructSnowflakeDialectdirectly.dialect-local.
.gitattributesfile is added to enforce LF line endings, which also resolves thecross-platform line-ending noise this repository had.
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, firebird, bigquery.
Testing
Local full-suite execution was skipped at the user's instruction. Verification relied on
remote CI on this exact tip
9e7e282, 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
This repository's history was flattened before the merge sequence, and
chore: add .gitattributes to enforce LF line endingsis included here. That file matters more than itlooks: with the derived-DDL diff spanning dozens of files, mixed line endings would otherwise
show up as unrelated whitespace changes in review and again in the diff-coverage calculation.