Skip to content

Expand live database integration coverage - #171

Merged
jogibear9988 merged 27 commits into
masterfrom
codex/expand-live-database-tests
Sep 22, 2026
Merged

jogibear9988 merged 27 commits into
masterfrom
codex/expand-live-database-tests

Conversation

@jogibear9988

@jogibear9988 jogibear9988 commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Expand live coverage from the existing SQL Server, PostgreSQL, Oracle, SQLite and MySQL smoke tests to ten database engines. MySQL now has a full migration contract suite; MariaDB, Firebird, Db2, Informix and Sybase ASE gain isolated live suites using public pinned images and drivers.

Each new suite checks persisted CRUD/defaults, table and column metadata/changes, identities, keys and constraints, ordered indexes, database/view catalogs, and repeated migration up/down with version tracking. Implement the missing native provider operations and add regressions for the metadata, index ownership and database lifecycle issues found by Copilot. Preserve explicit decimal/DECFLOAT precision, composite and identity key membership, BLOB/text/time/byte types, native Firebird DATE/TIME/BOOLEAN types, full Informix character lengths, and typed defaults when inspecting and recreating schemas. Honor Unique in provider-specific column alterations and preserve Informix fixed-length/large-text metadata. Decode Informix typed defaults and preserve ASE large-text capacity during metadata copies. Keep provider identifiers, public signatures and the existing target framework.

CI runs separate bounded jobs, retains database logs and test artifacts, rejects empty/new skipped suites, and audits all NUnit-discovered cases for complete job assignment. Linux-specific drivers remain conditional test dependencies. Reproduction commands, versions and engine limitations are in docs/live-database-tests.md.

Ingres is excluded because the official public image requires a commercial license.xml key. No secrets, external database services or private downloads were added.

Validation:

  • Rebased onto master at 8a269d0, including the latest SQLite foreign-key fixes.
  • Local build passes; 64 unit tests and 139 SQLite tests pass, with the existing issue-139 skip.
  • Final commit 8c0743b passes all 12 workflow jobs: https://github.com/dotnetprojects/Migrator.NET/actions/runs/35715528132 . NUnit reports 631 passed and 5 pre-existing ignored tests; all 636 discovered cases are assigned exactly once.
  • All 45 Copilot review threads are resolved after checking the findings and adding live regressions for valid issues. Every branch commit includes the reason for the change and validation details.

Final NUnit results:

Job Passed Existing skips
Unit 64 0
SQLite 139 1
SQL Server 109 2
PostgreSQL 132 1
Oracle 90 1
MySQL 13 0
MariaDB 12 0
Firebird 19 0
Db2 16 0
Informix 19 0
Sybase ASE 18 0

The five existing skips were reviewed: default-removal issue #139 in SQLite, SQL Server, PostgreSQL and Oracle, plus SQL Server issue #132. New suites execute nonzero tests with no skips. The final artifacts contain startup/container logs, inspection data, TRX and NUnit XML. An earlier Informix image pull encountered an IBM registry timeout; the final run successfully downloads, starts and tests it.

Copilot AI lite review requested due to automatic review settings September 22, 2026 07:44

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.

Copilot review overview

🟡 Changes recommended

Critical CI, live-contract, and Firebird metadata findings remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 High severity · 1 Medium severity

Open (4)
What changed in this PR

Expands live database integration coverage with isolated CI jobs, migration contract tests, and provider metadata updates. The PR remains a draft pending required checks.

Changes:

  • Adds live database contract tests and database-specific CI jobs.
  • Adds environment-based connection overrides and provider dependencies.
  • Enhances MySQL/MariaDB and Firebird metadata and DDL support.
  • Normalizes PostgreSQL test categories and filtering.
