bulk_create lack of implicit transaction.atomic usage surfaces improper nested error handling

[Paul Zakin]

Hello!

Just tried upgrading to v6 - and this snippet from a test (django and pytest) now breaks. Now, the first chunk passses but the second chunk requires a "current transaction is aborted, commands ignored until end of transaction block".

Historically, I got around that by wrapping the test in an with transaction.atomic() to prevent the error from moving to the next part of the test - but that no longer works.

Not sure if this counts as an error - but I'm not supposed to do this in a test anyway! But it did break from v5 to v6.

   # Do policies apply to .bulk_create()?
    with transaction.atomic():  # Required as any database error, like RLS, must be in transaction.atomic()
        try:
            Foo.objects.bulk_create([Foo(name="Foo", study_id=study.pk)])
            force_test_to_fail()
        except Exception as error:
            assert str(error) == 'new row violates row-level security policy for table "foo"'

    with transaction.atomic():  # Required as any database error, like RLS, must be in transaction.atomic()
        try:
            # Do policies apply to .bulk_update()?
            foo.name = "XXX"
            # No rows should have been updated, because the admin does not have access to the id
            Foo.objects.bulk_update([foo], fields=["*"])
            force_test_to_fail()
        except Exception as error:
            assert str(error) == f"id: {foo.pk} in table: foo does not exist!"

[Simon Charette]

Hello Paul,

It would help tremendously if you could provide a simplified project that passes againt Django 5.0 (you didn't mention 5.2 so I assumed you were still on 5.0) but fails against 6.0 with the exact frozen dependencies you are using.

Django might be at fault here but since you are using pytest (and I assume pytest-django) and transaction handling in tests can be quite finicky it can be quite hard to pinpoint what exactly might cause the problems you are facing with the little details you've provided (no exact test definition, no exact package versions, do model definitions, etc).

Please help us help you by providing more details about the problem you are experiencing so we can reproduce and adequately diagnoze the problem. In the mean time I'll close the ticket as lacking details but please reopen when you're able to provide us more details.

[Paul Zakin]

Totally fair - our setup is complicated enough that might be pretty challenging.

However, if I have the time and figure it out - I'll reopen - thanks a bunch!

[Paul Zakin]

Interesting - I bisected and I found the commit

​https://github.com/django/django/commit/e1671278e88265e64811657b8b939b5d786295cb

I think it is this - any idea why?

       with transaction.atomic(using=self.db, savepoint=False):
        if objs_with_pk and objs_without_pk:
            context = transaction.atomic(using=self.db, savepoint=False)
        else:
            context = nullcontext()
        with context:

[Simon Charette]

Well I think you have your answer here.

Prior to #36490 (e1671278e88265e64811657b8b939b5d786295cb) bulk_create was always creating an implicit transaction even if it only needed to issue a single INSERT.

Now that no explicit transaction.atomic usage is made the database error your RLS policy triggers and don't let the error reach transaction.Atomic.__exit__ which goes against its documented usage (see ​Avoid catching exceptions inside atomic!) it leaves the current transaction in an incoherent state.

TL;DR your usage of transaction.atomic is incorrect but it was masked by bulk_create creating a nested atomic previously (which was implementation detail) and you should do the following instead

try:
    with transaction.atomic():
        Foo.objects.bulk_create([Foo(name="Foo", study_id=study.pk)])
        force_test_to_fail()
except Exception as error:
    assert str(error) == 'new row violates row-level security policy for table "foo"'            

[Paul Zakin]

That makes perfect sense to me - feel free to close this - thank you so much for your help!