Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the multiplexing feature from Npgsql and simplifies the implementation and test suite accordingly (per #6449, intended to merge after #6314).
Changes:
- Removes multiplexing implementation, configuration surface area, and related infrastructure (data source, connector I/O loops, binding scopes, counters, logging).
- Updates tests to run without
MultiplexingTestBase/MultiplexingModeand deletes multiplexing-only test cases/workarounds. - Updates public API tracking to reflect the removal of
NpgsqlConnectionStringBuilder.MultiplexingandWriteCoalescingBufferThresholdBytes.
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/Npgsql.Tests/Types/TextTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/RecordTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/RangeTests.cs | Removes multiplexing workarounds; switches to TestBase. |
| test/Npgsql.Tests/Types/NumericTypeTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/NumericTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/NetworkTypeTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/MultirangeTests.cs | Removes multiplexing deadlock workaround (Task.Yield). |
| test/Npgsql.Tests/Types/MiscTypeTests.cs | Removes multiplexing skip/branching in tests; switches to TestBase. |
| test/Npgsql.Tests/Types/LegacyDateTimeTests.cs | Removes multiplexing-specific comment/import; remains non-parallelizable. |
| test/Npgsql.Tests/Types/LTreeTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/JsonTests.cs | Removes multiplexing fixtures and constructor parameter; keeps json/jsonb fixtures. |
| test/Npgsql.Tests/Types/JsonPathTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/JsonDynamicTests.cs | Removes multiplexing fixtures and constructor parameter; keeps json/jsonb fixtures. |
| test/Npgsql.Tests/Types/InternalTypeTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/HstoreTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/GeometricTypeTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/FullTextSearchTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/EnumTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/DomainTests.cs | Removes multiplexing ignore and switches to TestBase. |
| test/Npgsql.Tests/Types/DateTimeTests.cs | Removes multiplexing-specific comment. |
| test/Npgsql.Tests/Types/CubeTests.cs | Switches to TestBase and removes multiplexing ctor. |
| test/Npgsql.Tests/Types/CompositeTests.cs | Removes multiplexing-specific transaction workaround; updates comments. |
| test/Npgsql.Tests/Types/ByteaTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/BitStringTests.cs | Switches from multiplexing fixtures/base class to TestBase. |
| test/Npgsql.Tests/Types/ArrayTests.cs | Removes multiplexing ignores and switches to TestBase. |
| test/Npgsql.Tests/TransactionTests.cs | Removes multiplexing branches/ignores; switches to TestBase. |
| test/Npgsql.Tests/TracingTests.cs | Removes multiplexing fixture dimension; simplifies activity assertions to single mode. |
| test/Npgsql.Tests/TestUtil.cs | Removes Multiplexing=False from default test connection string. |
| test/Npgsql.Tests/Support/TestBase.cs | Removes multiplexing mentions and multiplexing-specific tx usage in type assertion helpers. |
| test/Npgsql.Tests/Support/MultiplexingTestBase.cs | Deleted multiplexing test base and MultiplexingMode enum. |
| test/Npgsql.Tests/Support/AssemblySetUp.cs | Removes Multiplexing=false from admin connection settings. |
| test/Npgsql.Tests/SecurityTests.cs | Removes multiplexing parameter dimension and related branching. |
| test/Npgsql.Tests/ReaderTests.cs | Removes multiplexing fixtures/branches/workarounds; simplifies ctor. |
| test/Npgsql.Tests/ReaderOldSchemaTests.cs | Removes commented multiplexing-specific guard. |
| test/Npgsql.Tests/ReaderNewSchemaTests.cs | Removes commented multiplexing ignore. |
| test/Npgsql.Tests/PrepareTests.cs | Removes multiplexing-not-supported test now that feature is removed. |
| test/Npgsql.Tests/PoolTests.cs | Removes multiplexing-only ignored test. |
| test/Npgsql.Tests/LoggingTests.cs | Removes multiplexing branching in logging assertions; switches to TestBase. |
| test/Npgsql.Tests/DataSourceTests.cs | Removes multiplexing dimension; removes multiplexing-specific regression test. |
| test/Npgsql.Tests/CopyTests.cs | Removes multiplexing ignores and updates comments; switches to TestBase. |
| test/Npgsql.Tests/ConnectionTests.cs | Removes multiplexing branches/ignores; switches to TestBase. |
| test/Npgsql.Tests/CommandTests.cs | Removes multiplexing branches/ignores; switches to TestBase. |
| test/Npgsql.Tests/CommandParameterTests.cs | Removes multiplexing branch for prepared parameter test; switches to TestBase. |
| test/Npgsql.Tests/BugTests.cs | Removes multiplexing-only regression test. |
| test/Npgsql.Tests/BatchTests.cs | Removes multiplexing fixture dimension; switches to TestBase. |
| test/Npgsql.Tests/AuthenticationTests.cs | Removes multiplexing-specific connection retry comment; switches to TestBase. |
| test/Npgsql.PluginTests/NodaTimeTests.cs | Switches from multiplexing test base to TestBase; removes multiplexing ctor. |
| src/Npgsql/Util/ManualResetValueTaskSource.cs | Deleted helper type used by multiplexing paths. |
| src/Npgsql/Replication/ReplicationConnection.cs | Removes multiplexing references and disables it implicitly by removal. |
| src/Npgsql/PublicAPI.Unshipped.txt | Marks multiplexing-related connection string builder properties as removed. |
| src/Npgsql/PoolingDataSource.cs | Removes multiplexing-specific asserts/comments around idle connectors and return path. |
| src/Npgsql/NpgsqlTransaction.cs | Removes transaction binding-scope unbinding (multiplexing-related). |
| src/Npgsql/NpgsqlSlimDataSourceBuilder.cs | Removes construction and validation logic for multiplexing data source. |
| src/Npgsql/NpgsqlSchema.cs | Removes temporary binding scope; uses bound connector directly for DataTypes schema. |
| src/Npgsql/NpgsqlRawCopyStream.cs | Removes binding-scope teardown (multiplexing-related). |
| src/Npgsql/NpgsqlMultiHostDataSource.cs | Removes multiplexing assertion in multi-host setup. |
| src/Npgsql/NpgsqlEventSource.cs | Removes multiplexing-specific counters and tracking. |
| src/Npgsql/NpgsqlEventId.cs | Leaves multiplexing-related event IDs but annotates them as removed feature. |
| src/Npgsql/NpgsqlDataReader.cs | Removes multiplexing-specific cleanup/unbinding behavior; simplifies send-task comment. |
| src/Npgsql/NpgsqlConnectionStringBuilder.cs | Removes Multiplexing and WriteCoalescingBufferThresholdBytes settings and validation. |
| src/Npgsql/NpgsqlConnection.cs | Removes connector binding scope infrastructure and multiplexing open/close paths. |
| src/Npgsql/NpgsqlCommand.cs | Removes multiplexing execution branch and related channel/value-task plumbing; simplifies query parsing. |
| src/Npgsql/NpgsqlBinaryImporter.cs | Removes binding-scope teardown (multiplexing-related). |
| src/Npgsql/NpgsqlBinaryExporter.cs | Removes binding-scope teardown (multiplexing-related). |
| src/Npgsql/MultiplexingDataSource.cs | Deleted multiplexing data source implementation. |
| src/Npgsql/LogMessages.cs | Removes multiplexing-specific log messages and event usage. |
| src/Npgsql/Internal/NpgsqlConnector.cs | Removes multiplexing I/O loops, command-in-flight tracking, and multiplexing-only restrictions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // TODO: Support type name restriction | ||
| try | ||
| { |
There was a problem hiding this comment.
GetDataTypes now directly dereferences conn.Connector without first validating the connection state. If GetSchema("DataTypes") is called on a closed/broken connection, this will throw a NullReferenceException instead of the expected InvalidOperationException. Add an explicit state check (e.g., conn.CheckReady()) before accessing conn.Connector (or otherwise handle the closed-connection case consistently with other schema collections).
Suggested change
| { | |
| { | |
| conn.CheckReady(); |
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
1 participant
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.
Merge only after #6314.
Closes #6449