File Summary
src/​Migrator/​Providers/​Impl/​Mysql/​MySqlTransformationProvider.cs Adds MySQL metadata, constraint, index, and column operations. tinyint(1) is currently mapped as DbType.Byte instead of Boolean (moderate; 3 votes).
src/​Migrator/​Providers/​Impl/​Mysql/​MariaDBTransformationProvider.cs Reuses shared MySQL initialization.
src/​Migrator/​Providers/​Impl/​Firebird/​FirebirdTransformationProvider.cs Adds Firebird metadata and DDL operations. BLOB text subtype mapping is incorrect (moderate; 1 vote), and numeric precision/scale are not assigned (critical; 2 votes).
src/​Migrator/​Providers/​Impl/​Firebird/​FirebirdDialect.cs Updates Firebird type and identity mappings.
src/​Migrator/​Providers/​Impl/​Firebird/​FirebirdColumnPropertiesMapper.cs Adjusts Firebird column definition ordering.
src/​Migrator.Tests/​Settings/​ConfigurationReader.cs Adds environment-based connection overrides.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgresSQLTransformationProvider_UpdateFromTableToTableTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProviderGenericTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_ViewExistsTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_TableExistsTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_ReservedWordsTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_PrimaryKeyWithIdentityTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_PrimaryKeyExistsTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_GetColumnsTypeTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_GetColumns_Tests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_GetColumns_DefaultValueTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_GetColumnContent_SizeTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_DefaultValueTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_CopyDataFromTableToTableTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_ConstraintExists.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_ChangeColumnTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_AddTableTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_AddPrimaryKeyTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​PostgreSQLTransformationProvider_AddIndexTests.cs Normalizes the PostgreSQL test category.
src/​Migrator.Tests/​Providers/​PostgreSQL/​Base/​PostgreSQLTransformationProviderTestBase.cs Updates the PostgreSQL base-test category.
src/​Migrator.Tests/​Providers/​Live/​LiveDatabaseTests.cs Adds live contract tests. Db2 and Informix jobs exercise unsupported metadata operations and fail early (critical; 3 votes).
src/​Migrator.Tests/​ProviderFactoryTest.cs Aligns provider test categories.
src/​Migrator.Tests/​Migrator.Tests.csproj Adds live database provider dependencies.
src/​Migrator.Tests/​Dialects/​PostgreSQLDialectTests.cs Updates the PostgreSQL dialect category.
.github/​workflows/​dotnetpull.yml Adds database matrix CI jobs. Container connection strings are not wired into the test environment (critical; 1 vote).
.github/​scripts/​test.ps1 Adds category-based test execution. SQLServer and SqlServer category mismatch causes incorrect test selection (moderate; 1 vote).
.github/​scripts/​start-database.sh Starts and initializes database containers.

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

Comment thread .github/workflows/dotnetpull.yml
Comment thread src/Migrator.Tests/Providers/Live/LiveDatabaseTests.cs Outdated
Comment thread src/Migrator/Providers/Impl/Firebird/FirebirdTransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Mysql/MySqlTransformationProvider.cs Outdated
Copilot AI review requested due to automatic review settings September 22, 2026 07:51

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.

Comment thread src/Migrator/Providers/Impl/DB2/DB2TransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/DB2/DB2TransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Informix/InformixTransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Mysql/MySqlTransformationProvider.cs Outdated
Copilot AI review requested due to automatic review settings September 22, 2026 08:07

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.

Copilot review overview

🟡 Changes recommended

Thirteen unresolved findings remain, including one critical and twelve moderate provider metadata and DDL issues.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 High severity · 7 Medium severity

Open (12)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Initialize Indexed flag for Db2 column mapping

src/​Migrator/​Providers/​Impl/​DB2/​DB2Dialect.cs:50

This custom mapper bypasses the base implementation but never initializes _Indexed. JoinIndexes uses that flag when creating a table, so columns marked ColumnProperty.Indexed silently lose their inline index on Db2. Copy the flag from the column before building parts.

Medium severity Initialize Indexed flag for Informix column mapping

src/​Migrator/​Providers/​Impl/​Informix/​InformixDialect.cs:51

This custom mapper bypasses the base implementation but never initializes _Indexed. JoinIndexes uses that flag when creating a table, so columns marked ColumnProperty.Indexed silently lose their inline index on Informix. Copy the flag from the column before building parts.

Comment thread src/Migrator/Providers/Impl/Informix/InformixTransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Sybase/SybaseDialect.cs
Comment thread src/Migrator/Providers/Impl/Sybase/SybaseTransformationProvider.cs Outdated
Copilot AI review requested due to automatic review settings September 22, 2026 08:16

Copilot AI left a comment

Copy link
Copy Markdown

