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

feat: support partitioned dml in dbapi #1103

Merged
merged 5 commits into from Feb 20, 2024
Merged

feat: support partitioned dml in dbapi #1103

merged 5 commits into from Feb 20, 2024

Conversation

ankiaga
Copy link
Contributor

@ankiaga ankiaga commented Feb 15, 2024

No description provided.

@ankiaga ankiaga requested review from a team as code owners February 15, 2024 07:38
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Feb 15, 2024
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Feb 15, 2024
@olavloite olavloite changed the title feat: Implementation to support executing partitioned dml query at dbapi feat: support partitioned dml in dbapi Feb 19, 2024
@@ -540,7 +550,8 @@ def partition_query(
return partition_ids

@check_not_closed
def run_partition(self, encoded_partition_id):
def run_partition(self, parsed_statement: ParsedStatement):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a breaking change of a public method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted

@@ -565,6 +576,24 @@ def run_partitioned_query(
partitioned_query, statement.params, statement.param_types
)

@check_not_closed
def set_autocommit_dml_mode(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that it would make more sense to have a public method that takes a AutocommitDmlMode enum value as an argument, rather than one that takes a parsed statement as an argument. This method looks like a private method that should only be called by our own parser.

Instead, we should:

  1. Have a private method that is basically the same as this.
  2. Have a public method that takes an AutocommitDmlMode enum as an input argument and that actually changes the value of the flag. That method would then also be usable for anyone using the dbapi driver programmatically, as they could just call that method to change the value.

See https://github.com/googleapis/java-spanner-jdbc/blob/910a130a02f72c4d8764f12e347f6c3d1bd51b2b/src/main/java/com/google/cloud/spanner/jdbc/CloudSpannerJdbcConnection.java#L136 for how the API in the JDBC driver looks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

google/cloud/spanner_dbapi/connection.py Show resolved Hide resolved
google/cloud/spanner_dbapi/connection.py Outdated Show resolved Hide resolved
@ankiaga ankiaga enabled auto-merge (squash) February 20, 2024 13:09
@ankiaga ankiaga merged commit 3aab0ed into main Feb 20, 2024
13 of 16 checks passed
@ankiaga ankiaga deleted the partitioned_dml branch February 20, 2024 14:01
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/python-spanner API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants