django-pgtrigger

django-pgtrigger copied to clipboard

Posting this here in case anyone is interested or happens to bump into the same issue. I'm just describing the problem I've encountered as I have not had time yet to work on a solution (hopefully tomorrow!).

After getting some feedback in issue #70 I was attempting to add a site wide admin action that would allow hard deletes by ignoring the soft _delete trigger. This worked fine on most of my models inheriting triggers through SaasBaseModel (see models below) but failed silently on my Message model.

I am using django-postgres-extra to do table partitioning. My Message model failed to allow deletes within the with pgtrigger.ignore() context.

Models

from django.contrib.gis.db import models

from psqlextra.models import PostgresModel
from psqlextra.models import PostgresPartitionedModel

import pgtrigger

class SaasBaseFields(models.Model):

    class Meta:
        abstract = True

    created = models.DateTimeField(auto_now_add=True)
    modified = models.DateTimeField(auto_now=True)
    is_active = models.BooleanField(default=True)
    deactivated = models.DateTimeField(blank=True, null=True)


class SaasBaseModel(
    PostgresModel,
    SaasBaseFields,
    models.Model
):

    class Meta:
        abstract = True
        triggers = [
            pgtrigger.Protect(
                name='read_only',
                operation=pgtrigger.Update,
                condition=pgtrigger.Q(old__created__df=pgtrigger.F('new__created'))
            ),
            pgtrigger.SoftDelete(name='soft_delete', field='is_active'),
            pgtrigger.Trigger(
                name='set_deactivated',
                when=pgtrigger.Before,
                operation=pgtrigger.Update,
                func="NEW.deactivated = NOW(); RETURN NEW;",
                condition=pgtrigger.Q(old__is_active=True, new__is_active=False)
            )
        ]


class SaasBasePartitionedByCreatedModel(
    PostgresPartitionedModel,
    SaasBaseFields,
    models.Model
):

    class Meta:
        abstract = True
        base_manager_name = "objects"
        triggers = [
            pgtrigger.Protect(
                name='read_only',
                operation=pgtrigger.Update,
                condition=pgtrigger.Q(old__created__df=pgtrigger.F('new__created'))
            ),
            pgtrigger.SoftDelete(name='soft_delete', field='is_active'),
            pgtrigger.Trigger(
                name='set_deactivated',
                when=pgtrigger.Before,
                operation=pgtrigger.Update,
                func="NEW.deactivated = NOW(); RETURN NEW;",
                condition=pgtrigger.Q(old__is_active=True, new__is_active=False)
            )
        ]

    # define PartitioningMeta on the child class
    # all partion management is defined in informatical.common.partitioning


class Room(SaasBaseModel):
    name = models.CharField(max_length=128)
    online = models.ManyToManyField(to='users.User', blank=True)


class Message(SaasBasePartitionedByCreatedModel):

    class PartitioningMeta:
        method = PostgresPartitioningMethod.RANGE
        key = ["created"]

    user = models.ForeignKey(to='users.User', on_delete=models.CASCADE)
    room = models.ForeignKey(to=Room, on_delete=models.CASCADE)
    content = models.CharField(max_length=512)

    def __str__(self):
        return f'{self.user.username}: {self.content} [{self.created}]'

Pgtrigger ls output

chat.Room:read_only default INSTALLED ENABLED chat.Room:soft_delete default INSTALLED ENABLED chat.Room:set_deactivated default INSTALLED ENABLED chat.Message:read_only default INSTALLED ENABLED chat.Message:soft_delete default INSTALLED ENABLED chat.Message:set_deactivated default INSTALLED ENABLED users.User:read_only default INSTALLED ENABLED users.User:soft_delete default INSTALLED ENABLED users.User:set_deactivated default INSTALLED ENABLED chat_message_2022_aug_15:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_2022_aug_11:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_2022_aug_13:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_2022_aug_14:pgtrigger_soft_delete_560fe default PRUNE ENABLED chat_message_2022_aug_16:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_2022_aug_12:pgtrigger_soft_delete_560fe default PRUNE ENABLED chat_message_default:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_2022_aug_10:pgtrigger_soft_delete_560fe default PRUNE ENABLED chat_message_2022_aug_14:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_2022_aug_12:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_2022_aug_13:pgtrigger_soft_delete_560fe default PRUNE ENABLED chat_message_2022_aug_10:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_2022_aug_15:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_2022_aug_16:pgtrigger_soft_delete_560fe default PRUNE ENABLED chat_message_2022_aug_11:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_2022_aug_16:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_2022_aug_14:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_default:pgtrigger_soft_delete_560fe default PRUNE ENABLED chat_message_2022_aug_13:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_default:pgtrigger_read_only_3c30d default PRUNE ENABLED chat_message_2022_aug_15:pgtrigger_soft_delete_560fe default PRUNE ENABLED chat_message_2022_aug_10:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_2022_aug_12:pgtrigger_set_deactivated_49e4c default PRUNE ENABLED chat_message_2022_aug_11:pgtrigger_soft_delete_560fe default PRUNE ENABLED

@jzmiller1 I think you were affected by what I mentioned in https://github.com/Opus10/django-pgtrigger/issues/57#issuecomment-1210000530

Since PartitioningMeta does not inherit SaasBasePartitionedByCreatedModel.Meta, the triggers from the base meta class will not be part of the child meta class.

This is the default functionality for how Django Meta works unfortunately.

I.e. you need to do:

class Message(SaasBasePartitionedByCreatedModel):

    class PartitioningMeta(SaasBasePartitionedByCreatedModel.Meta):
        # define other stuff...

@jzmiller1 even indexes, constraints, and other stuff in Meta will not be inherited if overriding the child Meta class.

What I can do is issue a warning or an error in pgtrigger that lets the user know that child triggers have not been inherited and allow users to turn this off if it is intentional. I need to think about it a bit more though, because Django does not try to issue any warnings for indexes or constraints that aren't inherited.

What I will absolutely do is add this to the docs in a section to better inform users of how it works. It's definitely not too user friendly that model Meta classes work like this

I frequently burn myself with meta inheritance so I think all the docs updates and warnings are a good idea.

In this case the PartioningMeta is a custom Meta class created by django-postgres-extra to do some special things and is not directly related to the Django Meta.

I'm not going to get as much time tonight as I thought to poke at this but I made some progress. Modifying get_prune_list cleaned up the registry but while this helps with my partitioned tables I'm not sure if it creates problems when triggers might be inherited in other situations. The tests still passed with my change.

This did not resolve the issue with ignore not working for my hard delete. I think that I need to change things in the opposite direction when doing the ignore by making sure that the inherited triggers are added to the ignore context and/or the ignore expands my ignore by adding the partition tables to it.

def get_prune_list(database=None):
    """Return triggers that will be pruned upon next full install

    Args:
        database (str, default=None): Only return results from this
            database. Defaults to returning results from all databases
    """
    installed = {
        (_quote(model._meta.db_table), trigger.get_pgid(model)) for model, trigger in get()
    }

    if isinstance(database, str):
        databases = [database]
    else:
        databases = database or settings.DATABASES

    prune_list = []
    for database in _postgres_databases(databases):
        with connections[database].cursor() as cursor:
            cursor.execute(
                'SELECT tgrelid::regclass, tgname, tgenabled'
                '    FROM pg_trigger'
                '    WHERE tgname LIKE \'pgtrigger_%\' AND tgparentid = 0'
            )
            triggers = set(cursor.fetchall())

        prune_list += [
            (trigger[0], trigger[1], trigger[2] == 'O', database)
            for trigger in triggers
            if (_quote(trigger[0]), trigger[1]) not in installed
        ]

    return prune_list

pgtrigger ls output

chat.Room:read_only default INSTALLED ENABLED chat.Room:soft_delete default INSTALLED ENABLED chat.Room:set_deactivated default INSTALLED ENABLED chat.Message:read_only default INSTALLED ENABLED chat.Message:soft_delete default INSTALLED ENABLED chat.Message:set_deactivated default INSTALLED ENABLED users.User:read_only default INSTALLED ENABLED users.User:soft_delete default INSTALLED ENABLED users.User:set_deactivated default INSTALLED ENABLED

@jzmiller1 ah I see, it totally didn't dawn on me that it was a complete different metaclass. I misread what was going on the first time.

I'm going to try out a partitioned model with django-postgres-extra and see what kinds of things happen. I did quite a few fixes for multi-schema support, but I'm not sure if those changes will help here

Heres my understanding of your problem:

1. You have a partitioned model with a soft delete trigger
2. pgtrigger.ignore doesn't seem to allow you to hard delete it

It also seems like there is an pgtrigger ls problem for all of those partitioned tables too, which is an issue if you are using management commands to install things.

I'm thinking the abstract model stuff may not matter here, but let me know if you think otherwise

Final question - how did you install triggers? I looked into the postgres-extra docs, and it looks like they have their own migration command, so it makes me wonder how you have things set up

I hope to have some more time soon to work on this!

I completely restarted my migrations and tested 4.0.0 with a new database and it has the same issues so the multischema support fixes are not impacting here as far as I can tell.

I don't think the abstract usage with django-pgtrigger is impacting things here. I think it all centers around triggers working slightly differently with partitions within postgres. The trigger against the logical table is handled as it should be via django-pgtrigger. The issue starts with the partitions inheriting the triggers applied to the logical table.

I'm not trying to do it but there's also the related issue of not having a way to handle creating a trigger on an individual partition via pgtrigger which is possible with postgres.

As far as installing triggers goes...django-postgres-extras has a command called pgmakemigrations that runs in place of make migrations to handle migrating the tables with partitions. So I run pgmakemigrations and then migrate. Here's the output migration files for the Message related models:

0001_initial

# Generated by Django 3.2.14 on 2022-08-15 12:43

from django.db import migrations, models
import psqlextra.backend.migrations.operations.add_default_partition
import psqlextra.backend.migrations.operations.create_partitioned_model
import psqlextra.manager.manager
import psqlextra.models.partitioned
import psqlextra.types


