Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FIX] PreparedStatement.executeBatch() When the Insert Sql Column Name Case Mismatch, Throws BatchUpdateException "Unable to retrieve column metadata." #2589

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[FIX] PreparedStatement.executeBatch() When the Insert Sql Column Name Case Mismatch, Throws BatchUpdateException "Unable to retrieve column metadata." #2589

wants to merge 10 commits into from

Conversation

wooln
Copy link

@wooln wooln commented Jan 21, 2025 •
edited
Loading

fix #2588 [BUG] PreparedStatement.executeBatch() When the Insert Sql Column Name Case Mismatch, Throws BatchUpdateException "Unable to retrieve column metadata."

@wooln
Copy link
Author

wooln commented Jan 21, 2025

@microsoft-github-policy-service agree

@wooln wooln mentioned this pull request Jan 22, 2025
Closed
@lilgreenbird
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

lilgreenbird
lilgreenbird previously requested changes Jan 23, 2025
View reviewed changes
Copy link
Contributor

@lilgreenbird lilgreenbird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix imports and test cleanup

wooln added 2 commits January 23, 2025 12:55
@wooln
test: add tests for column case sensitivity in batch execution
7e66b07 
- Add test case for matching column case in batch execution
- Add test case for mismatched column case in batch execution, expected SQLServerException
@wooln
fix(batch): improve column metadata handling for batch execution
902ac88 
- Update column index search to be case-insensitive in SQLServerPreparedStatement
- Remove failing test case in BatchExecutionTest that was checking for specific exception
@wooln wooln force-pushed the bugfix-execute-batch-case-sensitive branch from 719687f to 902ac88 Compare January 23, 2025 04:56
@lilgreenbird
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

David-Engel
David-Engel reviewed Jan 23, 2025
View reviewed changes
Copy link
Collaborator

@David-Engel David-Engel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to consider:

Does your database use a case-sensitive (like SQL_Latin1_General_CP1_CS_AS) or case-insensitive collation (like SQL_Latin1_General_CP1_CI_AS)?

What happens if you are using a case-sensitive collation and a table like this?

create table [Test1] ([C1] int, [c1] varchar)

@lilgreenbird
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@wooln
feat(collation): add case sensitivity check for database collation
d204251 
- Add getIsCaseSensitive() method to SQLCollation class
- Update SQLServerPreparedStatement to use case sensitivity information
- Improve performance by avoiding unnecessary case-insensitive comparisons
@wooln
Copy link
Author

wooln commented Jan 31, 2025

Something to consider:

Does your database use a case-sensitive (like SQL_Latin1_General_CP1_CS_AS) or case-insensitive collation (like SQL_Latin1_General_CP1_CI_AS)?

What happens if you are using a case-sensitive collation and a table like this?

create table [Test1] ([C1] int, [c1] varchar)

Good idea. I covered the unit tests that use both columns c1 and C1, and an exception occurred, indicating that no one has ever covered CS before. I'll try to fix it by the way.

com.microsoft.sqlserver.jdbc.SQLServerException: Duplicate column names are not allowed.

	at com.microsoft.sqlserver.jdbc.SQLServerBulkRecord.checkDuplicateColumnName(SQLServerBulkRecord.java:157)
	at com.microsoft.sqlserver.jdbc.SQLServerBulkBatchInsertRecord.addColumnMetadataInternal(SQLServerBulkBatchInsertRecord.java:323)
	at com.microsoft.sqlserver.jdbc.SQLServerBulkRecord.addColumnMetadata(SQLServerBulkRecord.java:83)
	at com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement.executeBatch(SQLServerPreparedStatement.java:2232)
	at com.microsoft.sqlserver.jdbc.unit.statement.BatchExecutionTest.testExecuteBatchColumnCaseMismatch_CS(BatchExecutionTest.java:632)

@wooln
fix(batch): handle case sensitivity for column names in batch execution
2d3f115 
- Add support for case-sensitive column names in SQLServerBulkBatchInsertRecord
- Update SQLServerBulkRecord to handle case sensitivity in column metadata
- Modify SQLServerPreparedStatement to pass case sensitivity information to batch records
- Add tests for case-sensitive and case-insensitive column name handling in batch execution
@wooln wooln force-pushed the bugfix-execute-batch-case-sensitive branch from ae48902 to 2d3f115 Compare January 31, 2025 13:43
@wooln
Copy link
Author

wooln commented Feb 6, 2025

/azp run

@azure-pipelines Azure Pipelines
Copy link

Commenter does not have sufficient privileges for PR 2589 in repo microsoft/mssql-jdbc

@Jeffery-Wasty Jeffery-Wasty added this to the 12.10.0 milestone Feb 11, 2025
@wooln wooln requested a review from David-Engel February 13, 2025 00:56
@wooln
Copy link
Author

wooln commented Feb 19, 2025

/azp run

@azure-pipelines Azure Pipelines
Copy link

Commenter does not have sufficient privileges for PR 2589 in repo microsoft/mssql-jdbc

@wooln wooln force-pushed the bugfix-execute-batch-case-sensitive branch from aebf086 to 6408cc4 Compare February 19, 2025 02:17
@wooln wooln changed the title [FIX] PreparedStatement.executeBatch() When the Insert Sql Column Name Case Mismatch, Throws SQLServerException "Unable to retrieve column metadata." [FIX] PreparedStatement.executeBatch() When the Insert Sql Column Name Case Mismatch, Throws BatchUpdateException "Unable to retrieve column metadata." Feb 19, 2025
@wooln
Copy link
Author

wooln commented Feb 19, 2025

@lilgreenbird /azp run, please.

@Jeffery-Wasty Jeffery-Wasty modified the milestones: 12.10.0, 12.11.0 Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmimica mmimica mmimica left review comments

@Jeffery-Wasty Jeffery-Wasty Awaiting requested review from Jeffery-Wasty

@tkyc tkyc Awaiting requested review from tkyc

@divang divang Awaiting requested review from divang

@Ananya2 Ananya2 Awaiting requested review from Ananya2

@muskan124947 muskan124947 Awaiting requested review from muskan124947

@machavan machavan Awaiting requested review from machavan

@lilgreenbird lilgreenbird Awaiting requested review from lilgreenbird

@David-Engel David-Engel Awaiting requested review from David-Engel

At least 2 approving reviews are required to merge this pull request.

Assignees

@wooln wooln

Labels
None yet
Projects
MSSQL JDBC
Status: Under Peer Review
Milestone
12.11.0
5 participants
@wooln @lilgreenbird @mmimica @David-Engel @Jeffery-Wasty