search

babelfish-for-postgresql / babelfish_extensions

Babelfish for PostgreSQL provides the capability for PostgreSQL to work with applications written for Microsoft SQL Server. Babelfish understands the SQL Server wire-protocol and T-SQL, the Microsoft SQL Server query and procedural language, so you don’t have to switch database drivers or rewrite all of your application queries.
https://babelfishpg.org/
Apache License 2.0
277 stars 93 forks source link

Fix Error Handling and Cleanup during Insert Bulk Process #2887

Closed KushaalShroff closed 2 months ago

KushaalShroff commented 2 months ago

Description

This commit fixes an issue with the error handling and cleanup phase of the Insert Bulk Process.

  1. For Error Handling there was a scenario where bulk_load_callback(0, 0, NULL, NULL) call for cleanup would flush the remaining rows during EndBulkCopy and could result in an error. To fix this, we move the transaction rollback logic from TSQL extension to TDS. We also improved it by using Savepoints in case of active transaction which is aligned with TSQL Behaviour
  2. In this case, if we have a reset-connection after the Bulk Load TDS packet we werent cleaning up the Bulk Load state. To do so we reset the offset.
  3. During Reset Connection TDS is not resetting any TSQL transaction semantic. To resolve this we introduce a wrapper of AbortOutOfAnyTransaction to reset NestedTranCount.

Issues Resolved BABEL-5200, BABEL-5199, BABEL-5220

Authored-by: Kushaal Shroff kushaal@amazon.com Signed-off-by: Kushaal Shroff kushaal@amazon.com

Test Scenarios Covered

  1. Testing explicit transaction (error case handled in 5.) a. Commit without error b. Rollback without error
  2. Index with without transaction
  3. Primary Key error case
  4. Unique constraint with error case
  5. Check constraint with error case a. transaction testing during error scenarios b. @@trancount test - error should not terminate transaction c. Test CheckConstraint BCP Option Enabled d. Test Reusing the same connection for BCP even after error scenarios
  6. Reset-connection testing with Primary Key error The above tests test the seq and index.

For Reset-connection, Although the tests in 6. do reset but for validation through logs I added a temp log to debug TdsResetConnection and with this log its evident that pid 21384 is being reset and reused for Bulk Load

2024-08-27 10:52:28.688 UTC [21384] CONTEXT:  TDS Protocol: Message Type: SQL BATCH, Phase: TDS_REQUEST_PHASE_FETCH. Resetting the TDS connection
2024-08-27 10:52:28.696 UTC [21384] ERROR:  duplicate key value violates unique constraint "destinationtable_pkey"
2024-08-27 10:52:28.696 UTC [21384] DETAIL:  Key (c1)=(2) already exists.
2024-08-27 10:52:28.696 UTC [21384] CONTEXT:  TDS Protocol: Message Type: Bulk Load, Phase: TDS_REQUEST_PHASE_PROCESS. Processing Bulk Load Request
2024-08-27 10:52:28.697 UTC [21384] LOG:  BULK LOAD RESET NOW
2024-08-27 10:52:28.697 UTC [21384] CONTEXT:  TDS Protocol: Message Type: SQL BATCH, Phase: TDS_REQUEST_PHASE_FETCH. Resetting the TDS connection
2024-08-27 10:52:28.706 UTC [21384] ERROR:  duplicate key value violates unique constraint "destinationtable_pkey"
2024-08-27 10:52:28.706 UTC [21384] DETAIL:  Key (c1)=(2) already exists.
2024-08-27 10:52:28.706 UTC [21384] CONTEXT:  TDS Protocol: Message Type: Bulk Load, Phase: TDS_REQUEST_PHASE_PROCESS. Processing Bulk Load Request
2024-08-27 10:52:28.707 UTC [21384] LOG:  BULK LOAD RESET NOW
2024-08-27 10:52:28.707 UTC [21384] CONTEXT:  TDS Protocol: Message Type: SQL BATCH, Phase: TDS_REQUEST_PHASE_FETCH. Resetting the TDS connection
2024-08-27 10:52:28.712 UTC [21384] ERROR:  duplicate key value violates unique constraint "destinationtable_pkey"
2024-08-27 10:52:28.712 UTC [21384] DETAIL:  Key (c1)=(2) already exists.
2024-08-27 10:52:28.712 UTC [21384] CONTEXT:  TDS Protocol: Message Type: Bulk Load, Phase: TDS_REQUEST_PHASE_PROCESS. Processing Bulk Load Request
2024-08-27 10:52:28.714 UTC [21384] LOG:  BULK LOAD RESET NOW
2024-08-27 10:52:28.714 UTC [21384] CONTEXT:  TDS Protocol: Message Type: SQL BATCH, Phase: TDS_REQUEST_PHASE_FETCH. Resetting the TDS connection
2024-08-27 10:52:28.719 UTC [21384] ERROR:  duplicate key value violates unique constraint "destinationtable_pkey"
2024-08-27 10:52:28.719 UTC [21384] DETAIL:  Key (c1)=(2) already exists.
2024-08-27 10:52:28.719 UTC [21384] CONTEXT:  TDS Protocol: Message Type: Bulk Load, Phase: TDS_REQUEST_PHASE_PROCESS. Processing Bulk Load Request
2024-08-27 10:52:28.720 UTC [21384] LOG:  BULK LOAD RESET NOW
2024-08-27 10:52:28.720 UTC [21384] CONTEXT:  TDS Protocol: Message Type: SQL BATCH, Phase: TDS_REQUEST_PHASE_FETCH. Resetting the TDS connection

Check List

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 10629449983

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
contrib/babelfishpg_tds/src/backend/tds/tdsbulkload.c 127 146 86.99%
<!-- Total: 137 156 87.82% -->
Files with Coverage Reduction New Missed Lines %
contrib/babelfishpg_tds/src/backend/tds/tdsutils.c 3 73.8%
contrib/babelfishpg_tds/src/backend/tds/tdscomm.c 3 76.03%
contrib/babelfishpg_tds/src/backend/tds/tdsbulkload.c 3 76.16%
contrib/babelfishpg_tsql/src/session.c 6 96.77%
contrib/babelfishpg_tsql/src/databasepropertyex.c 11 69.16%
contrib/babelfishpg_tsql/src/collation.c 94 81.27%
contrib/babelfishpg_common/src/collation.c 94 79.53%
contrib/babelfishpg_tds/src/backend/tds/tdslogin.c 97 76.02%
contrib/babelfishpg_tsql/src/dbcmds.c 101 74.8%
contrib/babelfishpg_tsql/src/pl_handler.c 107 90.15%
<!-- Total: 817 -->
Totals Coverage Status
Change from base Build 10571839540: 0.2%
Covered Lines: 44637
Relevant Lines: 60143
💛 - Coveralls