Comment thread src/Migrator/Providers/Impl/DB2/DB2Dialect.cs
Comment thread src/Migrator/Providers/Impl/DB2/DB2TransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Informix/InformixDialect.cs
Comment thread src/Migrator/Providers/Impl/Informix/InformixTransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Mysql/MySqlTransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Sybase/SybaseTransformationProvider.cs Outdated
Copilot AI review requested due to automatic review settings September 22, 2026 08:27
Why: The former combined workflow did not exercise most existing providers and MySQL only had a connection smoke test. Independent pinned containers make failures attributable to one engine and avoid requiring private infrastructure.

Changes: Add isolated contract tests for persisted CRUD, DDL, constraints, indexes and repeatable migration/version tracking. Preserve existing suites while adding public-container startup, bounded jobs and diagnostic artifacts.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
…est routing

Why: Enabling Db2 and Informix exposed placeholder catalog and DDL implementations. Excluding these engines for incomplete providers would leave the requested migration coverage unfulfilled.

Changes: Implement native catalog inspection, type/identity mapping and schema operations. Normalize database categories so each matrix job runs the complete existing suite, including provider-factory tests.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: The IBM Linux clients require their packaged native libraries to be discoverable. Startup failures were also masked by a logging pipeline, and Windows line endings broke bash execution.

Changes: Select Linux driver packages conditionally, configure library paths, and enforce bash pipefail. Normalize shell script line endings so container failures stop the job before misleading connection failures.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: Firebird live tests rejected the generic DROP COLUMN syntax. Public IBM registry downloads also intermittently timed out before a server could start.

Changes: Use Firebird's native DROP field syntax and retry image downloads only. Test failures remain fatal and are never retried or ignored.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: The public ASE developer image and managed client connect successfully, so Sybase can be included without secrets or private vendor downloads. Its previous provider methods were insufficient for migration tests.

Changes: Add the isolated ASE suite, pinned startup and conditional managed driver. Implement native catalogs, constraints, identity/type mapping and schema operations while rejecting unsupported referential actions explicitly.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
…lability

Why: Db2 live tests showed nullable primary-key declarations are rejected and migration commits reopen the connection, losing a session-only current-schema setting.

Changes: Emit explicit NOT NULL for key columns and persist the isolated schema in the connection string. This keeps migration version tracking and cleanup in the intended schema after reconnects.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: Informix onstat reports ONLINE using exit code 5, which the original readiness pipeline treated as failure. Firebird and Db2 reject removing an absent default. Category filtering also needs evidence that no tests are orphaned.

Changes: Handle the native readiness status, avoid redundant default removal, and audit NUnit discovery against all job results. Require nonzero execution and disallow skips in the new suites.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: ASE's default device lacks enough space for databases copied from its model, and Informix disallows CREATE DATABASE through a connection already attached to a database.

Changes: Allocate disposable ASE storage and use a server-only Informix administration connection. Document pinned versions, reproduction steps, engine limits and the concrete Ingres license-key blocker.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
…SE operations

Why: Copilot identified lost decimal precision, boolean/default metadata, constraint ownership and quoted identifier case. Live ASE tests additionally exposed incorrect ADD COLUMN syntax, asynchronous connection release and a string migration-version parameter.

Changes: Add focused live regressions and repair metadata round-trips, native DDL, bounded teardown synchronization and typed version deletion. Audit inherited/repeated NUnit cases without mistaking identical names within one job for cross-job duplication.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: The Firebird regression proved DROP DATABASE is not accepted through the driver's DSQL command path, even without a database name. ASE column changes requiring a data copy need an enabled logging option.

Changes: Invoke the registered Firebird driver's database API after validating the attached database, keeping the library driver-independent. Enable full ALTER TABLE logging in isolated ASE test databases.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: Custom column mappers discarded ColumnProperty.Indexed, and ASE constraint-owned indexes were indistinguishable from ordinary indexes. Removing them through DROP INDEX is incorrect.

Changes: Preserve index intent in the native mappers and read ASE constraint/clustered status bits. Add live tests for flagged column indexes and removal of primary/unique constraint-backed indexes.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: Preserving the indexed flag alone would emit the base provider's MySQL-specific inline INDEX syntax on other engines. Native engines require separate CREATE INDEX statements.

Changes: Disable only the inline syntax for Db2, Informix, ASE and Firebird, then create flagged column indexes after table creation. Validate this behavior against all four live engines.

Validation: local builds and targeted live GitHub Actions runs informed this change; the complete matrix is required on the final PR commit.
Why: ASE's full-logging option did not take effect on the mixed data/log
allocation, so both live column-change scenarios still rejected ALTER TABLE
operations that copy data. The disposable databases need a distinct log
allocation as well as the option setting.

Changes: create each ASE test database with 32 MB of data and a separate
16 MB log allocation. Issue CHECKPOINT before tests to apply database options,
and document these setup requirements.

Validation: based on the two failures in GitHub Actions run 35704716588;
the full matrix will validate the correction on the rebased PR commit.
Why: Copilot's follow-up review found that Informix index inspection omitted
primary/unique constraint ownership. RemoveAllIndexes would therefore issue
DROP INDEX for objects owned by constraints, which the engine rejects.

Changes: join the constraint catalog to populate index flags, and remove
constraints by their actual names before ordinary indexes. Constraint and
backing-index names are not interchangeable in Informix.

Validation: add a live regression that creates primary and unique constraints,
checks both metadata flags, removes all indexes, and verifies constraint
removal. Local build passes; the full CI matrix runs on this commit.
Why: LOG ON default can select ASE's master device, which rejects log
allocation because it contains only data. The test harness must select its
own devices explicitly to make isolated database creation reproducible.

Changes: initialize a separate 64 MB migrator_log device alongside the
128 MB migrator_data device, and name both in CREATE DATABASE. Document the
same device requirement for local connection-string overrides.

Validation: the preceding CI run identified the master-device allocation
error before tests started. The complete public matrix validates this setup;
no engine tests are skipped or allowed to fail.
Copilot AI review requested due to automatic review settings September 22, 2026 09:24

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.

Copilot review overview

🟡 Changes recommended

Unresolved critical uniqueness issues remain in DB2, Firebird, and Sybase, alongside moderate metadata, default-parsing, and CI allocation findings.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 High severity · 2 Medium severity

Open (5)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Parse Informix catalog defaults into typed values

src/​Migrator/​Providers/​Impl/​Informix/​InformixTransformationProvider.cs:87

Informix defaults are assigned as raw catalog text, unlike the typed parsing used by the other new providers. For example, an INTEGER DEFAULT 7 is returned as a string, so reusing GetColumns metadata re-emits it as a quoted string literal; parse the catalog source with CatalogDefaultValue.Parse(reader.GetString(3), type).

Comment thread src/Migrator/Providers/Impl/DB2/DB2TransformationProvider.cs
Comment thread src/Migrator/Providers/Impl/Sybase/SybaseTransformationProvider.cs
Comment thread src/Migrator/Providers/Impl/Informix/InformixDialect.cs Outdated
Why: five Copilot findings exposed missing Unique handling in Db2, Firebird and ASE ChangeColumn overrides, invalid oversized LVARCHAR DDL, and loss of fixed-length/large-text metadata in Informix schema copies.

Match the base Unique contract by clearing the flag during alteration and adding the named constraint afterward. Bound Informix LVARCHAR at 32739 bytes, use TEXT for larger string requests, support CHAR lengths through 32767, and distinguish fixed-length, TEXT and extended CLOB catalog metadata. Resolve extended types through sysxtdtypes instead of relying on ambiguous opaque type codes.

Add live regressions for uniqueness metadata, duplicate rejection and constraint removal in all three providers, plus Informix large-object recreation/persistence and fixed-length type preservation.

Validation: local build succeeds; Unit 64 passed and SQLite 139 passed with one existing issue-139 skip. Latest master is already included. Full container-backed GitHub Actions validation follows this push.
Copilot AI review requested due to automatic review settings September 22, 2026 09:38
…ptions

Why: CI confirms ASE enforces the new unique constraint, but its legacy driver exception does not inherit DbException. Informix rejects a maximum-width LVARCHAR sharing a row with a TEXT locator because the combined row exceeds 32767 bytes.

Reuse the existing provider-aware database-error assertion and put maximum-width LVARCHAR and TEXT boundary checks in separate tables. Keep duplicate rejection, exact boundary metadata, and large-text persistence assertions intact.

