fix(bigtable): encode str qualifier in DeleteRangeFromColumn and allow -2**63 in int mutations

packages/google-cloud-bigtable/google/cloud/bigtable/data/mutations.py

@@ -22,7 +22,10 @@
 
 import google.cloud.bigtable_v2.types.bigtable as types_pb
 import google.cloud.bigtable_v2.types.data as data_pb
-from google.cloud.bigtable.data.read_modify_write_rules import _MAX_INCREMENT_VALUE
+from google.cloud.bigtable.data.read_modify_write_rules import (
+    _MAX_INCREMENT_VALUE,
+    _MIN_INCREMENT_VALUE,
+)
 
 # special value for SetCell mutation timestamps. If set, server will assign a timestamp
 _SERVER_SIDE_TIMESTAMP = -1
@@ -169,10 +172,12 @@ def __init__(
         qualifier = qualifier.encode() if isinstance(qualifier, str) else qualifier
         if not isinstance(qualifier, bytes):
             raise TypeError("qualifier must be bytes or str")
+        if isinstance(new_value, bool):
+            raise TypeError("new_value must be bytes, str, or int")
         if isinstance(new_value, str):
             new_value = new_value.encode()
         elif isinstance(new_value, int):
-            if abs(new_value) > _MAX_INCREMENT_VALUE:
+            if not (_MIN_INCREMENT_VALUE <= new_value <= _MAX_INCREMENT_VALUE):
                 raise ValueError(
                     "int values must be between -2**63 and 2**63 (64-bit signed int)"
                 )
@@ -223,13 +228,17 @@ class DeleteRangeFromColumn(Mutation):
     """
 
     family: str
-    qualifier: bytes
+    qualifier: bytes | str
     # None represents 0
     start_timestamp_micros: int | None = None
     # None represents infinity
     end_timestamp_micros: int | None = None
 
     def __post_init__(self):
+        if isinstance(self.qualifier, str):
+            self.qualifier = self.qualifier.encode()
+        if not isinstance(self.qualifier, bytes):
+            raise TypeError("qualifier must be bytes or str")
         if (
             self.start_timestamp_micros is not None
             and self.end_timestamp_micros is not None
@@ -321,11 +330,11 @@ def __init__(
         qualifier = qualifier.encode() if isinstance(qualifier, str) else qualifier
         if not isinstance(qualifier, bytes):
             raise TypeError("qualifier must be bytes or str")
-        if not isinstance(value, int):
+        if isinstance(value, bool) or not isinstance(value, int):
             raise TypeError("value must be int")
-        if not isinstance(timestamp_micros, int):
+        if isinstance(timestamp_micros, bool) or not isinstance(timestamp_micros, int):
             raise TypeError("timestamp_micros must be int")
-        if abs(value) > _MAX_INCREMENT_VALUE:
+        if not (_MIN_INCREMENT_VALUE <= value <= _MAX_INCREMENT_VALUE):
             raise ValueError(
                 "int values must be between -2**63 and 2**63 (64-bit signed int)"
             )

packages/google-cloud-bigtable/google/cloud/bigtable/data/read_modify_write_rules.py

@@ -19,6 +19,7 @@
 import google.cloud.bigtable_v2.types.data as data_pb
 
 # value must fit in 64-bit signed integer
+_MIN_INCREMENT_VALUE = -(1 << 63)
 _MAX_INCREMENT_VALUE = (1 << 63) - 1
 
 
@@ -61,9 +62,9 @@ class IncrementRule(ReadModifyWriteRule):
     """
 
     def __init__(self, family: str, qualifier: bytes | str, increment_amount: int = 1):
-        if not isinstance(increment_amount, int):
+        if isinstance(increment_amount, bool) or not isinstance(increment_amount, int):
             raise TypeError("increment_amount must be an integer")
-        if abs(increment_amount) > _MAX_INCREMENT_VALUE:
+        if not (_MIN_INCREMENT_VALUE <= increment_amount <= _MAX_INCREMENT_VALUE):
             raise ValueError(
                 "increment_amount must be between -2**63 and 2**63 (64-bit signed int)"
             )

packages/google-cloud-bigtable/google/cloud/bigtable/data/row_filters.py

@@ -144,7 +144,7 @@ def __init__(self, regex: str | bytes):
         self.regex: bytes = _to_bytes(regex)
 
     def __eq__(self, other):
-        if not isinstance(other, self.__class__):
+        if type(other) is not type(self):
             return NotImplemented
         return other.regex == self.regex
 
@@ -302,21 +302,37 @@ def _to_dict(self) -> dict[str, int]:
         return timestamp_range_kwargs
 
     def __repr__(self) -> str:
-        return f"{self.__class__.__name__}(start={self.start}, end={self.end})"
+        return f"{self.__class__.__name__}(start={self.start!r}, end={self.end!r})"
 
 
 class TimestampRangeFilter(RowFilter):
     """Row filter that limits cells to a range of time.
 
-    :type range_: :class:`TimestampRange`
-    :param range_: Range of time that cells should match against.
+    :type start: :class:`datetime.datetime` or :class:`TimestampRange`
+    :param start: (Optional) The (inclusive) lower bound of the timestamp
+                  range, or a :class:`TimestampRange` instance.
+
+    :type end: :class:`datetime.datetime`
+    :param end: (Optional) The (exclusive) upper bound of the timestamp
+                range.
     """
 
-    def __init__(self, start: "datetime" | None = None, end: "datetime" | None = None):
-        self.range_: TimestampRange = TimestampRange(start, end)
+    def __init__(
+        self,
+        start: "datetime" | TimestampRange | None = None,
+        end: "datetime" | None = None,
+    ):
+        if isinstance(start, TimestampRange):
+            if end is not None:
+                raise ValueError(
+                    "Cannot specify both a TimestampRange and an end datetime."
+                )
+            self.range_: TimestampRange = start
+        else:
+            self.range_ = TimestampRange(start, end)
 
     def __eq__(self, other):
-        if not isinstance(other, self.__class__):
+        if not isinstance(other, TimestampRangeFilter):
             return NotImplemented
         return other.range_ == self.range_
 
@@ -408,7 +424,7 @@ def __init__(
         self.inclusive_end = inclusive_end
 
     def __eq__(self, other):
-        if not isinstance(other, self.__class__):
+        if not isinstance(other, ColumnRangeFilter):
             return NotImplemented
         return (
             other.family_id == self.family_id
@@ -946,7 +962,7 @@ def __init__(
         self.false_filter = false_filter
 
     def __eq__(self, other):
-        if not isinstance(other, self.__class__):
+        if not isinstance(other, ConditionalRowFilter):
             return NotImplemented
         return (
             other.predicate_filter == self.predicate_filter

packages/google-cloud-bigtable/google/cloud/bigtable/row.py

@@ -220,6 +220,10 @@ def _delete_cells(self, column_family_id, columns, time_range=None, state=None):
                 mutations.DeleteAllFromFamily(family_to_delete=column_family_id)
             )
         else:
+            if isinstance(columns, (str, bytes)):
+                raise TypeError(
+                    "columns must be an iterable of column names, not str or bytes"
+                )
             timestamps = time_range._to_dict() if time_range else {}
             start_timestamp_micros = timestamps.get("start_timestamp_micros")
             end_timestamp_micros = timestamps.get("end_timestamp_micros")

packages/google-cloud-bigtable/google/cloud/bigtable/row_filters.py

@@ -89,6 +89,9 @@ class TimestampRangeFilter(BaseTimestampRangeFilter):
     def __init__(self, range_):
         self.range_ = range_
 
+    def __repr__(self) -> str:
+        return f"{self.__class__.__name__}(range_={self.range_!r})"
+
 
 class ExactValueFilter(ValueRegexFilter):
     """Row filter for an exact value.

packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py

@@ -48,7 +48,10 @@ def row_keys(self):
 
     @property
     def row_ranges(self):
-        return self._read_rows_query.row_ranges
+        return [
+            r if isinstance(r, RowRange) else RowRange._from_pb(r._to_pb())
+            for r in self._read_rows_query.row_ranges
+        ]
 
     def add_row_key(self, row_key):
         """Add row key to row_keys list.
@@ -157,6 +160,31 @@ class RowRange(_MappableAttributesMixin, BaseRowRange):
         "end_inclusive": "end_is_inclusive",
     }
 
+    def __init__(self, *args, **kwargs):
+        start_inc = kwargs.get(
+            "start_inclusive",
+            kwargs.get("start_is_inclusive", args[2] if len(args) > 2 else None),
+        )
+        end_inc = kwargs.get(
+            "end_inclusive",
+            kwargs.get("end_is_inclusive", args[3] if len(args) > 3 else None),
+        )
+        super().__init__(*args, **kwargs)
+        self._start_inclusive = True if start_inc is None else bool(start_inc)
+        self._end_inclusive = False if end_inc is None else bool(end_inc)
+
+    @property
+    def start_inclusive(self) -> bool:
+        if self.start_key is not None:
+            return self.start_is_inclusive
+        return getattr(self, "_start_inclusive", True)
+
+    @property
+    def end_inclusive(self) -> bool:
+        if self.end_key is not None:
+            return self.end_is_inclusive
+        return getattr(self, "_end_inclusive", False)
+
     def get_range_kwargs(self):
         """Convert row range object to dict which can be passed to
         google.bigtable.v2.RowRange add method.

packages/google-cloud-bigtable/tests/unit/data/test_mutations.py

@@ -188,12 +188,18 @@ def _target_class(self):
     def _make_one(self, *args, **kwargs):
         return self._target_class()(*args, **kwargs)
 
-    @pytest.mark.parametrize("input_val", [2**64, -(2**64)])
+    @pytest.mark.parametrize("input_val", [2**63, -(2**63) - 1, 2**64, -(2**64)])
     def test_ctor_large_int(self, input_val):
         with pytest.raises(ValueError) as e:
             self._make_one(family="f", qualifier=b"b", new_value=input_val)
         assert "int values must be between" in str(e.value)
 
+    @pytest.mark.parametrize("input_val", [True, False])
+    def test_ctor_bool_value(self, input_val):
+        with pytest.raises(TypeError) as e:
+            self._make_one(family="f", qualifier=b"b", new_value=input_val)
+        assert "new_value must be bytes, str, or int" in str(e.value)
+
     @pytest.mark.parametrize("input_val", ["", "a", "abc", "hello world!"])
     def test_ctor_str_value(self, input_val):
         found = self._make_one(family="f", qualifier=b"b", new_value=input_val)
@@ -223,21 +229,25 @@ def test_ctor_str_inputs(self):
         assert instance.qualifier == expected_qualifier
         assert instance.new_value == expected_value
 
-    @pytest.mark.parametrize("input_val", [-20, -1, 0, 1, 100, int(2**60)])
+    @pytest.mark.parametrize(
+        "input_val", [-(2**63), -20, -1, 0, 1, 100, int(2**60), 2**63 - 1]
+    )
     def test_ctor_int_value(self, input_val):
         found = self._make_one(family="f", qualifier=b"b", new_value=input_val)
         assert found.new_value == input_val.to_bytes(8, "big", signed=True)
 
     @pytest.mark.parametrize(
         "int_value,expected_bytes",
         [
+            (-(2**63), b"\x80\x00\x00\x00\x00\x00\x00\x00"),
             (-42, b"\xff\xff\xff\xff\xff\xff\xff\xd6"),
             (-2, b"\xff\xff\xff\xff\xff\xff\xff\xfe"),
             (-1, b"\xff\xff\xff\xff\xff\xff\xff\xff"),
             (0, b"\x00\x00\x00\x00\x00\x00\x00\x00"),
             (1, b"\x00\x00\x00\x00\x00\x00\x00\x01"),
             (2, b"\x00\x00\x00\x00\x00\x00\x00\x02"),
             (100, b"\x00\x00\x00\x00\x00\x00\x00d"),
+            (2**63 - 1, b"\x7f\xff\xff\xff\xff\xff\xff\xff"),
         ],
     )
     def test_ctor_int_value_bytes(self, int_value, expected_bytes):
@@ -409,6 +419,18 @@ def test_ctor_no_timestamps(self):
         assert instance.start_timestamp_micros is None
         assert instance.end_timestamp_micros is None
 
+    @pytest.mark.parametrize("qualifier_str", ["", "a", "col", "my_column", "qual_str"])
+    def test_ctor_str_qualifier(self, qualifier_str):
+        instance = self._make_one("test-family", qualifier_str)
+        expected_bytes = qualifier_str.encode("utf-8")
+        assert instance.qualifier == expected_bytes
+        assert instance._to_pb().delete_from_column.column_qualifier == expected_bytes
+
+    @pytest.mark.parametrize("bad_qualifier", [123, None, object()])
+    def test_ctor_invalid_qualifier(self, bad_qualifier):
+        with pytest.raises(TypeError, match="qualifier must be bytes or str"):
+            self._make_one("test-family", bad_qualifier)
+
     def test_ctor_timestamps_out_of_order(self):
         expected_family = "test-family"
         expected_qualifier = b"test-qualifier"
@@ -725,14 +747,38 @@ def _target_class(self):
     def _make_one(self, *args, **kwargs):
         return self._target_class()(*args, **kwargs)
 
-    @pytest.mark.parametrize("input_val", [2**64, -(2**64)])
+    @pytest.mark.parametrize("input_val", [2**63, -(2**63) - 1, 2**64, -(2**64)])
     def test_ctor_large_int(self, input_val):
         with pytest.raises(ValueError) as e:
             self._make_one(
                 family="f", qualifier=b"b", value=input_val, timestamp_micros=123
             )
         assert "int values must be between" in str(e.value)
 
+    @pytest.mark.parametrize("input_val", [-(2**63), 2**63 - 1])
+    def test_ctor_int64_bounds(self, input_val):
+        instance = self._make_one(
+            family="f", qualifier=b"b", value=input_val, timestamp_micros=123
+        )
+        assert instance.value == input_val
+        assert instance._to_pb().add_to_cell.input.int_value == input_val
+
+    @pytest.mark.parametrize("input_val", [True, False])
+    def test_ctor_bool_value(self, input_val):
+        with pytest.raises(TypeError) as e:
+            self._make_one(
+                family="f", qualifier=b"b", value=input_val, timestamp_micros=123
+            )
+        assert "value must be int" in str(e.value)
+
+    @pytest.mark.parametrize("input_ts", [True, False])
+    def test_ctor_bool_timestamp(self, input_ts):
+        with pytest.raises(TypeError) as e:
+            self._make_one(
+                family="f", qualifier=b"b", value=123, timestamp_micros=input_ts
+            )
+        assert "timestamp_micros must be int" in str(e.value)
+
     @pytest.mark.parametrize("input_val", ["", "a", "abc", "hello world!"])
     def test_ctor_str_value(self, input_val):
         with pytest.raises(TypeError) as e:

packages/google-cloud-bigtable/tests/unit/data/test_read_modify_write_rules.py

@@ -50,6 +50,8 @@ def _target_class(self):
         [
             (("fam", b"qual", 1), ("fam", b"qual", 1)),
             (("fam", b"qual", -12), ("fam", b"qual", -12)),
+            (("fam", b"qual", -(2**63)), ("fam", b"qual", -(2**63))),
+            (("fam", b"qual", 2**63 - 1), ("fam", b"qual", 2**63 - 1)),
             (("fam", "qual", 1), ("fam", b"qual", 1)),
             (("fam", "qual", 0), ("fam", b"qual", 0)),
             (("", "", 0), ("", b"", 0)),
@@ -62,17 +64,21 @@ def test_ctor(self, args, expected):
         assert instance.qualifier == expected[1]
         assert instance.increment_amount == expected[2]
 
-    @pytest.mark.parametrize("input_amount", [1.1, None, "1", object(), "", b"", b"1"])
+    @pytest.mark.parametrize(
+        "input_amount", [True, False, 1.1, None, "1", object(), "", b"", b"1"]
+    )
     def test_ctor_bad_input(self, input_amount):
         with pytest.raises(TypeError) as e:
             self._target_class()("fam", b"qual", input_amount)
         assert "increment_amount must be an integer" in str(e.value)
 
-    @pytest.mark.parametrize("large_value", [2**64, 2**64 + 1, -(2**64), -(2**64) - 1])
+    @pytest.mark.parametrize(
+        "large_value", [2**63, -(2**63) - 1, 2**64, 2**64 + 1, -(2**64), -(2**64) - 1]
+    )
     def test_ctor_large_values(self, large_value):
         with pytest.raises(ValueError) as e:
             self._target_class()("fam", b"qual", large_value)
-            assert "too large" in str(e.value)
+        assert "increment_amount must be between -2**63 and 2**63" in str(e.value)
 
     @pytest.mark.parametrize(
         "args,expected",

packages/google-cloud-bigtable/tests/unit/data/test_row_filters.py

@@ -831,6 +831,18 @@ def test_value_regex_filter___repr__():
     assert eval(repr(row_filter)) == row_filter
 
 
+def test_value_regex_filter_not_equal_to_literal_value_filter():
+    from google.cloud.bigtable.data.row_filters import (
+        LiteralValueFilter,
+        ValueRegexFilter,
+    )
+
+    vrf = ValueRegexFilter(b"abc")
+    lvf = LiteralValueFilter(b"abc")
+    assert vrf != lvf
+    assert lvf != vrf
+
+
 def test_literal_value_filter_to_pb_w_bytes():
     from google.cloud.bigtable.data.row_filters import LiteralValueFilter