Skip to content

source-mssql: Fix primary key discovery and clustered index handling - #69194

Merged
Wenqi Hu (burakku) merged 4 commits into
masterfrom
wenqi-mssql-rollout-fix-3
Nov 7, 2025
Merged

Wenqi Hu (burakku) merged 4 commits into
masterfrom
wenqi-mssql-rollout-fix-3

Conversation

@burakku

@burakku Wenqi Hu (burakku) commented Nov 5, 2025 •

Copy link
Copy Markdown
Contributor

Summary

This PR fixes critical issues in the MSSQL source connector related to primary key discovery, views support, and datetime precision after migration to the new CDK (version 4.3.0-rc.3).

Changes

1. Fix CI vs PK Selection (commit aa10bb9)

Problem:

  • After migrating from 4.2.6 to 4.3.0-rc.3, tables with composite primary keys were only showing 1 PK column instead of all columns
  • The connector conflated two separate concepts: discovery PK (for catalog) and sync strategy (which column to use for ordered column loading)
  • Non-unique clustered indexes were not being filtered out, which could cause data integrity issues

Solution:

  • Separated primaryKey() function to return the actual PRIMARY KEY constraint for discovery/catalog purposes
  • Added new getOrderedColumnForSync() function to determine which column to use for syncing (prefers clustered index for performance)
  • Added AND i.is_unique = 1 filter to clustered index query to only use unique indexes for syncing
  • Updated 4 places in MsSqlServerJdbcPartitionFactory to use getOrderedColumnForSync() instead of just using first PK column

Behavior:

  • Discovery: Now correctly returns all columns in composite primary keys
  • Syncing: Prefers single-column unique clustered index, falls back to first PK column
  • Example: For a table with composite PK (RegID, AgentID, assignedDate) and unique clustered index on RegID:
    • Discovery shows all 3 PK columns (matching 4.2.6 behavior)
    • Syncing uses RegID (from clustered index for performance)

2. Fix TABLESAMPLE Error on Views (commit ab392d7)

Problem:

  • Cursor-based incremental reads on views were failing with error:
    Invalid object name 'TABLESAMPLE'
    
  • SQL Server doesn't support TABLESAMPLE clause on views, only on physical tables

Root Cause:

  • MsSqlServerJdbcCursorPartition inherited samplingQuery() method from MsSqlServerJdbcResumablePartition
  • This method used FromSample which generates TABLESAMPLE queries
  • Views cannot be sampled using TABLESAMPLE in SQL Server

Solution:

  • Overrode samplingQuery() method in MsSqlServerJdbcCursorPartition (lines 428-442)
  • New implementation uses regular FROM clause instead of FromSample
  • This makes semantic sense: cursor-based incremental reads typically involve small datasets (only new/changed data), so aggressive sampling isn't necessary

Files changed:

  • MsSqlServerJdbcPartition.kt: Added override of samplingQuery() method

3. Fix DateTime Precision Issue (commit 4e5fcaa)

Problem:

  • BigQuery destination was failing to load data with error:
    Invalid datetime string "2025-11-06T22:30:46.0033333"
    Error: CSV processing encountered too many errors
    
  • SQL Server datetime2 can have up to 7 decimal places of precision
  • BigQuery DATETIME type only supports 6 decimal places (microseconds)
  • Standard LocalDateTimeCodec also expects exactly 6 decimal places

Root Cause:

  • The JDBC read path was using standard TimestampAccessor which preserves full precision from SQL Server
  • When LocalDateTime values with 7 decimal places were encoded to JSON, they exceeded BigQuery's limit
  • Note: CDC path already handled this via MsSqlServerDebeziumConverter using outputDateFormatter with 6 decimals

Solution:

  • Created custom MsSqlServerLocalDateTimeAccessor that truncates datetime values to microsecond precision (lines 122-141)
  • Truncation formula: (localDateTime.nano / 1000) * 1000 - zeros out nanoseconds beyond microseconds
  • Created corresponding MsSqlServerLocalDateTimeFieldType (lines 143-148)
  • Updated field type mappings:
    • JDBCType.TIMESTAMP → MsSqlServerLocalDateTimeFieldType (line 94)
    • MsSqlServerSqlType.DATETIME_TYPES → MsSqlServerLocalDateTimeFieldType (line 239)
  • Now both CDC and JDBC paths handle datetime precision consistently:
    • CDC path: Already used outputDateFormatter with pattern yyyy-MM-dd'T'HH:mm:ss.SSSSSS (6 decimals) ✅
    • JDBC path: Now truncates via custom accessor to 6 decimals ✅

Files changed:

  • MsSqlSourceOperations.kt: Added custom datetime accessor and field type, updated mappings

Test Coverage

All 27 tests passing:

  • ✅ All existing partition factory tests (including view-specific tests)
  • ✅ Clustered index with no PK
  • ✅ PK with no clustered index
  • ✅ Discovery returns PK, sync prefers CI
  • ✅ PK when CI is composite
  • ✅ No PK and no CI
  • ✅ Non-unique CI filtered, falls back to PK
  • ✅ Composite PK discovered correctly
  • ✅ View with configured PK resuming from state ✅
  • ✅ View without configured PK ✅
  • ✅ View in full refresh mode ✅
  • ✅ View detection for table vs view ✅

Backward Compatibility

These changes maintain complete feature parity with version 4.2.6:

  • Discovery behavior matches (returns all PK columns)
  • Sync strategy matches (prefers clustered index for performance)
  • Non-unique clustered indexes are now correctly filtered out
  • Views now work in cursor-based incremental mode (previously failed)
  • DateTime precision matches legacy connector behavior (6 decimal places)
  • Compatible with BigQuery and other destinations expecting standard datetime precision

Impact

  • Views support: Views can now be synced in cursor-based incremental mode without TABLESAMPLE errors
  • BigQuery compatibility: DateTime values now load successfully into BigQuery DATETIME columns
  • Data consistency: Non-unique clustered indexes no longer cause sync issues
  • Discovery accuracy: Composite primary keys are fully discovered

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

@github-actions

github-actions Bot commented Nov 5, 2025

Copy link
Copy Markdown
Contributor

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Helpful Resources

PR Slash Commands

Airbyte Maintainers (that's you!) can execute the following slash commands on your PR:

  • /format-fix - Fixes most formatting issues.
  • /bump-version - Bumps connector versions.
    • You can specify a custom changelog by passing changelog. Example: /bump-version changelog="My cool update"
    • Leaving the changelog arg blank will auto-populate the changelog from the PR title.
  • /run-cat-tests - Runs legacy CAT tests (Connector Acceptance Tests)
  • /build-connector-images - Builds and publishes a pre-release docker image for the modified connector(s).
  • JVM connectors:
    • /update-connector-cdk-version connector=<CONNECTOR_NAME> - Updates the specified connector to the latest CDK version.
      Example: /update-connector-cdk-version connector=destination-bigquery
    • /bump-bulk-cdk-version type=patch changelog='foo' - Bump the Bulk CDK's version. type can be major/minor/patch.
  • Python connectors:
    • /poe connector source-example lock - Run the Poe lock task on the source-example connector, committing the results back to the branch.
    • /poe source example lock - Alias for /poe connector source-example lock.
    • /poe source example use-cdk-branch my/branch - Pin the source-example CDK reference to the branch name specified.
    • /poe source example use-cdk-latest - Update the source-example CDK dependency to the latest available version.

📝 Edit this welcome message.

@github-actions

github-actions Bot commented Nov 5, 2025 •

Copy link
Copy Markdown
Contributor

source-mssql Connector Test Results

 10 files   10 suites   14m 10s ⏱️
 92 tests  92 ✅ 0 💤 0 ❌
202 runs  202 ✅ 0 💤 0 ❌

Results for commit 89504bf.

♻️ This comment has been updated with latest results.

@burakku
Wenqi Hu (burakku) force-pushed the wenqi-mssql-rollout-fix-3 branch 2 times, most recently from 788ae4e to 10865d1 Compare November 5, 2025 23:42
@burakku
Wenqi Hu (burakku) requested a review from a team November 5, 2025 23:42
@github-actions

github-actions Bot commented Nov 5, 2025 •

Copy link
Copy Markdown
Contributor

Deploy preview for airbyte-docs ready!

✅ Preview
https://airbyte-docs-k10guy5fc-airbyte-growth.vercel.app

Built with commit 89504bf.
This pull request is being automatically deployed with vercel-action

@burakku
Wenqi Hu (burakku) force-pushed the wenqi-mssql-rollout-fix-3 branch from 10865d1 to 88f4b04 Compare November 6, 2025 22:55
@sophiecuiy
sophiecuiy self-requested a review November 6, 2025 23:03
@burakku
Wenqi Hu (burakku) force-pushed the wenqi-mssql-rollout-fix-3 branch from 88f4b04 to 43047a2 Compare November 7, 2025 00:57
@burakku
Wenqi Hu (burakku) force-pushed the wenqi-mssql-rollout-fix-3 branch from 4e5fcaa to 89504bf Compare November 7, 2025 18:36
@burakku
Wenqi Hu (burakku) merged commit f88fce7 into master Nov 7, 2025
47 of 48 checks passed
@burakku
Wenqi Hu (burakku) deleted the wenqi-mssql-rollout-fix-3 branch November 7, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants