Skip to content

Commit 8c1eb13

Browse files
authored
fix(spanner-django): db_returning should only be overridden for Spanner (#16111)
The db_returning attribute of AutoFields should only be overridden for Spanner database connections, and not globally for all databases. Note: Many tests are failing. These failures are not due to these changes, but due to existing failures for this library. Fixes #15930 Replaces googleapis/python-spanner-django#961
1 parent d3d960b commit 8c1eb13

File tree

2 files changed

+18
-6
lines changed

2 files changed

+18
-6
lines changed

packages/django-google-spanner/django_spanner/__init__.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,15 @@ def autofield_init(self, *args, **kwargs):
8484
== "true"
8585
):
8686
self.default = gen_rand_int64
87+
self.db_returning = False
88+
self.validators = []
8789
break
8890

8991

9092
AutoField.__init__ = autofield_init
91-
AutoField.db_returning = False
92-
AutoField.validators = []
9393

9494
SmallAutoField.__init__ = autofield_init
9595
BigAutoField.__init__ = autofield_init
96-
SmallAutoField.db_returning = False
97-
BigAutoField.db_returning = False
98-
SmallAutoField.validators = []
99-
BigAutoField.validators = []
10096

10197

10298
def get_prep_value(self, value):

packages/django-google-spanner/tests/unit/django_spanner/test_schema.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,19 +408,27 @@ def test_autofield_no_default(self):
408408
"""Spanner, default is not provided."""
409409
field = AutoField(name="field_name")
410410
assert gen_rand_int64 == field.default
411+
# db_returning must be explicitly False because Spanner is handling ID generation client-side
412+
assert getattr(field, "db_returning", True) is False
411413

412414
def test_autofield_default(self):
413415
"""Spanner, default provided."""
414416
mock_func = mock.Mock()
415417
field = AutoField(name="field_name", default=mock_func)
416418
assert gen_rand_int64 != field.default
417419
assert mock_func == field.default
420+
# A default was already provided, so Spanner does not generate random IDs client-side.
421+
# Therefore, db_returning does not need to be overridden to False.
422+
assert field.db_returning is True
418423

419424
def test_autofield_not_spanner(self):
420425
"""Not Spanner, default not provided."""
421426
connection.settings_dict["ENGINE"] = "another_db"
422427
field = AutoField(name="field_name")
423428
assert gen_rand_int64 != field.default
429+
# db_returning should remain untouched (implicitly True) for non-Spanner databases
430+
# so that Django retrieves the auto-increment ID correctly.
431+
assert field.db_returning is True
424432
connection.settings_dict["ENGINE"] = "django_spanner"
425433

426434
def test_autofield_not_spanner_w_default(self):
@@ -430,6 +438,8 @@ def test_autofield_not_spanner_w_default(self):
430438
field = AutoField(name="field_name", default=mock_func)
431439
assert gen_rand_int64 != field.default
432440
assert mock_func == field.default
441+
# Because it's not a Spanner database, the behavior shouldn't be altered in any way.
442+
assert field.db_returning is True
433443
connection.settings_dict["ENGINE"] = "django_spanner"
434444

435445
def test_autofield_spanner_as_non_default_db_random_generation_enabled(
@@ -441,6 +451,9 @@ def test_autofield_spanner_as_non_default_db_random_generation_enabled(
441451
connections.settings["secondary"]["RANDOM_ID_GENERATION_ENABLED"] = "true"
442452
field = AutoField(name="field_name")
443453
assert gen_rand_int64 == field.default
454+
# Since this specific connection explicitly enables client-side random generation,
455+
# we must tell Django not to attempt retrieving the DB's returned ID.
456+
assert getattr(field, "db_returning", True) is False
444457
connections.settings["default"]["ENGINE"] = "django_spanner"
445458
connections.settings["secondary"]["ENGINE"] = "django_spanner"
446459
del connections.settings["secondary"]["RANDOM_ID_GENERATION_ENABLED"]
@@ -450,4 +463,7 @@ def test_autofield_random_generation_disabled(self):
450463
connections.settings["default"]["RANDOM_ID_GENERATION_ENABLED"] = "false"
451464
field = AutoField(name="field_name")
452465
assert gen_rand_int64 != field.default
466+
# Because we're delegating ID generation back to the database backend,
467+
# Django needs to be able to retrieve the assigned ID.
468+
assert field.db_returning is True
453469
del connections.settings["default"]["RANDOM_ID_GENERATION_ENABLED"]

0 commit comments

Comments
 (0)