Validation: local solution build passes. The first CI run verified Firebird uniqueness and exposed these two fixture assumptions; the full matrix will rerun on this commit.
Why: CI reached the server ONLINE state while the image initializer still held a sysmaster lock, causing test-database creation to fail before any tests ran.

Bound bootstrap database creation to 24 attempts with five-second delays and retain errors/container logs on exhaustion. Test failures remain fatal and are never retried by this loop. Document Informix string boundaries and the TEXT representation used when recreating large-text metadata.

Validation: inspected the failed startup log (425/107 catalog lock); shell diff passes whitespace checks. Rerun the complete live matrix on this commit.

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.

Copilot review overview

🔵 Needs a closer look

Unresolved moderate provider behavior and live-test assertion issues remain.

Review effort: Lite
Findings: None

Resolved since last review (5)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Parse Informix catalog defaults instead of quoting SQL expressions

src/​Migrator/​Providers/​Impl/​Informix/​InformixTransformationProvider.cs:94

sysdefaults.default is copied into Column.DefaultValue as a string, but Dialect.Default quotes strings when recreating a column. A numeric default such as 7 becomes DEFAULT '7', and an expression such as CURRENT becomes a quoted literal rather than SQL. Parse this catalog SQL into a typed literal or expression, as the other new providers do.

Medium severity Map ASE money columns to DbType.Currency

src/​Migrator/​Providers/​Impl/​Sybase/​SybaseTransformationProvider.cs:61

ASE money is mapped to DbType.Decimal even though SybaseDialect registers DbType.Currency as MONEY. Re-inspecting a MONEY column and recreating it therefore emits DECIMAL instead of preserving the native type; map this catalog type to DbType.Currency (and handle any supported small-money variant consistently).

Medium severity Report ASE TEXT columns with unlimited size

src/​Migrator/​Providers/​Impl/​Sybase/​SybaseTransformationProvider.cs:78

ASE reports TEXT columns through the catalog length of their text pointer, not their usable text capacity. Assigning that length here makes an inspected TEXT column look small (commonly 16), so GetColumns → AddTable can recreate it as a short VARCHAR and lose capacity. Report large text columns with Size = int.MaxValue so the dialect selects TEXT.

Copilot AI review requested due to automatic review settings September 22, 2026 09:47

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.

Copilot review overview

🟡 Changes recommended

Unresolved findings affect standalone index creation, typed Informix defaults, and Sybase type-preserving schema round-trips.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)

Comment thread src/Migrator/Providers/Impl/Sybase/SybaseTransformationProvider.cs Outdated
Comment thread src/Migrator/Providers/Impl/Informix/InformixTransformationProvider.cs Outdated
Why: Copilot identified that ASE TEXT/UNITEXT pointer lengths were being mistaken for column capacity, and Informix defaults were returned as raw catalog strings. Recreated schemas could truncate large-text capacity or misquote defaults.

Use the large-text size sentinel for ASE TEXT/UNITEXT. Decode Informix literal prefixes and default-kind codes before using the shared typed parser; preserve unquoted string literals and distinguish CURRENT/TODAY expressions from literals. Render typed boolean defaults using Informix's quoted t/f syntax.

Add live metadata-copy regressions for both ASE large-text types with 5000-character persistence and Informix integer, decimal, true/false, quoted string, current timestamp/date, and null defaults.

Validation: local solution build passes; Unit 64 passed and SQLite 139 passed with its existing issue-139 skip. Latest origin/master is included. Full database CI will verify the new regressions after push.
Copilot AI review requested due to automatic review settings September 22, 2026 10:05
Why: the live metadata-copy regression passed integer and decimal defaults but exposed a boolean literal representation that the initial t/f decoder returned as an expression.

Normalize quoted literal text and any catalog prefix before interpreting t/f. Add default-value/type diagnostics to the regression output so any remaining engine-specific representation is visible in CI artifacts.

Validation: local solution build passes. Full live CI reruns on this commit; no assertions or skip policies were weakened.
Why: live diagnostics show SYSDEFAULTS character/boolean literals contain a NUL terminator before catalog padding. It made t/f fail typed parsing and leaked into inspected string defaults.

Read literal content only up to its terminator, preserving meaningful leading and trailing spaces. Strengthen the live regression to verify both spaces survive metadata inspection and insertion into a recreated table.

Validation: local build passes. CI confirmed numeric defaults already parse correctly and isolated the t-NUL/f-NUL/string-NUL representations. Full live matrix reruns on this commit.

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.

Copilot review overview

🔵 Needs a closer look

Unresolved moderate findings remain across DB2, Firebird, Informix, and Sybase provider behavior.

Review effort: Lite
Findings: None

Resolved since last review (2)
Previously missed (5)

In code that hasn't changed since last review

Medium severity Create standalone indexes when adding indexed columns

src/​Migrator/​Providers/​Impl/​DB2/​DB2TransformationProvider.cs:28

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql cannot emit an inline index. This AddColumn overload only executes the mapped column SQL and never creates the standalone index, so adding a Column with ColumnProperty.Indexed silently loses the requested index even though AddTable handles it. Add the same AddIndex call used after AddTable here (and cover the AddColumn path).

Medium severity Create standalone indexes when adding indexed columns

src/​Migrator/​Providers/​Impl/​Firebird/​FirebirdTransformationProvider.cs:30

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql cannot emit an inline index. This AddColumn overload only executes the mapped column SQL and never creates the standalone index, so adding a Column with ColumnProperty.Indexed silently loses the requested index even though AddTable handles it. Add the same AddIndex call used after AddTable here (and cover the AddColumn path).

Medium severity Create standalone indexes when adding indexed columns

src/​Migrator/​Providers/​Impl/​Informix/​InformixTransformationProvider.cs:27

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql cannot emit an inline index. This AddColumn overload only executes the mapped column SQL and never creates the standalone index, so adding a Column with ColumnProperty.Indexed silently loses the requested index even though AddTable handles it. Add the same AddIndex call used after AddTable here (and cover the AddColumn path).

Medium severity Create standalone indexes when adding indexed columns

src/​Migrator/​Providers/​Impl/​Sybase/​SybaseTransformationProvider.cs:25

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql cannot emit an inline index. This AddColumn overload only executes the mapped column SQL and never creates the standalone index, so adding a Column with ColumnProperty.Indexed silently loses the requested index even though AddTable handles it. Add the same AddIndex call used after AddTable here (and cover the AddColumn path).

Medium severity Map ASE MONEY columns to DbType.Currency

src/​Migrator/​Providers/​Impl/​Sybase/​SybaseTransformationProvider.cs:62

ASE MONEY is explicitly represented by DbType.Currency in SybaseDialect (line 20), but this mapping reads it as DbType.Decimal. A GetColumns → AddTable round trip therefore recreates MONEY as DECIMAL and loses the native type; map money to DbType.Currency, as the SQL Server provider does.

Copilot AI review requested due to automatic review settings September 22, 2026 10:16
Why: the live round-trip now verifies all typed literal defaults, but Informix rejects bare CURRENT as a default for DATETIME YEAR TO FRACTION(5).

Decode the catalog's CURRENT default kind with an explicit YEAR TO FRACTION(5) qualifier, matching the timestamp declaration produced by InformixDialect. Keep the live insertion check for timestamp persistence.

Validation: local build passes. CI verified integer, decimal, both boolean values, whitespace-preserving string literals, and null metadata before exposing this default-expression precision mismatch. Full CI reruns on the corrected commit.
Why: the live regression now recreates the table and verifies numeric/boolean persistence, but a raw VARCHAR read removes the trailing blank from the returned value.

Append a nonblank marker in SQL before reading the value. This keeps the trailing-space persistence assertion exact while distinguishing server data from client trimming. Catalog metadata still must preserve both leading and trailing whitespace.

Validation: local build passes. Full live CI reruns without removing any default-value assertions.

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.

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

Previously missed (4)

In code that hasn't changed since last review

Medium severity AddColumn drops indexed property when standalone indexes are required

src/​Migrator/​Providers/​Impl/​DB2/​DB2TransformationProvider.cs:28

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql returns null, so this path only adds the column and silently drops ColumnProperty.Indexed. The AddTable override handles the same flag, but AddColumn does not; create the standalone index after ALTER TABLE and cover this overload with a regression.

Medium severity AddColumn drops indexed property when standalone indexes are required

src/​Migrator/​Providers/​Impl/​Firebird/​FirebirdTransformationProvider.cs:30

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql returns null, so this path only adds the column and silently drops ColumnProperty.Indexed. The AddTable override handles the same flag, but AddColumn does not; create the standalone index after ALTER TABLE and cover this overload with a regression.

