This repository was archived by the owner on Apr 1, 2026. It is now read-only.
perf: optimize row merging#628
Merged
Merged
Conversation
- extract read-rows-acceptance-test.json based tests into own file - update the json to match the latest available in https://github.com/googleapis/conformance-tests/tree/main/bigtable/v2 - use parameterized pytest test to run all of the scenarios (instead of creating a function for each json blob) - use json protobufs to parse the file I left a TODO to allow easy updates of the file, unfortunately its not straight forward as the canonical protos get renamed for python gapic Next PR will extract row merging functionality from row_data to make it easier to maintain
No region tags are edited in this PR.This comment is generated by snippet-bot.
|
mutianf
suggested changes
Aug 9, 2022
mutianf
approved these changes
Aug 10, 2022
Contributor
|
Can you add the documentation of row_merger to the Sphinx documentation? Then add We should also update the usages of row_data.Cell / row_data.PartialRowData / row_data.InvalidChunk in the documentation to use the row_merger instead. |
igorbernstein2
added a commit
to igorbernstein2/python-bigtable
that referenced
this pull request
Aug 11, 2022
This is in preparation for extracting row merging into a separate class. See googleapis#628
dbolduc
approved these changes
Aug 12, 2022
dbolduc
left a comment
Member
There was a problem hiding this comment.
The row merging logic LGTM. I left some nits that you can fix or not.
Mariatta
pushed a commit
that referenced
this pull request
Aug 17, 2022
* chore: move row value classes out of row_data This is in preparation for extracting row merging into a separate class. See #628 Co-authored-by: Anthonios Partheniou <partheniou@google.com>
# Conflicts: # google/cloud/bigtable/row_data.py
# Conflicts: # google/cloud/bigtable/row_merger.py
Mariatta
approved these changes
Aug 17, 2022
parthea
added a commit
to googleapis/google-cloud-python
that referenced
this pull request
Nov 22, 2025
* chore: move row value classes out of row_data This is in preparation for extracting row merging into a separate class. See googleapis/python-bigtable#628 Co-authored-by: Anthonios Partheniou <partheniou@google.com>
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.
5 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 rewrites the row merging logic to be more correct and improve performance:
{family: { qualifier: [] }}data, this should maintain serverside ordering (family in creation order and qualifier in lexiographical).Overall this improves performance by 20% and in my opinion is a lot more readable