[sql_server] Create all *Source related types, support CREATE CONNECTION ... TO SQL SERVER#32087
Merged
ParkMyCar merged 6 commits intoMaterializeInc:mainfrom Apr 9, 2025
Merged
Conversation
ParkMyCar
force-pushed
the
sql_server/create-connection
branch
2 times, most recently
from
8a961cf to
d50d51d
Compare
ParkMyCar
force-pushed
the
sql_server/create-connection
branch
3 times, most recently
from
7d050e4 to
cfed48d
Compare
5 tasks
def-
reviewed
Apr 8, 2025
ci/test/pipeline.template.yml
Outdated
| timeout_in_minutes: 30 | ||
| parallelism: 1 | ||
| depends_on: build-x86_64 | ||
| timeout_in_minutes: 10 |
Contributor
There was a problem hiding this comment.
10 minutes is probably not enough.
Contributor
There was a problem hiding this comment.
Pushed with 30 min and worked.
martykulma
approved these changes
Apr 8, 2025
Contributor
martykulma
left a comment
martykulma
left a comment
There was a problem hiding this comment.
wow, that is a significant amount of stuff, thanks for putting it into a separate PR!
aside from the copy-paste, it all makes sense to me!
src/storage-types/src/connections.rs
Outdated
| for (compatible, field) in compatibility_checks { | ||
| if !compatible { | ||
| tracing::warn!( | ||
| "MySqlConnection incompatible at {field}:\nself:\n{:#?}\n\nother\n{:#?}", |
Contributor
There was a problem hiding this comment.
s/MySqlConnection/SqlServerConnection/
…ONNECTION * Creates all of the various 'Source' and 'Connection' related types with their purification and SourceRender implementations stubbed out * Adds support for CREATE CONNECTION ... TO SQL SERVER
ParkMyCar
force-pushed
the
sql_server/create-connection
branch
from
6aa48fb to
b4739dc
Compare
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
3 participants
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.
This PR adds all of the various SQL Server 'Source' and 'Connection' types, and leaves the implementations of Purification and Source Rendering as TODOs. Given the number of new types required, and files that need to be touched, I figured it was easier to make this into its own PR.
The two main types this PR adds are:
SqlServerSource(analogous toPostgresSourceConnection)SourceConnection,AlterCompatible, andSourceRendertraits.SqlServerConnectionDetails(analogous toPostgresConnection)IntoInlineConnectionandAlterCompatible.Note: I slightly differed from the naming convention here because to me
PostgresSourceConnectionandPostgresConnectionwere similar enough that I kept getting the two confused.We also add several types related to these two:
SqlServerSourceSqlServerSourceExtras- like the MySQL source, this type is empty but include to conform with other sources.SqlServerSourceExportDetails- details of the specific upstream tables we'll be replicating.GenericSourceConnection::SqlServervariant - enum wrapper around allSOURCEtypes.SqlServerConnectionDetailsConnection::SqlServervariant - enum wrapper around allCONNECTIONtypes.Feature Changes
Everything in this PR is still gated behind the
enable_sql_server_sourceflag. When this flag is enabled, the only feature related change isCREATE CONNECTION ... TO SQL SERVERis now supported!Otherwise purification in the Adapter and implementation of the
SourceRendertrait are left as a TODO which I will include in a follow-up PR.Testing
This PR also adds two tests to exercise the new connection behavior, and will eventually be used to exercise CDC:
tests/sql-server-cdc- testdrive based mzcompose workflows, asserts creating and validating aCONNECTIONis successfultests/platform-checks- a newSqlServerCdccheck is added to ensure creating and dropping connections works across restarts and upgradesMotivation
Related to https://github.com/MaterializeInc/database-issues/issues/9198
Progress towards https://github.com/MaterializeInc/database-issues/issues/8762
Tips for reviewer
Overall a lot of this is just plumbing and cargo culting what already exists for Postgres and MySql. I would focus on the changes in the
storage-typescrate, that's where the most net new code is.Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.