Date: Mon, 17 Apr 2023 17:47:41 +0900 (JST)
Message-Id: <20230417.174741.829927450239624934.horikyota.ntt@gmail.com>
To: michael@paquier.xyz
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: eclg -C ORACLE breaks data
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <ZDz8u6Aswu3PGn33@paquier.xyz>
References: <20230410.173500.440060475837236886.horikyota.ntt@gmail.com>
	<ZDz8u6Aswu3PGn33@paquier.xyz>
User-Agent: Mew version 6.8 on Emacs 27.2
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20230417.174741.829927450239624934.horikyota.ntt%40gmail.com>
Precedence: bulk

(sorry for the wrong subject..)

At Mon, 17 Apr 2023 17:00:59 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Mon, Apr 10, 2023 at 05:35:00PM +0900, Kyotaro Horiguchi wrote:
> > This results in overwriting str[-1], the last byte of the preceding
> > numeric in this case, with 0x00, representing the digit '0'. When
> > callers of ecpg_get_data() explicitly pass zero as varcharsize, they
> > provide storage that precisely fitting the data.
> 
> Good find, that's clearly wrong.  The test case is interesting.  On
> HEAD, the processing of the second field eats up the data of the first
> field.
>
> > However, it remains
> > uncertain if this assumption is valid when ecpg_store_result() passes
> > var->varcharsize which is also zero. Consequently, the current fix
> > presumes exact-fit storage when varcharsize is zero.
> 
> Based on what I can read in sqlda.c (ecpg_set_compat_sqlda() and
> ecpg_set_native_sqlda()), the data length calculated adds an extra
> byte to the data length when storing the data references in sqldata.
> execute.c and ecpg_store_result() is actually much trickier than that
> (see particularly the part where the code does an "allocate memory for
> NULL pointers", where varcharsize could also be 0), still I agree that
> this assumption should be OK.  The code is as it is for many years,
> with its logic to do an estimation of allocation first, and then read
> the data at once in the whole area allocated..
> 
> This thinko has been introduced by 3b7ab43, so this needs to go down
> to v11.  I'll see to that.

Thanks for picking this up.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center