Skip to content

chore: include client statement type in parsed statement#1849

Merged
gcf-merge-on-green[bot] merged 5 commits intomainfrom
include-client-statement-type
Apr 28, 2022
Merged

chore: include client statement type in parsed statement#1849
gcf-merge-on-green[bot] merged 5 commits intomainfrom
include-client-statement-type

Conversation

@olavloite
Copy link
Collaborator

Include the ClientSideStatementType in ParsedStatement. This enables
clients to directly check what type of statement it is, which removes
the need for textual checks.

This will for example remove these types of checks: https://github.com/GoogleCloudPlatform/pgadapter/blob/2a3f2eb7cd16cc3c71ffc673544ca9e2baaba21c/src/main/java/com/google/cloud/spanner/pgadapter/statements/IntermediateStatement.java#L354

Include the ClientSideStatementType in ParsedStatement. This enables
clients to directly check what type of statement it is, which removes
the need for textual checks.
@olavloite olavloite requested review from a team April 22, 2022 14:34
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner API. labels Apr 22, 2022
"name": "SHOW VARIABLE AUTOCOMMIT",
"executorName": "ClientSideStatementNoParamExecutor",
"resultType": "RESULT_SET",
"statementType": "SHOW_AUTOCOMMIT",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we seem to have added extra indentation to all modified lines in this file, please check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm... That file needs a small cleanup. It uses both tabs AND spaces for indentation (first indentation is one tab, then second indentation is two spaces).
I've made this change consistent with the current format of the file. Let's do the cleanup in a separate PR.

Copy link
Contributor

@ansh0l ansh0l left a comment

Choose a reason for hiding this comment

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

LGTM with minor nit

@olavloite olavloite added the automerge Merge the pull request once unit tests and other checks pass. label Apr 28, 2022
@gcf-merge-on-green gcf-merge-on-green bot merged commit 6e1aa8e into main Apr 28, 2022
@gcf-merge-on-green gcf-merge-on-green bot deleted the include-client-statement-type branch April 28, 2022 14:20
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the googleapis/java-spanner API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants