MIME-Version: 1.0
References: <4280598e-cbe7-c5d3-fbce-fefce11da5f3@cmatte.me>
 <723b45bd-0c14-83eb-1911-b4f481e1b74c@cmatte.me>
In-Reply-To: <723b45bd-0c14-83eb-1911-b4f481e1b74c@cmatte.me>
From: Magnus Hagander <magnus@hagander.net>
Date: Wed, 27 Oct 2021 11:19:46 +0200
Message-ID: 
 <CABUevEyFpYPEHh0AAyTAsgymRKOOVA1SY_pDHPCbBQQ9BawfTA@mail.gmail.com>
Subject: Re: [PATCH] pgarchives: Bugfix: allow message's parentid to be null
To: =?UTF-8?Q?C=C3=A9lestin_Matte?= <celestin.matte@cmatte.me>
Cc: PostgreSQL WWW <pgsql-www@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="0000000000000236c105cf521972"
Archived-At: 
 <https://www.postgresql.org/message-id/CABUevEyFpYPEHh0AAyTAsgymRKOOVA1SY_pDHPCbBQQ9BawfTA%40mail.gmail.com>
Precedence: bulk

--0000000000000236c105cf521972
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Oct 26, 2021 at 8:31 PM C=C3=A9lestin Matte <celestin.matte@cmatte.=
me>
wrote:

> Attached: associated migration
>
> On 25/10/2021 16:52, C=C3=A9lestin 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

--0000000000000236c105cf521972
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Tue, Oct 26, 2021 at 8:31 PM C=C3=
=A9lestin Matte &lt;<a href=3D"mailto:celestin.matte@cmatte.me">celestin.ma=
tte@cmatte.me</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" sty=
le=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddi=
ng-left:1ex">Attached: associated migration<br>
<br>
On 25/10/2021 16:52, C=C3=A9lestin Matte wrote:<br>
&gt; Hello,<br>
&gt; <br>
&gt; Upon execution of load_messages.py, when creating a new thread, I get =
a crash:<br>
&gt; (Sorry about the format, I get this out of strace)<br>
&gt; <br>
&gt; Traceback (most recent call last):\n=C2=A0 File \&quot;/srv/pgarchives=
/local/loader/load_message.py\&quot;, lin\<br>
&gt; e 174, in &lt;module&gt;\n=C2=A0 =C2=A0 ap.store(conn, listid, opt.ove=
rwrite, opt.overwrite)\n=C2=A0 File \&quot;/srv/pgarchives/local/loader/li\=
<br>
&gt; b/storage.py\&quot;, line 232, in store\n=C2=A0 =C2=A0 &#39;rawtxt&#39=
;: bytearray(self.rawtxt),\npsycopg2.errors.NotNullViolation: null valu\<br=
>
&gt; e in column \&quot;parentid\&quot; violates not-null constraint\nDETAI=
L:=C2=A0 Failing row contains (2, 2, cmatte &lt;cmatte@[my_domain]\<br>
&gt; .tld&gt;, helloworld@[my_pglister_domain], , Test archive after redepl=
oyment 2, 2021-10-25 14:13:32+00, 1635171212.3\<br>
&gt; 49997.1068459.nullmailer@[my_domain], hello world\n\n\n\n, null, f, nu=
ll, \\x46726f6d2068656c6c6f776f726c642d6f776\<br>
&gt; e65722b617263686976654062..., &#39;2&#39;:5A &#39;after&#39;:3A &#39;a=
rchive&#39;:2A &#39;hello&#39;:6 &#39;redeployment&#39;:4A &#39;test...).\n=
\n<br>
&gt; <br>
&gt; I think the way to fix this is simply to allow message.parentid to be =
null.<br><br></blockquote><div><br></div><div>Yeah, this looks to be anothe=
r disreptency between the Django model and the schema.sql file. The table w=
hen created from schema.sql will allow nulls. So I can confirm that the fie=
ld *should* allow nulls, and the only reason it hasn&#39;t been an actual p=
roblem is that we never ever do any writes to the system through the django=
 model.=C2=A0</div><div><br></div><div>That said, I think the correct solut=
oin is still, as I mentioned in one of the other threads, to completely mov=
e everything into the initial migration, instead of doing a piecemeal=C2=A0=
hack that&#39;s going to result in a lot of small migrations for no real re=
ason. We should=C2=A0 just accept that the existing one is broken and repla=
ce it.</div><div><br></div><div>If you&#39;re willing to look into working =
on that I&#39;d be happy to review on that one, but if it seems a bit too m=
uch I can work on that one myself as well, but it might take a bit before I=
 have time to dig into it properly.</div><div><br></div><div>//Magnus</div>=
<div><br></div></div><div dir=3D"ltr" class=3D"gmail_signature"><div dir=3D=
"ltr"></div></div></div>

--0000000000000236c105cf521972--