Bugfix: Allow Bigtable ReadRows to handle empty string column qualifier (googleapis#4252)

garye · dhermes · commit 49e744c81c50 · 2017-11-02T12:43:18.000-07:00
diff --git a/bigtable/google/cloud/bigtable/row_data.py b/bigtable/google/cloud/bigtable/row_data.py
@@ -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)
@@ -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):
diff --git a/bigtable/tests/unit/read-rows-acceptance-test.json b/bigtable/tests/unit/read-rows-acceptance-test.json
@@ -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
+        }
+      ]
     }
   ]
 }
diff --git a/bigtable/tests/unit/test_row_data.py b/bigtable/tests/unit/test_row_data.py
@@ -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, [])
 
@@ -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.
@@ -678,7 +680,7 @@ class _PartialCellData(object):
 
     row_key = ''
     family_name = u''
-    qualifier = b''
+    qualifier = None
     timestamp_micros = 0
 
     def __init__(self, **kw):