Medium severity AddColumn drops indexed property when standalone indexes are required

src/​Migrator/​Providers/​Impl/​Informix/​InformixTransformationProvider.cs:27

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql returns null, so this path only adds the column and silently drops ColumnProperty.Indexed. The AddTable override handles the same flag, but AddColumn does not; create the standalone index after ALTER TABLE and cover this overload with a regression.

Medium severity AddColumn drops indexed property when standalone indexes are required

src/​Migrator/​Providers/​Impl/​Sybase/​SybaseTransformationProvider.cs:25

Because this dialect sets SupportsIndex=false, ColumnPropertiesMapper.IndexSql returns null, so this path only adds the column and silently drops ColumnProperty.Indexed. The AddTable override handles the same flag, but AddColumn does not; create the standalone index after ALTER TABLE and cover this overload with a regression.

Copilot AI review requested due to automatic review settings September 22, 2026 10:28

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.

Copilot review overview

🔵 Needs a closer look

Eight unresolved moderate findings affect indexed-column creation, lifecycle compatibility, and native monetary or decimal metadata preservation.

Review effort: Lite
Findings: None

Previously missed (6)

In code that hasn't changed since last review

Medium severity Create standalone indexes for indexed columns

src/​Migrator/​Providers/​Impl/​DB2/​DB2TransformationProvider.cs:28

Because this dialect sets SupportsIndex=false, the mapper's Indexed flag cannot emit an inline index. This override adds the column but never creates a standalone index, so AddColumn(table, new Column(..., ColumnProperty.Indexed)) silently loses the requested index; the typed AddColumn overload also needs to funnel through the same handling.

Medium severity Create standalone indexes for indexed columns

src/​Migrator/​Providers/​Impl/​Firebird/​FirebirdTransformationProvider.cs:30

Because this dialect sets SupportsIndex=false, the mapper's Indexed flag cannot emit an inline index. This override adds the column but never creates a standalone index, so AddColumn(table, new Column(..., ColumnProperty.Indexed)) silently loses the requested index; the typed AddColumn overload also needs to funnel through the same handling.

Medium severity Create standalone indexes for indexed columns

src/​Migrator/​Providers/​Impl/​Informix/​InformixTransformationProvider.cs:27

Because this dialect sets SupportsIndex=false, the mapper's Indexed flag cannot emit an inline index. This override adds the column but never creates a standalone index, so AddColumn(table, new Column(..., ColumnProperty.Indexed)) silently loses the requested index; the typed AddColumn overload also needs to funnel through the same handling.

Medium severity Map Informix MONEY to Currency

src/​Migrator/​Providers/​Impl/​Informix/​InformixTransformationProvider.cs:68

Informix catalog type code 8 is MONEY, but it is grouped with DECIMAL here. That makes metadata copies use DECIMAL(18,5) rather than InformixDialect's DbType.Currency mapping (DECIMAL(18,4)); keep MONEY as DbType.Currency so its native precision/scale semantics survive inspection and recreation.

Medium severity Create standalone indexes for indexed columns

src/​Migrator/​Providers/​Impl/​Sybase/​SybaseTransformationProvider.cs:25

Because this dialect sets SupportsIndex=false, the mapper's Indexed flag cannot emit an inline index. This override adds the column but never creates a standalone index, so AddColumn(table, new Column(..., ColumnProperty.Indexed)) silently loses the requested index; the typed AddColumn overload also needs to funnel through the same handling.

Medium severity Map ASE MONEY to Currency

src/​Migrator/​Providers/​Impl/​Sybase/​SybaseTransformationProvider.cs:62

ASE's native MONEY type is mapped to DbType.Decimal here, so GetColumns -> AddTable recreates it with the dialect's DECIMAL(18,5) mapping instead of MONEY. Map MONEY to DbType.Currency, as SybaseDialect maps Currency back to MONEY, to preserve the native type and scale semantics.

@jogibear9988
jogibear9988 merged commit 02e8bf3 into master Sep 22, 2026
13 checks passed
@jogibear9988
jogibear9988 deleted the codex/expand-live-database-tests branch September 22, 2026 12:04
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