Skip to content

Conversation

@tkyc
Copy link
Contributor

@tkyc tkyc commented Dec 17, 2021

No description provided.

@tkyc tkyc requested a review from VeryVerySpicy December 17, 2021 21:22
@lilgreenbird lilgreenbird marked this pull request as draft January 28, 2022 19:12
@tkyc tkyc marked this pull request as ready for review February 15, 2022 17:24
@lilgreenbird lilgreenbird added this to the 10.3.0 milestone Feb 16, 2022
@tkyc tkyc changed the title Sp prepare Added new connection string property useSpPrepare to toggle use sp_prepare Feb 16, 2022
@tkyc tkyc changed the title Added new connection string property useSpPrepare to toggle use sp_prepare Added new connection string property useSpPrepare to toggle use of sp_prepare Feb 16, 2022
@tkyc tkyc changed the title Added new connection string property useSpPrepare to toggle use of sp_prepare Added new connection string property useSpPrepExec to toggle use of sp_prepare Feb 16, 2022
@David-Engel
Copy link
Collaborator

My only thought as I skimmed this was, is a boolean, useSpPrepExec, a good way to control this option? Or should we use a string value? My thinking it that there might be a different way we want to control preparation in the future and if we use a string value, we can easily add another option without a breaking change.

For example, jTDS has the prepareSQL option with 4 values (it doesn't exactly match up to what we are doing but the idea is similar): http://jtds.sourceforge.net/faq.html

Perhaps "prepareMethod=prepexec|prepare"? (Default=prepexec)

Thoughts?

@otbutz
Copy link

otbutz commented Feb 21, 2022

I'd rather take the approach of jTDS as it allows to add different strategies in the future without breaking compatibility.

@tkyc tkyc force-pushed the sp-prepare branch 3 times, most recently from f841b78 to 0f7a423 Compare February 24, 2022 20:05
Jeffery-Wasty
Jeffery-Wasty previously approved these changes Feb 25, 2022
@lilgreenbird lilgreenbird added the Public API Changes in Public API label Mar 1, 2022
@lilgreenbird lilgreenbird changed the title Added new connection string property useSpPrepExec to toggle use of sp_prepare Added new connection string property prepareMethod to toggle use of sp_prepare Mar 1, 2022
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.

  • can we also add a test that test the property in connection string? (instead of just datasource)
  • we should add this property to updateDataSource in AbstractTest as well
  • we should also add tests for this property when AE is enabled

VeryVerySpicy
VeryVerySpicy previously approved these changes Mar 1, 2022
@tkyc tkyc dismissed stale reviews from VeryVerySpicy and Jeffery-Wasty via e240e68 March 2, 2022 00:15
@tkyc tkyc merged commit d3cb66e into microsoft:main Mar 4, 2022
@otbutz
Copy link

otbutz commented Aug 9, 2022

@Jeffery-Wasty
Copy link
Contributor

Hi @otbutz,

We're adding this under Prepared statement metadata caching for the JDBC driver. The proposed changes are currently under review by the MS Docs team, but expect the docs to be updated soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Public API Changes in Public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance difference in prepared statements compared to jTDS due to differing execution plan

6 participants