Repository navigation
feat(sqlserver)!: add introspection families and rename sync classes after the family - #10
Merged
Merged
Conversation
…on stats, and projects its sequences sys.dm_db_partition_stats holds one row per partition per index, which is where SQL Server answers what a table occupies. The table listing reads it as three scalar subqueries -- the total, the table's own partitions (index_id 0 for a heap, 1 for a clustered index) and the nonclustered ones' -- each multiplied by the engine's fixed 8 KiB page. A scalar subquery per slot rather than a GROUP BY join, because a view has no partition rows and must stay on the listing with its storage slots None instead of vanishing from it; a view's storage is not a question the engine answers for it, which is a different answer from zero bytes. supports_table_storage_introspection now declares the family, so a caller gating on the switch learns SQL Server answers it -- before, the slots were permanently None while the engine was able to answer. row_count is left None on purpose: an exact count is a scan and an estimate is a table SQL Server keeps no estimate in. SQLServerSequenceInfo keeps its full semantics -- exact Decimals, the caching flag -- and gains to_core(), the projection onto core's common minimum SequenceInfo. A non-integral value (a sequence declared on decimal(10,2)) projects to None there and stays complete on the own class; the minimum does not widen to hold one engine's non-integral sequence (rules section 5). Version numbers in pyproject.toml are untouched on this branch. CI: the core pin in test.yml points at feature/introspection-enhancement for this cross-repository round, because the changes here consume core APIs that exist only on that branch; move the pin back to the release line once core merges.
… never needed The `WindowsSelectorEventLoopPolicy` switch in the connection pool conftest was copied from the postgres suite, where it was load-bearing: psycopg waits on the libpq socket through loop.add_reader/add_writer, which Windows' default ProactorEventLoop does not implement. This backend's async path has no such dependency -- the driver talks over asyncio streams (or, where there is no async path at all, a thread pool), which the Proactor serves natively -- so the switch bought nothing and cost a deprecated API (announced for removal in Python 3.16) plus a process-wide side effect a selector loop cannot serve asyncio subprocess or pipe I/O under. The switch is removed, with the reason recorded where it stood so the copy is not reintroduced. Should a future async driver need a selector loop, the postgres suite's pytest_asyncio_loop_factories hook is the pattern to follow: it scopes the choice to the tests' loops, not to the process. Verified locally: the conftest imports cleanly and the suite collects (pytest-asyncio 1.4.1, no async failures attributable to the loop).
… family, not after the sync Follows the core rename (53842f99) that took the `Sync` prefix off the introspection family's sync side, so the pair's shape now matches the backend family's: `SQLServerIntrospector` and `AsyncSQLServerIntrospector`, one plain name and one marked variant, the way `SQLServerBackend` and `AsyncSQLServerBackend` already read. Both concrete classes drop the prefix -- `SyncSQLServerIntrospector` and `SyncSQLServerStatusIntrospector` -- along with the three core base names their imports refer to (`SyncAbstractIntrospector`, `SyncAbstractStatusIntrospector`, `SyncIntrospectorExecutor`), renamed underneath us by the core change. The backend factory, the two CLI surfaces (introspect and status), the package's exports, and the three test modules (CLI introspect, CLI status, SQL builders) follow. Alias-free by the same reasoning as the core change: a `SyncXxx = Xxx` abbreviation would put two names on one fact, and no other family here spells a pair that way. Method names and the sync/async parity they carry are untouched -- this commit only changes what the classes are called. Verified: imports of all four classes through the package's own export surface, black showing no new debt in any touched file, and a pytest collection pass over the three touched modules (218 cases) with no NameError or ImportError. The four ruff findings -- three in the backend module and one in the CLI status test -- are present at HEAD in the same files at the same sites, untouched by this change. The suite needs a live SQL Server to run further, which CI provides.
…lared gaps Core grew `UnusedIndexesQueryStatement` and `TableUsageStatsQueryStatement`, and each backend's roundtrip walk enumerates every core expression class and demands one of four answers: render, `UnsupportedFeatureError` from a probe, `UnsupportedFeatureError` from the dispatch, or a named row in `LEGITIMATE_NON_RENDERS` with its exception type and message fragment. These two answered none of them -- they reached a `runtime_checkable` protocol stub in the dialect's MRO, which satisfies `getattr` and then returns `None`, and `ProtocolNotImplementedError` is not in the accepted set. The rows are added with each engine's real situation as the reason, so the table reads as the audit it is: * MariaDB -- no per-index use counter and no per-table I/O summary; its statistics are size-oriented. * SQL Server -- its use statistics live in dynamic management views (`sys.dm_db_index_usage_stats`), which describe a live instance rather than a schema-scoped catalogue; pending a decision on how to carry that dependency. * Oracle -- the same shape, through `GV$`/`V$`. * ClickHouse -- no secondary indexes at all, so an unused-index listing is meaningless rather than unanswered; likewise no per-table access counter. * Firebird -- size-oriented statistics only. The probes keep answering False for all five, so a caller learns the gap from `supports_*` as well as from the render attempt. PostgreSQL and MySQL answer both families for real and have their own commits.
Core grew RuntimeMetrics / IndexUsageStats / ObjectDependency / DDLStatement this round, and each backend's roundtrip walk demands one of four answers per class. These seven dialects cannot answer any of the four, so each takes the fourth answer -- a named row with its reason, not a silent absence, so the next reader can tell why rather than guess whether the backend forgot. The reasons differ per engine and say what the engine actually lacks: MariaDB and Firebird keep no per-index use counter or dependency catalogue; SQL Server and Oracle hold theirs in DMV / V$ views that name a live instance, which the dialect layer has no slot to carry (recorded as pending a decision rather than as a permanent no); BigQuery has no secondary indexes at all, so the listing is meaningless rather than unanswered; ClickHouse has no index or per-object use counter and its query_log is a query log, not an object-use stat. bigquery.schema.py also needed its two core-class pins for the previous round -- its collection error had been masking that gap, which is exactly the failure mode the harness.existence check is there to catch.
Core grew the three data-profile statements this round. None of these dialects implements them yet, so each takes the fourth answer the walk demands -- a named row with a reason -- rather than reading as an accident. The reasons are the audit's findings, written where the next reader will look: MySQL and BigQuery can only answer by aggregating and simply have not been written; MariaDB's mysql.column_stats is empty unless an operator asked for statistics, so answering from it would report an empty profile as a measurement; PostgreSQL's pg_stats carries a negative _distinct when it means a fraction and hides min/max inside histogram_bounds, so a naive projection would put a negative count and a string bound in slots that promise otherwise; Oracle's low_value/high_value are RAW and need per-type conversion; SQL Server keys its statistics by stats object rather than column; ClickHouse and Firebird keep no per-column statistics at all. Snowflake also needed the previous round's two rows: its collection error had been masking them, which is exactly what the matrix-exactness assertion is there to catch. MySQL's file uses a different table shape -- a string exception name rather than the class -- so its rows are recorded there in that form.
SQL Server publishes statistics, and their shape is why the catalogue cannot answer a per-column profile. sys.stats names a statistics object, sys.stats_columns maps that object to one or more columns, and sys.dm_db_stats_properties reports what the last update of *that object* measured -- rows, rows_sampled, steps, modification_counter, last_updated and persisted_sample_percent. The key is the object, not the column: a single-column object is a fair proxy for its column, but a two-column one describes the pair's leading prefix, and the same column can appear in several objects with different samples, so summing or picking one would invent a per-column fact the catalogue does not hold. What is missing is sharper than what is keyed oddly. There is no per-column null count anywhere in the catalogue -- nullability is a declaration in sys.columns, never a measurement -- and no per-column distinct count either; sys.dm_db_stats_histogram reports range_high_key, equal_rows and distinct_range_rows per step of one object, a bucket summary for that object's leading column rather than a count of the values in a column. The one figure the catalogue could contribute, last_updated, would be the vintage of a statistics object rather than of the profile, and this family's rule is that a figure travels with the source and vintage that produced it, so it is not borrowed. So all five figures come from one place, and it is the rows: exact, and paid for in time -- one UNION ALL arm per column, each arm one scan of the table, exactly as on MySQL. The provenance is projected into each row as "aggregate" rather than stamped onto the result afterwards, so a consumer comparing this profile with a PostgreSQL estimate can tell the two apart without knowing which engine it asked. An empty table answers a row count of zero and no fraction, because SUM over no rows is NULL and dividing by zero would claim a measurement nobody made. Four SQL Server specifics are written into the projection rather than left to be discovered. SUM and COUNT return INT, so a table past 2,147,483,647 rows -- exactly the table worth profiling -- would overflow the very counts the profile exists to report, and the null flag is summed as BIGINT while the row count is COUNT_BIG. A union resolves one result type per column position by data-type preference, so an int beside a varchar would have the text converted to a number and fail; both endpoints are therefore cast to NVARCHAR(MAX), the one target every scalar column converts to, and they arrive as the engine's own rendering of the value. The column name is the one value in the statement that is caller data rather than a label, so it is bound as a parameter instead of written as a literal: a non-prefixed T-SQL string literal is typed with the database's code page, and an identifier outside that page would come back as question marks. And the coverage fractions are multiplied by 1.0 before the division, because SQL Server divides two integers with integer division and every share below one would answer 0. The three rows that stood for this family in the roundtrip gap table are gone, since all three statements now render. A table holding an xml, text/ntext/image or CLR-typed column cannot be profiled this way at all -- SQL Server refuses MIN and MAX on those types -- and the statement's docstring says so rather than leaving it as a server error to discover. The live test builds a hundred-row table whose every figure is arithmetic, pins the aggregate label and the absence of collected_at, and adds one table with a non-ASCII column name so the bound label cannot regress to a literal without something going red.
The introspect command now offers the same nine families the MySQL and PostgreSQL CLIs ask for, and each one is wired the way those two are: a capability switch in one table, a branch in both dispatch chains, NAME and the type-specific flags validated before a connection opens, and a capability refusal -- exit 2, "not supported by this server" -- rather than an empty answer read as "this schema holds none". What each family answers here is not what the other engines answer, so the work was an audit before it was a wiring job. Two families are answered and the audit found nothing. Per-table usage counters: SQL Server's counters are per *index*, in sys.dm_db_index_usage_stats, and summing them per table would be a derived figure wearing the shape of a measured counter; inserts and deletes are not tracked at all, and sequential scans are not distinguishable from indexed ones at the table level. DDL retrieval: the engine has no per-object call of the kind SHOW CREATE TABLE is -- OBJECT_DEFINITION and sys.sql_modules hold a routine's body, not a CREATE TABLE -- so answering would mean assembling a reconstruction and labelling it native, which is exactly what DDLStatement.native exists to prevent. Both probes answer False, both types stay offered so the refusal is the capability answer rather than an invalid choice, and the roundtrip matrix keeps its two pins with these reasons. Five families are answered, four of them from the dynamic management views the previous round recorded as pending a way to carry the dependency. The way was the join this backend already uses for per-table storage: sys.dm_db_index_usage_stats is joined through sys.objects and sys.schemas, so the schema filter is applied where the statement is built and the counters arrive with the address a caller reads. The unused-index family is the three read counters being zero and index_id > 0 (a heap's row is not an index); the index-counter family projects their sum as scans and leaves rows_read and rows_fetched unprojected, because the engine keeps no per-index row counter and an operation is not a row. Runtime measurements come from sys.dm_os_performance_counters -- the counters the instance actually maintains, so an idle counter produces no row rather than a zero -- with the owning object projected as source and the instance name riding in extra, which is what makes two rows of one counter distinguishable. The dependency graph comes from sys.sql_expression_dependencies: one edge per by-name reference, joined on the referencing object because the referenced side is a name rather than an id. Each of those four, and the value listing that renders OFFSET ... FETCH, asks the version accessor rather than answering a bare True: the views arrived across three releases and the pagination clause is 2012 syntax, so a server below a floor is refused at the capability question instead of handing the engine a statement it cannot parse. Every release this repository tests against -- 2017, 2019, 2022 and 2025 -- is past all of them, which the new tests assert on both sides of each floor. The three data families were already declared and unchanged: every figure is aggregated from the rows because the catalogue cannot answer them, each row carries source='aggregate' so a number never masquerades as a catalogue reading, and the note that goes to stderr says what the count costs. They join the CLI here, so the CLI can ask every family the dialect answers -- which on this engine is seven of the nine plus the families it already had. No changelog fragment and no version bump; the release line takes both.
…ture branch ix/column-constraint-gates has been merged: its tree and elease/v1.0.0.dev17 are identical (the branch carries 21 commits the release commit squashes into one, and the content matches). Pinning CI to the feature branch was always a temporary arrangement -- it meant every job installed a ref that only existed to get one fix in, and the day that branch was deleted the suite would have started failing to install rather than failing a test. The release line is now the thing to test against, which is also what the core repository already pins.
Core now ships SetOperationStatement in backend.expression.statements, re-exported from backend.expression, with format_set_operation_statement on SetOperationMixin and a SetOperationSupport protocol. The roundtrip matrix enumerates every class in the package, so this class had to answer for itself in four parts: it renders and the rendered SQL plus its bind parameters are byte-identical after all three round-trips, or it refuses with UnsupportedFeatureError because the dialect lacks the feature, or it refuses with NotImplementedError because it is an abstract base, or it refuses for some other reason and is named in the not-rendered table with that reason. Every one of those is asserted exactly in both directions, so a class nobody listed is a failure and so is a row left behind after the class started working. Each backend here now gives the answer its own dialect earns, and pins the SQL where it renders, because a render verdict without the bytes says nothing about the dialect.
The NAME rule table was spelled _NAME_RULES here and _NAME_RULE in the four that already agree (core, mysql, mariadb, clickhouse). One spelling across the repositories, so a reader who learns it in one backend finds it in the next, so _NAME_RULE it is and this repo adopts the majority. The table itself is untouched: the same twenty-one types, the same forbidden/optional/required values, the same one reader in _validate_name. Rename only -- the definition, its reader, and the one test that names it. No version bump, no changelog fragment.
The profile arm carried its provenance as the bare string "aggregate"; it now reads ProfileSource.AGGREGATE.value, the member core declares for that closed set. The profile statement rendered before and after the change is byte-identical, so the server receives the same SQL and only the spelling a reader checks changed. DEFAULT_VALUE_SAMPLE_LIMIT = 100 was a second place writing the introspector's own default. It is deleted, and _value_sample_arguments contributes limit to the dispatch kwargs only when --limit was supplied, so a run without the flag sends no limit parameter and the signature answers -- the one intended behaviour change. The help text now says the default is the introspector's own, and the test that asserted 100 asserts the keyword is absent instead. The aggregate-profile sentence was a constant read at the sync and async print sites and pinned by nothing; it is inlined at both, same wording. OUTPUT_CHOICES was written independently by introspect, query and status, and info reused the name for its own two-format list; the four-format list is now single in cli/output.py beside the provider factory, info's list is INFO_OUTPUT_CHOICES beside it, and each subcommand imports the one it offers. Ruff reports four findings in cli/info.py and the CLI blackbox suite reports three errors from its scenario map; both are identical at HEAD and unrelated to this change (verified by stashing the work and re-running). The rest of the CLI suites and the 984-entry roundtrip matrix pass; the live data-profile suite is CI's.
This backend's introspection modules import AbstractIntrospector, ProfileSource, ColumnDataProfile, introspection.defaults.VALUE_SAMPLE_LIMIT and the nine new introspection query statement classes, none of which exist in core 1.0.0.dev29. The >=1.0.0.dev0 floor admitted dev29, which is the version PyPI publishes and what pip install resolved, so the install succeeded and the first import failed. The floor is now >=1.0.0.dev30,<2.0.0, the line that actually carries these symbols, so a fresh resolve either gets a core that imports or stops instead of quietly installing a broken one. The extras deliberately carry no core floor of their own: pip installs extras on top of the main dependencies, so they inherit this floor rather than resolving core independently.
The rule was already a table here, and it was still being re-decided where it was consumed: _require_name carried its own "Table name is required" sentence at four call sites inside _steps, with the same exit and the same wording as _validate_name, which had already run before the connection was opened. So every run refused twice -- once from the table, once from the branch -- and the second copy is the one a new branch would have copied, because it sits beside the method it feeds. _name_of reads _NAME_RULE and refuses on it, and _steps' docstring now says the NAME decision is not made there. A missing name is still an incomplete invocation with exit 1 and the same sentence; a name the type has no question for is still refused with exit 2 and the same sentence; the optional types still narrow. Nothing a run can observe changed. The table was pinned as a set and not as data, so editing one type's answer was invisible. The contents are asserted now, the required set is asserted to be exactly the seven table-scoped types the dispatcher consumes a name for, and each of the three categories is driven through the validator. That is what the cross-repository clause of .claude/rules/constants.md asks for: a private table read in several places earns its name because the readings are asserted. No changelog fragment: no user-visible change.
…e it The floor is right; the order that enforced it was not. Each matrix job installed the requirements and extras first, so anything there that names rhosocial-activerecord resolved against PyPI, whose newest release is 1.0.0.dev29, and stopped before the step that installs the matching core from git ever ran. This workflow's requirements happen not to name the core today, which is why the last run passed, but that left the guarantee resting on which files are edited rather than on the order of the steps. The order was already wrong before the floor moved: the old >=1.0.0.dev0 floor let the wrong core install silently and moved the failure to import time. Every job in this workflow now installs the core branch and the testsuite pin from git before the requirements and extras. Those files are unchanged, constraints included; pip now finds an installed 1.0.0.dev30 that satisfies >=1.0.0.dev30,<2.0.0 and installs only the pieces still missing from the index, instead of looking for a core version PyPI does not have. The floor, the version, the git refs and the testsuite pin are untouched; the install order is the only change.
A new docs/introspection.md page measures SQL Server's answers rather than inferring them: the probe table as the dialect declares it, the four operational families with their DMV sources, the data families with the projection facts (COUNT_BIG, text endpoints, the 1.0 division), the version floors, and the CLI types with their NAME rules and exit codes. The README's Documentation section points at it.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/v1.0.0.dev3 #10 +/- ##
=======================================================
+ Coverage 77.87% 78.30% +0.43%
=======================================================
Files 144 144
Lines 8306 8514 +208
Branches 1289 1324 +35
=======================================================
+ Hits 6468 6667 +199
+ Misses 1408 1375 -33
- Partials 430 472 +42
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
Applies core's settled ownership rule to this backend:
.claude/rules/introspection.md§5–§6, landed by rhosocial/python-activerecord#128 (merged into corerelease/v1.0.0.dev30, 67a76716). Ownership is decided by divergence, never byextra.SyncSQLServerIntrospector→SQLServerIntrospector,SyncSQLServerStatusIntrospector→SQLServerStatusIntrospector, on core'sAbstractIntrospector,AbstractStatusIntrospectorandIntrospectorExecutor. Sync and async stay separate classes.sys.dm_db_partition_statsholds one row per partition per index, so the listing reads three scalar subqueries — total, heap/clustered (index_id0/1), nonclustered — each multiplied by the fixed 8 KiB page. A view keeps its row withNoneslots.supports_table_storage_introspection()now declares the family;row_countstaysNonebecause an exact count is a scan and the listing reads no estimate.SQLServerSequenceInfokeeps its exactDecimals andis_cached;to_core()maps onto core'sSequenceInfo(name drift included:current_value→last_value,is_cycling→cycle). Adecimal(10,2)sequence projects non-integral figures toNonerather than widening the minimum.sys.dm_db_index_usage_stats, runtime metrics fromsys.dm_os_performance_counters, dependency edges fromsys.sql_expression_dependencies.table-statsis refused because that DMV is keyed by object and index,ddlbecause no per-object rebuild text exists; both exit 2 without sending a statement.UNION ALLarm per column (COUNT_BIGpast 2^31 rows, endpoints cast toNVARCHAR(MAX)because a set operator resolves one type per position), samples useOFFSET … FETCH(2012 floor), join coverage multiplies by1.0because the engine divides integers._TYPE_CAPABILITIES, one NAME rule table applied before connecting, and a newdocs/introspection.md.Type of change
feat: A new featuredocs: Documentation only changes (if related to the new feature)perf: A code change that improves performance (if part of the feature)test: Adding missing tests or correcting existing tests (for the new feature)chore: Changes to the build process or auxiliary tools and libraries suchs as documentation generation (if related to the new feature)ci: Changes to our CI configuration files and scripts (if related to the new feature)build: Changes that affect the build system or external dependencies (if related to the new feature)revert: Reverts a previous commitBreaking Change
Impact on End-Users:
No backward-incompatible changes are expected for typical end-user applications.
backend.introspectorandintrospector.statuskeep their entry points; the new types and methods are additive, and a declined family says "not supported" rather than showing an empty list.Impact on Backend Developers (Custom Implementations):
SyncSQLServerIntrospectorandSyncSQLServerStatusIntrospectorare gone; subclasses must attach to core'sAbstractIntrospector/AbstractStatusIntrospectorwithIntrospectorExecutor.SQLServerSequenceInfogainsto_core()and must stay projectable.Related Repositories/Packages
python-activerecord— consumes#128(SequenceInfo, the storage slots, the predicates andformat_*_query_statement); the dependency floor becomes>=1.0.0.dev30.python-activerecord-testsuite— CI installsrelease/v1.0.0.dev17.release/v1.0.0.dev30(which now carries #128) and testsuiterelease/v1.0.0.dev17, so this PR is verified against the merged core rather than the development branch.Testing
Test Plan:
7 test files added or updated — two new (
test_introspect_families_cli.py,test_introspection_data_profile.py), five updated, including the roundtrip matrix, which now answers for core'sSetOperationStatementand records the new core classes and families as declared gaps.Local full-suite execution is prohibited in this workspace, so CI is authoritative: nine jobs across SQL Server 2017–2025 and Python 3.9–3.14, free-threaded included.
Checklist
.claude/code_style.md.README.md, etc.)..claude/version_control.md(changelog.d/<issue_number>.<type>.md).Additional Notes
introspection.mdclause it satisfies; the new docs page states the price of every family.