Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

getRowsColumnRange always has cells grouped by row #4687

Merged
merged 10 commits into from
Apr 1, 2020

Conversation

mswintermeyer
Copy link
Contributor

Ensure that throughout the iterator manipulation that happens
internally to getRowsColumnRange that we maintain the invariant
that all cells for a given row will be grouped together at the end.

Goals (and why):
After #4668 (though maybe before as well if the BatchSizeIncreasingIterator didn't kick in?), getRowsColumnRange could return an iterator where the cells for a given row are not always grouped together, which could break assumptions made by clients.

Implementation Description (bullets):
Two potential implementation options, and I'm curious what people think is cleanest: we can sort the rows passed in, so that they're in the same order that we internally sort Cells in, or we should purely rely on the ordering provided by the KVS.

Testing (What was existing testing like? What have you done to improve it?):
Changing the existing tests to shuffle the order of rows passed in, which was enough to break things somewhat. But that combined with having rows not have a cell count that's a multiple of the number of the batch size is where the test demonstrates what could could actually go bad.

Concerns (what feedback would you like?):
What option to fix this is cleanest?

Where should we start reviewing?:

Priority (whenever / two weeks / yesterday):

Ensure that throughout the iterator manipulation that happens
internally to getRowsColumnRange that we maintain the invariant
that all cells for a given row will be grouped together at the end.
@changelog-app
Copy link

changelog-app bot commented Mar 30, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Fixes a bug in 0.198.9. Transaction#getRowsColumnRange (the 4 parameter version) will now properly return cells for a row all grouped together in one batch (no cells from other rows mixed in).


Check the box to generate changelog(s)

  • Generate changelog entry

@mswintermeyer
Copy link
Contributor Author

After taking another look, I think I'm more decided on fixing this by ensuring the rows stay returned in their original order, rather than by trying sorting the rows as they're passed in. Some callers pass in a List of rows and assume that the resulting cells will come back in the same row-order.

@mswintermeyer mswintermeyer merged commit e1ac1ac into develop Apr 1, 2020
@delete-merged-branch delete-merged-branch bot deleted the michaelw/fixGetRowsColumnRangeRowGrouping branch April 1, 2020 17:55
@svc-autorelease
Copy link
Collaborator

Released 0.202.2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants