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.

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

Execute Stored Procedures Directly #2154

Merged
merged 52 commits into from
Nov 16, 2023
Merged

Execute Stored Procedures Directly #2154

merged 52 commits into from
Nov 16, 2023

Conversation

tkyc
Copy link
Contributor

@tkyc tkyc commented Jun 28, 2023

Continued work from #547.

Points of interest:

  1. In order to use parameter names and indexes interchangeably for acquiring/setting parameter eg. cstmt.getXXX or cstmt.registerOutputParameter, the driver will need to continue to use metadata query to get the parameter names when the new CS property useFlexibleCallableStatements=true (default behaviour).

  2. xp_sqljdbc_xa calls have a special case registering their output parameters for CHAR and BINARY types as non-PLP. Prior to this PR, because cstmts calls were wrapped the RPC parameters sent to the server were defined within the sp_execute calls and so it wasn't necessary to do so.

  3. To read the output parameter return values, parsing the tds response is deferred to the getXXX calls. So, if a user executes a cstmt but doesn't get/read the output params, the return values in the tds response will be in limbo/waiting for resolution. In such case, on statement close we will now skip these return values in the tds response which is safe to do.

  4. Prior to the PR, it was possible to get the return value from sprocs as a byte array. This shouldn't be possible and this is corrected in this PR. It will throw a casting exception.

  5. To allow for unordered setting/registering of parameters, we'll need to continue to use a metadata query for the parameter names (this goes along with point 1 when useFlexibleCallableStatements=true).

  6. If outParams are null then likely the statement was closed, so we throw an exception.

  7. Potentially we can skip building the type definitions as this is no longer needed since we send the RPC params directly. However, this breaks AE and so needs more investigation.

  8. For user defined functions (UDF) to work, we need to know when the TDS's response return value is for a sproc or a UDF. To do so, new logic was added to parse the TDS's response to determine this.

  9. On the main branch for datetime (datetime, datetime2), the RPC call would always send the parameter as datetime2(7). However, since the cstmts are wrapped, the real param definition was set in the sp_executesql/sp_prepexec by building the type definitions in buildParamTypeDefinitions which are affected by the datetimeParameterType CS property. Since we are executing cstmts directly, datetime (datetime, datetime2) will now do the RPC with the exact datetime parameter definition. Meaning, depending on the datetimeParameterType CS property, datetime2 will be sent in the RPC as datetime2(3) exactly by default (which was the prior behaviour done by/indirectly set in the build type definitions) or sent plainly as a datetime.

  10. New CS property useFlexibleCallableStatements. Default behaviour is useFlexibleCallableStatements=true, and in such case the driver will continue to use sp_sproc_columns. Otherwise, when useFlexibleCallableStatements=false, setting/registering parameters for callable statements require the following:

  • a) Setting/Registering callable statement parameters must be exclusively by index or exclusively by parameter name. A mix of index and parameter names will not work.
  • b) Setting/Registering callable statement parameters must be in same order as the sproc definition. Setting the parameters in an adhoc order will not work.
  • c) Sprocs with cursorable methods will continue as is with sp_sproc_calls regardless of each new CS prop scenario.

@tkyc tkyc marked this pull request as ready for review November 2, 2023 21:20
Jeffery-Wasty
Jeffery-Wasty previously approved these changes Nov 15, 2023
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.

Please add tests that exercise CallableStatement with useFlexibleCallableStatements set to false and validate success and failure cases. We should detect if they are mixing named/index and throw an error. Not sure if we can detect out of order params.

tkyc and others added 4 commits November 15, 2023 15:31
…e.java

Co-authored-by: David Engel <v-davidengel@microsoft.com>
…atement.java

Co-authored-by: David Engel <v-davidengel@microsoft.com>
…java

Co-authored-by: David Engel <v-davidengel@microsoft.com>
…atement.java

Co-authored-by: David Engel <v-davidengel@microsoft.com>
Jeffery-Wasty
Jeffery-Wasty previously approved these changes Nov 16, 2023
@tkyc tkyc merged commit 11680a6 into main Nov 16, 2023
17 checks passed
@Jeffery-Wasty Jeffery-Wasty deleted the exec-cstmt-directly branch January 9, 2024 22:24
tkyc added a commit that referenced this pull request Jul 11, 2024
Jeffery-Wasty added a commit that referenced this pull request Aug 6, 2024
Jeffery-Wasty added a commit that referenced this pull request Aug 6, 2024
tkyc added a commit that referenced this pull request Aug 13, 2024
tkyc added a commit that referenced this pull request Aug 21, 2024
* Revert "Execute Stored Procedures Directly (#2154)"

This reverts commit 11680a6.

* Revert "Execute cstmt directly - Additional testing and changes (#2284)"

This reverts commit 92cfe0d.

* Revert "Re-added support for stored procedure 'exec' escape syntax in CallableStatements (#2325)"

This reverts commit ba88da8.

* Additional revert of missed lines

* Added no-op for getters/setters

* RequestBoundaryMethods no-op test fix
tkyc added a commit that referenced this pull request Aug 21, 2024
* Revert "Execute Stored Procedures Directly (#2154)"

This reverts commit 11680a6.

* Revert "Execute cstmt directly - Additional testing and changes (#2284)"

This reverts commit 92cfe0d.

* Revert "Re-added support for stored procedure 'exec' escape syntax in CallableStatements (#2325)"

This reverts commit ba88da8.

* Additional revert of missed lines

* Added no-op for getters/setters

* RequestBoundaryMethods no-op test fix
tkyc added a commit that referenced this pull request Aug 22, 2024
* Revert "Execute Stored Procedures Directly (#2154)"

This reverts commit 11680a6.

* Revert "Execute cstmt directly - Additional testing and changes (#2284)"

This reverts commit 92cfe0d.

* Revert "Re-added support for stored procedure 'exec' escape syntax in CallableStatements (#2325)"

This reverts commit ba88da8.

* Additional revert of missed lines

* Added no-op for getters/setters

* RequestBoundaryMethods no-op test fix
tkyc added a commit that referenced this pull request Aug 23, 2024
* Revert "Execute Stored Procedures Directly (#2154)"

This reverts commit 11680a6.

* Revert "Execute cstmt directly - Additional testing and changes (#2284)"

This reverts commit 92cfe0d.

* Revert "Re-added support for stored procedure 'exec' escape syntax in CallableStatements (#2325)"

This reverts commit ba88da8.

* Additional revert of missed lines

* Added no-op for getters/setters

* RequestBoundaryMethods no-op test fix
tkyc added a commit that referenced this pull request Aug 28, 2024
* Revert "Execute Stored Procedures Directly (#2154)"

This reverts commit 11680a6.

* Revert "Execute cstmt directly - Additional testing and changes (#2284)"

This reverts commit 92cfe0d.

* Revert "Re-added support for stored procedure 'exec' escape syntax in CallableStatements (#2325)"

This reverts commit ba88da8.

* Additional revert of missed lines

* Added no-op for getters/setters

* RequestBoundaryMethods no-op test fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

Extra metadata lookup for stored procedures with named parameters
4 participants