This repository was archived by the owner on Mar 4, 2026. It is now read-only.
feat: support promise based signatures for execute method#2301
Merged
alkatrivedi merged 8 commits intomainfrom May 12, 2025
Merged
feat: support promise based signatures for execute method#2301alkatrivedi merged 8 commits intomainfrom
alkatrivedi merged 8 commits intomainfrom
Conversation
af2afc9 to
872fe6e
Compare
98006b4 to
8d7d5d7
Compare
| >; | ||
|
|
||
| type executeResponse = | ||
| | [Rows] // For read |
Contributor
There was a problem hiding this comment.
We can instead use
type executeResponse = ReadResponse | RunResponse ;
nit: ExecuteResponse
Why are we not exporting this type ?
Contributor
Author
There was a problem hiding this comment.
I have removed it and have used Promise<ReadResponse | RunResponse>
instead of Promise<ExecuteResponse>
| execute(partition, callback) { | ||
| if (is.string(partition.table)) { | ||
| this.read(partition.table, partition, callback); | ||
| execute(partition: ReadRequest | ExecuteSqlRequest): Promise<executeResponse>; |
Contributor
There was a problem hiding this comment.
Also modify the documentation as per the response type
|
|
||
| let row_count = 0; | ||
| try { | ||
| const [partitions] = |
|
|
||
| let read_row_count = 0; | ||
| try { | ||
| const [partitions] = await transaction.createReadPartitions(QUERY); |
| execute( | ||
| partition: ReadRequest | ExecuteSqlRequest, | ||
| cb?: ReadCallback | RunCallback, | ||
| ): void | Promise<executeResponse> { |
Contributor
There was a problem hiding this comment.
We may need to create ExecuteResponse instead can use ReadResponse | RunResponse here
Contributor
Author
There was a problem hiding this comment.
refactored, thanks for the suggestion!
surbhigarg92
approved these changes
May 12, 2025
| * @see [`ExecuteSqlRequest`](https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.v1#google.spanner.v1.ExecuteSqlRequest) | ||
| * @typedef {object} QueryPartition | ||
| * @property {string} partitionToken The partition token. | ||
| * @property {string} partitionToken The partition token that identifies this partition. Used when executing the partition later. |
Contributor
There was a problem hiding this comment.
Suggested change
| * @property {string} partitionToken The partition token that identifies this partition. Used when executing the partition later. | |
| * @property {string} partitionToken A token representing the partition, used to identify and execute the partition at a later time. |
| * @callback CreateQueryPartitionsCallback | ||
| * @param {?Error} err Request error, if any. | ||
| * @param {QueryPartition[]} partitions List of query partitions. | ||
| * @param {ExecuteSqlRequest[]} partitions Array of ExecuteSqlRequest. |
Contributor
There was a problem hiding this comment.
Suggested change
| * @param {ExecuteSqlRequest[]} partitions Array of ExecuteSqlRequest. | |
| * @param {ExecuteSqlRequest[]} partitions Array of ExecuteSqlRequest partitions. |
Contributor
There was a problem hiding this comment.
Fix the documentation in CreateQueryPartitionsResponse as well
Comment on lines
+152
to
+153
| * @returns {Promise<CreateQueryPartitionsResponse>|void} A promise resolving to an array containing the list of | ||
| * `ExecuteSqlRequest's and the full API response, or void if a callback is provided. |
Contributor
There was a problem hiding this comment.
Suggested change
| * @returns {Promise<CreateQueryPartitionsResponse>|void} A promise resolving to an array containing the list of | |
| * `ExecuteSqlRequest's and the full API response, or void if a callback is provided. | |
| * @returns {Promise<CreateQueryPartitionsResponse>|void} A promise resolving to an array of | |
| * `ExecuteSqlRequest' partitions and `IPartitionResponse` , or void if a callback is provided. |
| * @param {TransactionRequestReadCallback|RunCallback} [callback] Callback | ||
| * function. | ||
| * @returns {Promise<RunResponse>|Promise<TransactionRequestReadResponse>} | ||
| * @returns {Promise<RunResponse | RunResponse>|void} Returns a promise when a callback is provided, |
Contributor
There was a problem hiding this comment.
Suggested change
| * @returns {Promise<RunResponse | RunResponse>|void} Returns a promise when a callback is provided, | |
| * @returns {Promise<ReadResponse | RunResponse>|void} Returns a promise when a callback is provided, |
| * @param {ReadCallback|RunCallback} [callback] Optional Callback function. If not provided, | ||
| * a promise will be returned. | ||
| * | ||
| * If the partition is a `ReadPartition`, it will execute a read using {@link Transaction#read} |
Contributor
There was a problem hiding this comment.
What is ReadPartition here, a link is also used below.
Contributor
Author
There was a problem hiding this comment.
what I meant with this was if a partition is read partition
updated the link with ReadRequest
0d5f13c to
5dc7fca
Compare
…ateReadPartitions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this PR contains code changes for supporting promise based signatures for execute method in BatchTransaction