Skip to content

feat(django-google-spanner): support Django 6.0 - #18128

Open
sakthivelmanii wants to merge 3 commits into
mainfrom
add-django-6.0-support
Open

feat(django-google-spanner): support Django 6.0#18128
sakthivelmanii wants to merge 3 commits into
mainfrom
add-django-6.0-support

Conversation

@sakthivelmanii

@sakthivelmanii sakthivelmanii commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds support for Django 6.0 in django-google-spanner while maintaining full backwards compatibility with Django 5.2.

Fixes #18053


Key Changes

  • Covering Indexes (STORING): Added support for include clauses in index creation, generating Cloud Spanner GoogleSQL STORING (col1, col2) DDL; declared supports_covering_indexes = True.
  • Composite Primary Keys: Declared support for Django 6.0 composite primary keys (supports_composite_primary_keys = True).
  • DML Returning Clauses: Enabled can_return_columns_from_insert = True and updated returning_columns() in operations.py to safely handle column names and expression objects, allowing Django to automatically populate database defaults and GeneratedField values on INSERT ... THEN RETURN.

Behavioral Notes for Users

  • DML THEN RETURN: With can_return_columns_from_insert = True, Django will now generate THEN RETURN clauses for models with database-generated defaults or GeneratedField columns upon .save(). Applications wishing to preserve legacy behavior can opt out via AppConfig:
    from django.apps import AppConfig
    
    class MyAppConfig(AppConfig):
        name = "myapp"
    
        def ready(self):
            from django_spanner.features import DatabaseFeatures
            DatabaseFeatures.can_return_columns_from_insert = False
    

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #18053 🦕

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Django 6.0, including dependency updates, tuple-casting for lookup parameters, an asynchronous autocommit setter, and a Spanner-specific JSON path compiler. The review feedback highlights critical improvements: resolving syntax, type, and SQL injection issues in compile_json_path by utilizing json.dumps; wrapping the async autocommit operation in self.execute_wrapper to align with Django standards; and fixing invalid shell syntax and compatibility issues in the new test suite script.

Comment thread packages/django-google-spanner/django_spanner/operations.py Outdated
Comment thread packages/django-google-spanner/django_spanner/base.py Outdated
Comment thread packages/django-google-spanner/django_test_suite_6.0.sh Outdated
Comment thread packages/django-google-spanner/setup.py Outdated
@sakthivelmanii
sakthivelmanii force-pushed the add-django-6.0-support branch 4 times, most recently from 311d5d1 to cfa5943 Compare August 19, 2026 13:39
@sakthivelmanii
sakthivelmanii marked this pull request as ready for review August 19, 2026 13:39
@sakthivelmanii
sakthivelmanii requested review from a team as code owners August 19, 2026 13:40
Comment thread packages/django-google-spanner/setup.py
@parthea

parthea commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Django 6.0 in the django-google-spanner package. Key changes include updating supported versions and dependencies, implementing async autocommit handling, adding Django 6.0 test exclusions, and updating database features, operations, and schema editors to support index inclusion columns and returning columns from inserts. Feedback on these changes highlights two important issues: first, the _index_include_sql helper in schema.py should resolve actual database column names via model._meta.get_field(field).column rather than using str(field) directly; second, the async autocommit wrapper in base.py should use thread_sensitive=True with sync_to_async to ensure thread safety and prevent connection sharing issues.

Comment thread packages/django-google-spanner/django_spanner/schema.py
Comment thread packages/django-google-spanner/django_spanner/base.py Outdated
Comment thread packages/django-google-spanner/django_test_suite_6.0.sh Outdated
@sakthivelmanii
sakthivelmanii force-pushed the add-django-6.0-support branch from cfa5943 to b219da1 Compare August 19, 2026 16:05
Comment thread packages/django-google-spanner/django_spanner/operations.py Outdated
Comment thread packages/django-google-spanner/django_spanner/lookups.py Outdated
"sqlparse >= 0.3.0",
"google-cloud-spanner >= 3.13.0",
"django >= 5.2, < 6.0",
"google-cloud-spanner >= 3.69.1",

@parthea parthea Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you clarify if we really need to bump google-cloud-spanner? Do tests fail with 3.13.0?

Is there a lower minimum that we can set here?

If 3.69.1 is yanked or has a regression, users may not be able to install/use the latest version of django-google-spanner
https://pypi.org/project/google-cloud-spanner/3.69.1/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#18152 This fix is needed for can_return_columns_from_insert to work correctly. I will update this version to 3.69.2 once we have a new version released. I am planning to push this change to 3.70.0 or 3.69.3

Comment thread packages/django-google-spanner/django_spanner/features.py
Comment thread packages/django-google-spanner/django_spanner/__init__.py
Comment thread packages/django-google-spanner/django_spanner/features.py
Comment thread packages/django-google-spanner/django_spanner/base.py Outdated
Comment on lines 58 to 67
UNIT_TEST_DEPENDENCIES = [
"django~=5.2",
"sqlparse==0.3.1",
"django>=5.2,<6.1",
"sqlparse>=0.3.1",
]

UNIT_TEST_MOCKSERVER_DEPENDENCIES = [
"django~=5.2",
"google-cloud-spanner>=3.55.0",
"django>=5.2,<6.1",
"google-cloud-spanner>=3.69.1",
"sqlparse>=0.4.4",
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only expands the allowed versions, it does not ensure that we actually run the tests for both supported Django versions. We should probably use something like nox.parameterize to ensure that we really run the tests for both versions that we support, so something like this:

DJANGO_VERSIONS = ["5.2", "6.0"]

def default(session, django_version="5.2"):
    # Target specific Django version
    django_dep = f"django~={django_version}.0" if django_version == "6.0" else "django~=5.2.0"

    session.install(
        *UNIT_TEST_STANDARD_DEPENDENCIES,
        *UNIT_TEST_EXTERNAL_DEPENDENCIES,
        *UNIT_TEST_MOCKSERVER_DEPENDENCIES,
        django_dep,
    )
    session.install("-e", ".")

    # Run py.test against unit and mockserver tests.
    session.run(
        "py.test",
        "--quiet",
        "--cov=django_spanner",
        "--cov-append",
        "--cov-config=.coveragerc",
        "--cov-report=",
        "--cov-fail-under=0",
        os.path.join("tests", "unit"),
        os.path.join("tests", "mockserver_tests"),
        *session.posargs,
    )

@nox.session(python=ALL_PYTHON)
@nox.parametrize("django", DJANGO_VERSIONS)
def unit(session, django):
    """Run the unit test suite across Django versions."""
    default(session, django_version=django)

@nox.session(python=MOCKSERVER_TEST_PYTHON_VERSION)
@nox.parametrize("django", DJANGO_VERSIONS)
def mockserver(session, django):
    """Run mockserver tests across Django versions."""
    default(session, django_version=django)

We should also check the GitHub Actions workflows to ensure that:

  1. They actually run the right nox sessions so they test both versions (I think they do)
  2. They use Python versions that support both versions that we support (django-spanner-integration-tests-against-emulator-3.10.yml probably does not). Alternatively: Add a matrix to these workflows to test with multiple Python versions to ensure that the behavior is correct in those cases as well.

@olavloite olavloite left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes LGTM, but please fix the problem that the tests are not actually running for both versions before merging (and of course ensure that the tests pass for both versions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(django-spanner): support Django 6.0

3 participants