Re: [Bug Fix]ECPG: cancellation of significant digits on ECPG

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Higuchi, Daisuke" <higuchi(dot)daisuke(at)jp(dot)fujitsu(dot)com>
Cc: "'Dmitry Dolgov'" <9erthalion6(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [Bug Fix]ECPG: cancellation of significant digits on ECPG
Date: 2018-11-12 23:07:20
Message-ID: 8107.1542064040@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Higuchi, Daisuke" <higuchi(dot)daisuke(at)jp(dot)fujitsu(dot)com> writes:
> From: Dmitry Dolgov [mailto:9erthalion6(at)gmail(dot)com]
>> Thanks for the patches. Unfortunately, judging from the cfbot.cputube.org they
>> can't be applied anymore to the current master, could you please rebase them?

> Thank you for checking!
> I rebased patches on the current master, so I attach them.

I took a quick look at this. I concur that the code is broken as-is;
it's failing to reproduce the state of the digit buffer accurately.
However, I think there's a second bug, which is that it doesn't even
try to duplicate the state when ndigits = 0. That basically means that
the sqlda area might contain dangling pointers (they'll point to the
PGTYPESnumeric_from_asc output, which is freed immediately after we
set up the sqlda copy). Now *maybe* that doesn't lead to any obvious
problem, but I don't think that this code deserves any expectation of
correctness given that you just found such a large bug in it. So
I think that we ought to unconditionally make the sqlda value's digit
buffer look just like the one we're copying, even when ndigits = 0,
which just requires removing the tests on ndigits.

I also noted that the comment adjacent to this code was badly obsolete.

In short, the attached. (I didn't bother to keep the test changes
separate from the code fix.)

regards, tom lane

Attachment Content-Type Size
ecpg_numeric_bug_fix_v3.patch text/x-diff 46.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-11-12 23:17:39 Re: Speeding up INSERTs and UPDATEs to partitioned tables
Previous Message Jürgen Strobel 2018-11-12 23:06:45 Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation