Skip to content

Bigtable ReadRows doesn't handle empty string column qualifier #4252

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.

Sign up for GitHub

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

Merged
dhermes merged 4 commits into from
Nov 2, 2017
+38 −4
Merged

Bigtable ReadRows doesn't handle empty string column qualifier #4252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7226577
Add test case for empty string qualifier.
garye Oct 25, 2017
b1b8297
Small tweaks during review of #4252.
dhermes Oct 25, 2017
670fded
Lint fix (moved a comment).
dhermes Oct 25, 2017
ff576b6
Properly set qualifier when encountering new cell.
garye Nov 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions bigtable/google/cloud/bigtable/row_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,14 @@ def consume_next(self):
row = self._row = PartialRowData(chunk.row_key)

if cell is None:
qualifier = None
if chunk.HasField('qualifier'):
qualifier = chunk.qualifier.value

cell = self._cell = PartialCellData(
chunk.row_key,
chunk.family_name.value,
chunk.qualifier.value,
qualifier,
chunk.timestamp_micros,
chunk.labels,
chunk.value)
Expand Down Expand Up @@ -421,7 +425,8 @@ def _copy_from_previous(self, cell):
cell.row_key = previous.row_key
if not cell.family_name:
cell.family_name = previous.family_name
if not cell.qualifier:
# NOTE: ``cell.qualifier`` **can** be empty string.
if cell.qualifier is None:
cell.qualifier = previous.qualifier

def _save_current_row(self):
Expand Down
27 changes: 27 additions & 0 deletions bigtable/tests/unit/read-rows-acceptance-test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,33 @@
"error": false
}
]
},
{
"name": "empty second qualifier",
"chunks": [
"row_key: \"RK\"\nfamily_name: \u003c\n value: \"A\"\n\u003e\nqualifier: \u003c\n value: \"C\"\n\u003e\ntimestamp_micros: 99\nvalue: \"value-VAL_1\"\ncommit_row: false\n",
"qualifier: \u003c\n value: \"\"\n\u003e\ntimestamp_micros: 98\nvalue: \"value-VAL_2\"\ncommit_row: true\n"
],
"results": [
{
"rk": "RK",
"fm": "A",
"qual": "C",
"ts": 99,
"value": "value-VAL_1",
"label": "",
"error": false
},
{
"rk": "RK",
"fm": "A",
"qual": "",
"ts": 98,
"value": "value-VAL_2",
"label": "",
"error": false
}
]
}
]
}
6 changes: 4 additions & 2 deletions bigtable/tests/unit/test_row_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def test__copy_from_previous_unset(self):
prd._copy_from_previous(cell)
self.assertEqual(cell.row_key, '')
self.assertEqual(cell.family_name, u'')
self.assertEqual(cell.qualifier, b'')
self.assertIsNone(cell.qualifier)
self.assertEqual(cell.timestamp_micros, 0)
self.assertEqual(cell.labels, [])

Expand Down Expand Up @@ -635,6 +635,8 @@ def test_reset_in_between_chunks(self):
def test_empty_cell_chunk(self):
self._match_results('empty cell chunk')

def test_empty_second_qualifier(self):
self._match_results('empty second qualifier')

def _flatten_cells(prd):
# Match results format from JSON testcases.
Expand Down Expand Up @@ -678,7 +680,7 @@ class _PartialCellData(object):

row_key = ''
family_name = u''
qualifier = b''
qualifier = None

This comment was marked as spam.

Sign in to view

timestamp_micros = 0

def __init__(self, **kw):
Expand Down