Thread: [COMMITTERS] pgsql: Fix collprovider of predefined collations
Fix collprovider of predefined collations An earlier version of the patch had collprovider as an integer and thus set these to 0, but the correct setting is now null. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/ec7129b7812ce276520f749d0946875663c34093 Modified Files -------------- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_collation.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
Peter Eisentraut <peter_e@gmx.net> writes:
> Fix collprovider of predefined collations
> An earlier version of the patch had collprovider as an integer and thus
> set these to 0, but the correct setting is now null.
Surely this is not right. Neither collprovider nor the other fixed-length
fields following it in pg_collation are marked nullable, and most of them
are accessed as struct fields so that's not an easy thing to change.
The fact that the buildfarm didn't blow up is a bit odd. Maybe we're
missing some enforcement somewhere?
regards, tom lane
I wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> Fix collprovider of predefined collations
>> An earlier version of the patch had collprovider as an integer and thus
>> set these to 0, but the correct setting is now null.
> Surely this is not right. Neither collprovider nor the other fixed-length
> fields following it in pg_collation are marked nullable, and most of them
> are accessed as struct fields so that's not an easy thing to change.
Ah, on looking closer, I see what the patch actually changes to null is
collversion not collprovider. So the patch is ok, the commit message
not so much.
> The fact that the buildfarm didn't blow up is a bit odd. Maybe we're
> missing some enforcement somewhere?
As penance for the false alarm, I looked into that angle and indeed we
didn't have any cross-check against storing NULL from the BKI data into
a putatively not null column. We do now.
regards, tom lane
On 6/13/17 10:57, Tom Lane wrote: > I wrote: >> Peter Eisentraut <peter_e@gmx.net> writes: >>> Fix collprovider of predefined collations >>> An earlier version of the patch had collprovider as an integer and thus >>> set these to 0, but the correct setting is now null. > >> Surely this is not right. Neither collprovider nor the other fixed-length >> fields following it in pg_collation are marked nullable, and most of them >> are accessed as struct fields so that's not an easy thing to change. > > Ah, on looking closer, I see what the patch actually changes to null is > collversion not collprovider. So the patch is ok, the commit message > not so much. yup :( >> The fact that the buildfarm didn't blow up is a bit odd. Maybe we're >> missing some enforcement somewhere? > > As penance for the false alarm, I looked into that angle and indeed we > didn't have any cross-check against storing NULL from the BKI data into > a putatively not null column. We do now. great -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services