class Migration(migrations.Migration):

    initial = True

    dependencies = [
    ]

    operations = [
        psqlextra.backend.migrations.operations.create_partitioned_model.PostgresCreatePartitionedModel(
            name='Message',
            fields=[
                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('created', models.DateTimeField(auto_now_add=True)),
                ('modified', models.DateTimeField(auto_now=True)),
                ('is_active', models.BooleanField(default=True)),
                ('deactivated', models.DateTimeField(blank=True, null=True)),
                ('content', models.CharField(max_length=512)),
            ],
            options={
                'abstract': False,
                'base_manager_name': 'objects',
            },
            partitioning_options={
                'method': psqlextra.types.PostgresPartitioningMethod['RANGE'],
                'key': ['created'],
            },
            bases=(psqlextra.models.partitioned.PostgresPartitionedModel,),
            managers=[
                ('objects', psqlextra.manager.manager.PostgresManager()),
            ],
        ),
        psqlextra.backend.migrations.operations.add_default_partition.PostgresAddDefaultPartition(
            model_name='Message',
            name='default',
        ),
        migrations.CreateModel(
            name='Room',
            fields=[
                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('created', models.DateTimeField(auto_now_add=True)),
                ('modified', models.DateTimeField(auto_now=True)),
                ('is_active', models.BooleanField(default=True)),
                ('deactivated', models.DateTimeField(blank=True, null=True)),
                ('name', models.CharField(max_length=128)),
            ],
            options={
                'abstract': False,
                'base_manager_name': 'objects',
            },
            managers=[
                ('objects', psqlextra.manager.manager.PostgresManager()),
            ],
        ),
    ]

0002_initial.py

# Generated by Django 3.2.14 on 2022-08-15 12:43

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion
import pgtrigger
import pgtrigger.migrations


class Migration(migrations.Migration):

    initial = True

    dependencies = [
        ('chat', '0001_initial'),
        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
    ]

    operations = [
        migrations.AddField(
            model_name='room',
            name='online',
            field=models.ManyToManyField(blank=True, to=settings.AUTH_USER_MODEL),
        ),
        migrations.AddField(
            model_name='message',
            name='room',
            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='chat.room'),
        ),
        migrations.AddField(
            model_name='message',
            name='user',
            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
        ),
        pgtrigger.migrations.AddTrigger(
            model_name='room',
            trigger=pgtrigger.Protect(condition=pgtrigger.Q(('old__created__df', pgtrigger.F('new__created'))), declare=None, func=None, level=pgtrigger.Level(name='ROW'), name='read_only', operation=pgtrigger.Operation(name='UPDATE'), referencing=None, timing=None, when=pgtrigger.When(name='BEFORE')),
        ),
        pgtrigger.migrations.AddTrigger(
            model_name='room',
            trigger=pgtrigger.SoftDelete(condition=None, field='is_active', name='soft_delete', value=False),
        ),
        pgtrigger.migrations.AddTrigger(
            model_name='room',
            trigger=pgtrigger.Trigger(condition=pgtrigger.Q(('new__is_active', False), ('old__is_active', True)), declare=None, func='NEW.deactivated = NOW(); RETURN NEW;', level=pgtrigger.Level(name='ROW'), name='set_deactivated', operation=pgtrigger.Operation(name='UPDATE'), referencing=None, timing=None, when=pgtrigger.When(name='BEFORE')),
        ),
        pgtrigger.migrations.AddTrigger(
            model_name='message',
            trigger=pgtrigger.Protect(condition=pgtrigger.Q(('old__created__df', pgtrigger.F('new__created'))), declare=None, func=None, level=pgtrigger.Level(name='ROW'), name='read_only', operation=pgtrigger.Operation(name='UPDATE'), referencing=None, timing=None, when=pgtrigger.When(name='BEFORE')),
        ),
        pgtrigger.migrations.AddTrigger(
            model_name='message',
            trigger=pgtrigger.SoftDelete(condition=None, field='is_active', name='soft_delete', value=False),
        ),
        pgtrigger.migrations.AddTrigger(
            model_name='message',
            trigger=pgtrigger.Trigger(condition=pgtrigger.Q(('new__is_active', False), ('old__is_active', True)), declare=None, func='NEW.deactivated = NOW(); RETURN NEW;', level=pgtrigger.Level(name='ROW'), name='set_deactivated', operation=pgtrigger.Operation(name='UPDATE'), referencing=None, timing=None, when=pgtrigger.When(name='BEFORE')),
        ),
    ]

Thanks for the details @jzmiller1 !

One final clarification - Do the triggers not work on all/some of the partitions? Or is your issue solely related to ignoring triggers?

Hey, no problem! If you want any other info let me know.

As far as I can tell the triggers work fine on all of the partitions within Postgres. The only issues are the ignoring and the management commands seeing all of the inherited partition triggers as undeclared triggers that need to be pruned.

@jzmiller1 fixed in version 4.3!

In order to properly pull in the fix, you'll need to run python manage.py pgtrigger install one time to have the latest ignore function running.

Partitioning was tested in the test suite, and the pruning/ignoring bugs were reproduced and fixed.

Awesome! It is working for me. Thanks for the update!