Skip to content

Revert "Handled sp_prepexec call for Column encryption enabled (#2663)" #2675

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.

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

Merged
merged 1 commit into from
Jun 9, 2025

Conversation

divang
Copy link
Contributor

@divang divang commented Jun 9, 2025

This reverts commit 52303d8.

Issues Summary:
The incident involved unexpected behavior when inserting using Always Encrypted with Secure Enclaves in Azure SQL DB.

Issue in details at code level:
The doExecutePreparedStatement() method invokes buildPreparedStrings(), which updates the preparedSQL and preparedTypeDefinitions, and also returns a flag indicating whether preparedTypeDefinitions was modified.
The issue arises because the method is expected to use only the result of the first buildPreparedStrings() call to determine the hasNewTypeDefinitions flag. This flag is crucial, as it helps decide whether to initiate the sp_prepexec call—based on its value in combination with other conditions.
However, the current implementation updates the hasNewTypeDefinitions variable on each buildPreparedStrings() call. This leads to incorrect behavior: if later calls do not indicate a type definition change, the earlier (valid) indication of change is overwritten, and the sp_prepexec call is incorrectly skipped when parameter type definitions actually change.

Revert reason:
In the buildPreparedStrings method, caching the preparedTypeDefinitions variable in preparedTypeDefinitionsPrev to prevent it from being overwritten is both unnecessary and not an appropriate solution to the underlying issue. A more accurate and effective resolution has been identified (below one) and will be implemented in a follow-up PR.

Resolution details:
The issue was resolved by ensuring that the hasNewTypeDefinitions flag is set only once, based on the result of the first call to buildPreparedStrings().
Subsequent calls to buildPreparedStrings() are now used solely to update the preparedSQL variable and are no longer allowed to overwrite the hasNewTypeDefinitions flag. This guarantees that any change in parameter type definitions detected during the first evaluation is preserved, ensuring that the sp_prepexec call is correctly triggered when required.
This change aligns with the intended design and prevents silent failures due to skipped sp_prepexec calls when parameter type definitions change.

@divang divang marked this pull request as ready for review June 9, 2025 05:54
Copy link

codecov bot commented Jun 9, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 51.92%. Comparing base (52303d8) to head (584ca67).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 66.66% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2675      +/-   ##
============================================
+ Coverage     51.62%   51.92%   +0.29%     
- Complexity     4006     4059      +53     
============================================
  Files           147      147              
  Lines         33804    33800       -4     
  Branches       5652     5650       -2     
============================================
+ Hits          17453    17549      +96     
+ Misses        13888    13820      -68     
+ Partials       2463     2431      -32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@divang divang merged commit f455ac9 into main Jun 9, 2025
19 checks passed
@divang divang deleted the users/divang/revert-pr-2663-column-encryption branch June 11, 2025 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants