public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH] pgarchives: Bugfix: allow message's parentid to be null
5+ messages / 2 participants
[nested] [flat]

* [PATCH] pgarchives: Bugfix: allow message's parentid to be null
@ 2021-10-25 14:52 Célestin Matte <[email protected]>
  2021-10-26 18:31 ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
  0 siblings, 1 reply; 5+ messages in thread

From: Célestin Matte @ 2021-10-25 14:52 UTC (permalink / raw)
  To: PostgreSQL WWW <[email protected]>

Hello,

Upon execution of load_messages.py, when creating a new thread, I get a crash:
(Sorry about the format, I get this out of strace)

Traceback (most recent call last):\n  File \"/srv/pgarchives/local/loader/load_message.py\", lin\
e 174, in <module>\n    ap.store(conn, listid, opt.overwrite, opt.overwrite)\n  File \"/srv/pgarchives/local/loader/li\
b/storage.py\", line 232, in store\n    'rawtxt': bytearray(self.rawtxt),\npsycopg2.errors.NotNullViolation: null valu\
e in column \"parentid\" violates not-null constraint\nDETAIL:  Failing row contains (2, 2, cmatte <cmatte@[my_domain]\
.tld>, helloworld@[my_pglister_domain], , Test archive after redeployment 2, 2021-10-25 14:13:32+00, 1635171212.3\
49997.1068459.nullmailer@[my_domain], hello world\n\n\n\n, null, f, null, \\x46726f6d2068656c6c6f776f726c642d6f776\
e65722b617263686976654062..., '2':5A 'after':3A 'archive':2A 'hello':6 'redeployment':4A 'test...).\n\n

I think the way to fix this is simply to allow message.parentid to be null.

Cheers,
-- 
Célestin Matte

Attachments:

  [text/x-patch] 0001-Bugfix-allow-message-s-parentid-to-be-null.patch (1.1K, 2-0001-Bugfix-allow-message-s-parentid-to-be-null.patch)
  download | inline diff:
From ba3852df7a69a1a25dd366fd3657fce3740cf23f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Mon, 25 Oct 2021 16:42:49 +0200
Subject: [PATCH] Bugfix: allow message's parentid to be null

Otherwise, load_message.py will crash upon creation of a new thread
---
 django/archives/mailarchives/models.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/django/archives/mailarchives/models.py b/django/archives/mailarchives/models.py
index 44c4469..879d280 100644
--- a/django/archives/mailarchives/models.py
+++ b/django/archives/mailarchives/models.py
@@ -25,7 +25,7 @@ class Message(models.Model):
     messageid = models.TextField(null=False)
     bodytxt = models.TextField(null=False)
     # rawtxt is a bytea field, which django doesn't support (easily)
-    parentid = models.IntegerField(null=False, blank=False)
+    parentid = models.IntegerField(null=True, blank=False)
     has_attachment = models.BooleanField(null=False, default=False)
     hiddenstatus = models.IntegerField(null=True)
     # fti is a tsvector field, which django doesn't support (easily)
-- 
2.33.1



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null
  2021-10-25 14:52 [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
@ 2021-10-26 18:31 ` Célestin Matte <[email protected]>
  2021-10-27 09:19   ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Magnus Hagander <[email protected]>
  0 siblings, 1 reply; 5+ messages in thread

From: Célestin Matte @ 2021-10-26 18:31 UTC (permalink / raw)
  To: [email protected]

Attached: associated migration

On 25/10/2021 16:52, Célestin Matte wrote:
> Hello,
> 
> Upon execution of load_messages.py, when creating a new thread, I get a crash:
> (Sorry about the format, I get this out of strace)
> 
> Traceback (most recent call last):\n  File \"/srv/pgarchives/local/loader/load_message.py\", lin\
> e 174, in <module>\n    ap.store(conn, listid, opt.overwrite, opt.overwrite)\n  File \"/srv/pgarchives/local/loader/li\
> b/storage.py\", line 232, in store\n    'rawtxt': bytearray(self.rawtxt),\npsycopg2.errors.NotNullViolation: null valu\
> e in column \"parentid\" violates not-null constraint\nDETAIL:  Failing row contains (2, 2, cmatte <cmatte@[my_domain]\
> .tld>, helloworld@[my_pglister_domain], , Test archive after redeployment 2, 2021-10-25 14:13:32+00, 1635171212.3\
> 49997.1068459.nullmailer@[my_domain], hello world\n\n\n\n, null, f, null, \\x46726f6d2068656c6c6f776f726c642d6f776\
> e65722b617263686976654062..., '2':5A 'after':3A 'archive':2A 'hello':6 'redeployment':4A 'test...).\n\n
> 
> I think the way to fix this is simply to allow message.parentid to be null.
> 
> Cheers,
> 


-- 
Célestin Matte

Attachments:

  [text/x-patch] 0002-Add-migration-associated-with-ba3852df-allow-parenti.patch (1.1K, 2-0002-Add-migration-associated-with-ba3852df-allow-parenti.patch)
  download | inline diff:
From 9049a67567c568ee3a35217ee620d40cb567d8ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Tue, 26 Oct 2021 20:29:58 +0200
Subject: [PATCH 2/2] Add migration associated with ba3852df (allow parentid to
 be null)

---
 .../migrations/0005_auto_20211026_1827.py      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 django/archives/mailarchives/migrations/0005_auto_20211026_1827.py

diff --git a/django/archives/mailarchives/migrations/0005_auto_20211026_1827.py b/django/archives/mailarchives/migrations/0005_auto_20211026_1827.py
new file mode 100644
index 0000000..95e81ed
--- /dev/null
+++ b/django/archives/mailarchives/migrations/0005_auto_20211026_1827.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.24 on 2021-10-26 18:27
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('mailarchives', '0004_resend_rate_limit'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='message',
+            name='parentid',
+            field=models.IntegerField(null=True),
+        ),
+    ]
-- 
2.33.1



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null
  2021-10-25 14:52 [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
  2021-10-26 18:31 ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
@ 2021-10-27 09:19   ` Magnus Hagander <[email protected]>
  2021-10-27 10:04     ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
  0 siblings, 1 reply; 5+ messages in thread

From: Magnus Hagander @ 2021-10-27 09:19 UTC (permalink / raw)
  To: Célestin Matte <[email protected]>; +Cc: PostgreSQL WWW <[email protected]>

On Tue, Oct 26, 2021 at 8:31 PM Célestin Matte <[email protected]>
wrote:

> Attached: associated migration
>
> On 25/10/2021 16:52, Célestin Matte wrote:
> > Hello,
> >
> > Upon execution of load_messages.py, when creating a new thread, I get a
> crash:
> > (Sorry about the format, I get this out of strace)
> >
> > Traceback (most recent call last):\n  File
> \"/srv/pgarchives/local/loader/load_message.py\", lin\
> > e 174, in <module>\n    ap.store(conn, listid, opt.overwrite,
> opt.overwrite)\n  File \"/srv/pgarchives/local/loader/li\
> > b/storage.py\", line 232, in store\n    'rawtxt':
> bytearray(self.rawtxt),\npsycopg2.errors.NotNullViolation: null valu\
> > e in column \"parentid\" violates not-null constraint\nDETAIL:  Failing
> row contains (2, 2, cmatte <cmatte@[my_domain]\
> > .tld>, helloworld@[my_pglister_domain], , Test archive after
> redeployment 2, 2021-10-25 14:13:32+00, 1635171212.3\
> > 49997.1068459.nullmailer@[my_domain], hello world\n\n\n\n, null, f,
> null, \\x46726f6d2068656c6c6f776f726c642d6f776\
> > e65722b617263686976654062..., '2':5A 'after':3A 'archive':2A 'hello':6
> 'redeployment':4A 'test...).\n\n
> >
> > I think the way to fix this is simply to allow message.parentid to be
> null.
>
>
Yeah, this looks to be another disreptency between the Django model and the
schema.sql file. The table when created from schema.sql will allow nulls.
So I can confirm that the field *should* allow nulls, and the only reason
it hasn't been an actual problem is that we never ever do any writes to the
system through the django model.

That said, I think the correct solutoin is still, as I mentioned in one of
the other threads, to completely move everything into the initial
migration, instead of doing a piecemeal hack that's going to result in a
lot of small migrations for no real reason. We should  just accept that the
existing one is broken and replace it.

If you're willing to look into working on that I'd be happy to review on
that one, but if it seems a bit too much I can work on that one myself as
well, but it might take a bit before I have time to dig into it properly.

//Magnus


^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null
  2021-10-25 14:52 [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
  2021-10-26 18:31 ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
  2021-10-27 09:19   ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Magnus Hagander <[email protected]>
@ 2021-10-27 10:04     ` Célestin Matte <[email protected]>
  2021-10-27 13:50       ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Magnus Hagander <[email protected]>
  0 siblings, 1 reply; 5+ messages in thread

From: Célestin Matte @ 2021-10-27 10:04 UTC (permalink / raw)
  To: [email protected]

> 
> That said, I think the correct solutoin is still, as I mentioned in one of the other threads, to completely move everything into the initial migration, instead of doing a piecemeal hack that's going to result in a lot of small migrations for no real reason. We should  just accept that the existing one is broken and replace it.

Can everything be moved to django's model though? The schema.sql file defines several other components (indexes, text search configurations, text search dictionaries, functions, triggers) and I'm not sure django is able to handle all of that by itself. Is it? Or do you only want to move tables?

Cheers,
-- 
Célestin Matte





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null
  2021-10-25 14:52 [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
  2021-10-26 18:31 ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
  2021-10-27 09:19   ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Magnus Hagander <[email protected]>
  2021-10-27 10:04     ` Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
@ 2021-10-27 13:50       ` Magnus Hagander <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Magnus Hagander @ 2021-10-27 13:50 UTC (permalink / raw)
  To: Célestin Matte <[email protected]>; +Cc: PostgreSQL WWW <[email protected]>

On Wed, Oct 27, 2021 at 12:04 PM Célestin Matte <[email protected]>
wrote:

> >
> > That said, I think the correct solutoin is still, as I mentioned in one
> of the other threads, to completely move everything into the initial
> migration, instead of doing a piecemeal hack that's going to result in a
> lot of small migrations for no real reason. We should  just accept that the
> existing one is broken and replace it.
>
> Can everything be moved to django's model though? The schema.sql file
> defines several other components (indexes, text search configurations, text
> search dictionaries, functions, triggers) and I'm not sure django is able
> to handle all of that by itself. Is it? Or do you only want to move tables?
>

The models can't handle them, but migrations can.

I'm thinking similar to pglister, where we use django models to create the
parts that *can* be created, and then RunSQL() migrations to do the rest.
That way the pure SQL parts can still run as part of the same flow, and can
be mixed in with the django generated ones. The end result in the database
should be the same, but avoiding to have to maintain a separate .sql file
for it to keep in sync.

I suggest moving everything, for that reason -- to ensure the order of
application.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/;
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/;


^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2021-10-27 13:50 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 14:52 [PATCH] pgarchives: Bugfix: allow message's parentid to be null Célestin Matte <[email protected]>
2021-10-26 18:31 ` Célestin Matte <[email protected]>
2021-10-27 09:19   ` Magnus Hagander <[email protected]>
2021-10-27 10:04     ` Célestin Matte <[email protected]>
2021-10-27 13:50       ` Magnus Hagander <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox