Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 17 Apr 2023 17:00:59 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: eclg -C ORACLE breaks data
Message-ID: <ZDz8u6Aswu3PGn33@paquier.xyz>
References: <20230410.173500.440060475837236886.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="CkUq6Dy9GWog6ivg"
Content-Disposition: inline
In-Reply-To: <20230410.173500.440060475837236886.horikyota.ntt@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZDz8u6Aswu3PGn33%40paquier.xyz>
Precedence: bulk


--CkUq6Dy9GWog6ivg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.
--
Michael

--CkUq6Dy9GWog6ivg
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmQ8/LsACgkQnvQgOdby
QH0qmhAAia0XTL4U7o4p+3Mki98ipYkWdfzqkEcg3Ey3qW2eMicQXS6+FjGTNApp
0GYxJvOXPlNtgcgdMXm1KTnLCwIONVtJTauYALRrKV71zu4cqqX+EO4mRdZmgMA0
TpNO/MuonhFQF3ePW/DAJg4tWtq+q+kHylnEnD70jZAj2rqFth1HN6AnoM9dLrIs
xX238uICz8p+m+Uw9G4QZvC7mwUUUW9AwMKK6KozIZUYH9slWVYqz50OqtxWNDz2
CD+a6VDLQSCsASG07icwIZPnPom7coEy0wnMt0iVn4ASigpvmE7qhJOacFkRsBmu
qmNRAIuds73Bs1pExuNbdEi1vZp5ir0l9gHW5Mtcg48yZXuwO+795sbZ/8778u0L
Bsf6G0X6wINIsfDNHVjMCP/CdbCNGF6W5nPC1qq8GdZiJ4BqWGH3q/u8MrtdJTGh
ZiQ+Mj7nBc+0CSXuACcc+czS/QE/3qxmUdQTVifShqzvT+LVzg10wZjK/CfgezDF
st1hELP4Z6A9K4pyxfpy75UguL6KoBS1AWJGZ/4roLDMXos55lb/aNrif4U/h+68
tkQIkcAl9+LGk0TTc4sQtInrbEJhrFbUGyo0p/Iag7bInXdCt9uFmpkGwfq2U3zU
FFzhnzhPoS4NlFmrnrY2in5BC44sain3QuPQ35o1oQQ+sK1hYnA=
=eJBo
-----END PGP SIGNATURE-----

--CkUq6Dy9GWog6ivg--