Skip to content

Commit 1b2c6f8

Browse files
authored
Prevent error when time partitioning is populated with empty dict (#7904)
* Prevent error when time partitioning is populated with empty dict As reported in internal bug 131167013, calling `table.time_partitioning` can sometimes fail with `KeyError: 'type'` when attempting to read this property on a non-partitioned table. * Add tests for TimePartitioning.from_api_repr * Remove unused imports * Speed up lint session by installing local deps inplace.
1 parent a4abd9b commit 1b2c6f8

File tree

3 files changed

+74
-4
lines changed

3 files changed

+74
-4
lines changed

bigquery/google/cloud/bigquery/table.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -1787,7 +1787,7 @@ def type_(self):
17871787
"""google.cloud.bigquery.table.TimePartitioningType: The type of time
17881788
partitioning to use.
17891789
"""
1790-
return self._properties["type"]
1790+
return self._properties.get("type")
17911791

17921792
@type_.setter
17931793
def type_(self, value):
@@ -1849,7 +1849,7 @@ def from_api_repr(cls, api_repr):
18491849
google.cloud.bigquery.table.TimePartitioning:
18501850
The ``TimePartitioning`` object.
18511851
"""
1852-
instance = cls(api_repr["type"])
1852+
instance = cls()
18531853
instance._properties = api_repr
18541854
return instance
18551855

bigquery/noxfile.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,10 @@ def lint(session):
141141
serious code quality issues.
142142
"""
143143

144-
session.install("black", "flake8", *LOCAL_DEPS)
145-
session.install(".")
144+
session.install("black", "flake8")
145+
for local_dep in LOCAL_DEPS:
146+
session.install("-e", local_dep)
147+
session.install("-e", ".")
146148
session.run("flake8", os.path.join("google", "cloud", "bigquery"))
147149
session.run("flake8", "tests")
148150
session.run("flake8", os.path.join("docs", "snippets.py"))

bigquery/tests/unit/test_table.py

+68
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,54 @@ def test__build_resource_w_custom_field_not_in__properties(self):
871871
with self.assertRaises(ValueError):
872872
table._build_resource(["bad"])
873873

874+
def test_time_partitioning_getter(self):
875+
from google.cloud.bigquery.table import TimePartitioning
876+
from google.cloud.bigquery.table import TimePartitioningType
877+
878+
dataset = DatasetReference(self.PROJECT, self.DS_ID)
879+
table_ref = dataset.table(self.TABLE_NAME)
880+
table = self._make_one(table_ref)
881+
882+
table._properties["timePartitioning"] = {
883+
"type": "DAY",
884+
"field": "col1",
885+
"expirationMs": "123456",
886+
"requirePartitionFilter": False,
887+
}
888+
self.assertIsInstance(table.time_partitioning, TimePartitioning)
889+
self.assertEqual(table.time_partitioning.type_, TimePartitioningType.DAY)
890+
self.assertEqual(table.time_partitioning.field, "col1")
891+
self.assertEqual(table.time_partitioning.expiration_ms, 123456)
892+
self.assertFalse(table.time_partitioning.require_partition_filter)
893+
894+
def test_time_partitioning_getter_w_none(self):
895+
dataset = DatasetReference(self.PROJECT, self.DS_ID)
896+
table_ref = dataset.table(self.TABLE_NAME)
897+
table = self._make_one(table_ref)
898+
899+
table._properties["timePartitioning"] = None
900+
self.assertIsNone(table.time_partitioning)
901+
902+
del table._properties["timePartitioning"]
903+
self.assertIsNone(table.time_partitioning)
904+
905+
def test_time_partitioning_getter_w_empty(self):
906+
from google.cloud.bigquery.table import TimePartitioning
907+
908+
dataset = DatasetReference(self.PROJECT, self.DS_ID)
909+
table_ref = dataset.table(self.TABLE_NAME)
910+
table = self._make_one(table_ref)
911+
912+
# Even though there are required properties according to the API
913+
# specification, sometimes time partitioning is populated as an empty
914+
# object. See internal bug 131167013.
915+
table._properties["timePartitioning"] = {}
916+
self.assertIsInstance(table.time_partitioning, TimePartitioning)
917+
self.assertIsNone(table.time_partitioning.type_)
918+
self.assertIsNone(table.time_partitioning.field)
919+
self.assertIsNone(table.time_partitioning.expiration_ms)
920+
self.assertIsNone(table.time_partitioning.require_partition_filter)
921+
874922
def test_time_partitioning_setter(self):
875923
from google.cloud.bigquery.table import TimePartitioning
876924
from google.cloud.bigquery.table import TimePartitioningType
@@ -2211,6 +2259,20 @@ def test_constructor_explicit(self):
22112259
self.assertEqual(time_partitioning.expiration_ms, 10000)
22122260
self.assertTrue(time_partitioning.require_partition_filter)
22132261

2262+
def test_from_api_repr_empty(self):
2263+
klass = self._get_target_class()
2264+
2265+
# Even though there are required properties according to the API
2266+
# specification, sometimes time partitioning is populated as an empty
2267+
# object. See internal bug 131167013.
2268+
api_repr = {}
2269+
time_partitioning = klass.from_api_repr(api_repr)
2270+
2271+
self.assertIsNone(time_partitioning.type_)
2272+
self.assertIsNone(time_partitioning.field)
2273+
self.assertIsNone(time_partitioning.expiration_ms)
2274+
self.assertIsNone(time_partitioning.require_partition_filter)
2275+
22142276
def test_from_api_repr_minimal(self):
22152277
from google.cloud.bigquery.table import TimePartitioningType
22162278

@@ -2223,6 +2285,12 @@ def test_from_api_repr_minimal(self):
22232285
self.assertIsNone(time_partitioning.expiration_ms)
22242286
self.assertIsNone(time_partitioning.require_partition_filter)
22252287

2288+
def test_from_api_repr_doesnt_override_type(self):
2289+
klass = self._get_target_class()
2290+
api_repr = {"type": "HOUR"}
2291+
time_partitioning = klass.from_api_repr(api_repr)
2292+
self.assertEqual(time_partitioning.type_, "HOUR")
2293+
22262294
def test_from_api_repr_explicit(self):
22272295
from google.cloud.bigquery.table import TimePartitioningType
22282296

0 commit comments

Comments
 (0)