Re: [HACKERS] btree_gist macaddr valgrind woes
On Sat, May 17, 2014 at 11:46:39PM +0300, Heikki Linnakangas wrote: AFAICS, what we have to do is mark the wider gbtreekeyNN types as requiring double alignment. This will break pg_upgrade'ing any index in which they're used as non-first columns, unless perhaps all the preceding columns have at least double size/alignment. I guess pg_upgrade can check for that, but it'll be kind of a pain. Another issue is what the heck btree_gist's extension upgrade script ought to do about this. It can't just go and modify the type declarations. Actually, on further thought, this isn't an issue for pg_upgrade at all, just for the extension upgrade script. Maybe we just have to make it poke through the catalogs looking for at-risk indexes, and refuse to complete the extension upgrade if there are any? I think it would be best to just not allow pg_upgrade if there are any indexes using the ill-defined types. The upgrade script could then simply drop the types and re-create them. The DBA would need to drop the affected indexes before upgrade and re-create them afterwards, but I think that would be acceptable. I doubt there are many people using btree_gist on int8 or float8 columns. Another way to attack this would be to change the code to memcpy() the values before accessing them. That would be ugly, but it would be back-patchable. In HEAD, I'd rather bite the bullet and get the catalogs fixed, though. What did we decide about this issue/fix and pg_upgrade? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
Heikki Linnakangas wrote: On 05/18/2014 12:23 AM, Tom Lane wrote: A larger issue is that we evidently have no buildfarm animals that are picky about alignment, or at least none that are running a modern-enough buildfarm script to be running the contrib/logical_decoding test. That seems like a significant gap. I don't want to volunteer to run a critter on my HPPA box: it's old enough, and eats enough electricity, that I no longer want to leave it on 24x7. Plus a lot of the time its response to a bus error is to lock up in a tight loop rather than report an error, so a failure wouldn't get reported usefully by the buildfarm anyway. Does anyone have an ARM or PPC box where they can configure the kernel not to mask misaligned fetches? I did echo 4 /proc/cpu/alignment on chipmunk - let's see what it crops up. In quick testing with a little test program, it looks like an unaligned access to a 32-bit int still works without error. But an unaligned access to a 64-bit long long causes a SIGBUS now. There seems to have been no failure here, FWIW. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
On 05/18/2014 12:23 AM, Tom Lane wrote: A larger issue is that we evidently have no buildfarm animals that are picky about alignment, or at least none that are running a modern-enough buildfarm script to be running the contrib/logical_decoding test. That seems like a significant gap. I don't want to volunteer to run a critter on my HPPA box: it's old enough, and eats enough electricity, that I no longer want to leave it on 24x7. Plus a lot of the time its response to a bus error is to lock up in a tight loop rather than report an error, so a failure wouldn't get reported usefully by the buildfarm anyway. Does anyone have an ARM or PPC box where they can configure the kernel not to mask misaligned fetches? I did echo 4 /proc/cpu/alignment on chipmunk - let's see what it crops up. In quick testing with a little test program, it looks like an unaligned access to a 32-bit int still works without error. But an unaligned access to a 64-bit long long causes a SIGBUS now. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
I wrote: BTW, the *real* problem with all this stuff is that the gbtreekeyNN types are declared as having int alignment, even though some of the opclasses store double-aligned types in them. I imagine it's possible to provoke bus errors on machines that are picky about alignment. The first column of an index is safe enough because index tuples will be double-aligned anyway, but it seems like there's a hazard for lower-order columns. And indeed there is: regression=# create extension btree_gist ; CREATE EXTENSION regression=# create table tt (f1 int2, f2 float8); CREATE TABLE regression=# create index on tt using gist(f1,f2); CREATE INDEX regression=# insert into tt values(1,2); INSERT 0 1 regression=# insert into tt values(2,4); INSERT 0 1 regression=# set enable_seqscan TO 0; SET regression=# explain select * from tt where f1=2::int2 and f2=4; QUERY PLAN Index Scan using tt_f1_f2_idx on tt (cost=0.14..8.16 rows=1 width=10) Index Cond: ((f1 = 2::smallint) AND (f2 = 4::double precision)) Planning time: 1.043 ms (3 rows) regression=# select * from tt where f1=2::int2 and f2=4; bus error This is something we cannot fix compatibly :-( AFAICS, what we have to do is mark the wider gbtreekeyNN types as requiring double alignment. This will break pg_upgrade'ing any index in which they're used as non-first columns, unless perhaps all the preceding columns have at least double size/alignment. I guess pg_upgrade can check for that, but it'll be kind of a pain. Another issue is what the heck btree_gist's extension upgrade script ought to do about this. It can't just go and modify the type declarations. Actually, on further thought, this isn't an issue for pg_upgrade at all, just for the extension upgrade script. Maybe we just have to make it poke through the catalogs looking for at-risk indexes, and refuse to complete the extension upgrade if there are any? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
On 05/17/2014 11:12 PM, Tom Lane wrote: I wrote: BTW, the *real* problem with all this stuff is that the gbtreekeyNN types are declared as having int alignment, even though some of the opclasses store double-aligned types in them. I imagine it's possible to provoke bus errors on machines that are picky about alignment. The first column of an index is safe enough because index tuples will be double-aligned anyway, but it seems like there's a hazard for lower-order columns. And indeed there is: regression=# create extension btree_gist ; CREATE EXTENSION regression=# create table tt (f1 int2, f2 float8); CREATE TABLE regression=# create index on tt using gist(f1,f2); CREATE INDEX regression=# insert into tt values(1,2); INSERT 0 1 regression=# insert into tt values(2,4); INSERT 0 1 regression=# set enable_seqscan TO 0; SET regression=# explain select * from tt where f1=2::int2 and f2=4; QUERY PLAN Index Scan using tt_f1_f2_idx on tt (cost=0.14..8.16 rows=1 width=10) Index Cond: ((f1 = 2::smallint) AND (f2 = 4::double precision)) Planning time: 1.043 ms (3 rows) regression=# select * from tt where f1=2::int2 and f2=4; bus error This is something we cannot fix compatibly :-( AFAICS, what we have to do is mark the wider gbtreekeyNN types as requiring double alignment. This will break pg_upgrade'ing any index in which they're used as non-first columns, unless perhaps all the preceding columns have at least double size/alignment. I guess pg_upgrade can check for that, but it'll be kind of a pain. Another issue is what the heck btree_gist's extension upgrade script ought to do about this. It can't just go and modify the type declarations. Actually, on further thought, this isn't an issue for pg_upgrade at all, just for the extension upgrade script. Maybe we just have to make it poke through the catalogs looking for at-risk indexes, and refuse to complete the extension upgrade if there are any? I think it would be best to just not allow pg_upgrade if there are any indexes using the ill-defined types. The upgrade script could then simply drop the types and re-create them. The DBA would need to drop the affected indexes before upgrade and re-create them afterwards, but I think that would be acceptable. I doubt there are many people using btree_gist on int8 or float8 columns. Another way to attack this would be to change the code to memcpy() the values before accessing them. That would be ugly, but it would be back-patchable. In HEAD, I'd rather bite the bullet and get the catalogs fixed, though. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
On 2014-05-17 23:46:39 +0300, Heikki Linnakangas wrote: On 05/17/2014 11:12 PM, Tom Lane wrote: I wrote: BTW, the *real* problem with all this stuff is that the gbtreekeyNN types are declared as having int alignment, even though some of the opclasses store double-aligned types in them. I imagine it's possible to provoke bus errors on machines that are picky about alignment. The first column of an index is safe enough because index tuples will be double-aligned anyway, but it seems like there's a hazard for lower-order columns. And indeed there is: regression=# create extension btree_gist ; CREATE EXTENSION regression=# create table tt (f1 int2, f2 float8); CREATE TABLE regression=# create index on tt using gist(f1,f2); CREATE INDEX regression=# insert into tt values(1,2); INSERT 0 1 regression=# insert into tt values(2,4); INSERT 0 1 regression=# set enable_seqscan TO 0; SET regression=# explain select * from tt where f1=2::int2 and f2=4; QUERY PLAN Index Scan using tt_f1_f2_idx on tt (cost=0.14..8.16 rows=1 width=10) Index Cond: ((f1 = 2::smallint) AND (f2 = 4::double precision)) Planning time: 1.043 ms (3 rows) regression=# select * from tt where f1=2::int2 and f2=4; bus error This is something we cannot fix compatibly :-( Too bad. Another issue is what the heck btree_gist's extension upgrade script ought to do about this. It can't just go and modify the type declarations. Actually, on further thought, this isn't an issue for pg_upgrade at all, just for the extension upgrade script. Maybe we just have to make it poke through the catalogs looking for at-risk indexes, and refuse to complete the extension upgrade if there are any? Hm. I guess it could just add a C function to do that job. I think pg_upgrade already has some. I think it would be best to just not allow pg_upgrade if there are any indexes using the ill-defined types. The upgrade script could then simply drop the types and re-create them. The DBA would need to drop the affected indexes before upgrade and re-create them afterwards, but I think that would be acceptable. I doubt there are many people using btree_gist on int8 or float8 columns. I don't think it's that unlikely. It can make a fair amount of sense to have multicolumn indexes where one columns is over an int8 and the other over the datatype requiring the gist index to be used. I've certainly done that. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
Andres Freund and...@2ndquadrant.com writes: On 2014-05-17 23:46:39 +0300, Heikki Linnakangas wrote: I doubt there are many people using btree_gist on int8 or float8 columns. I don't think it's that unlikely. It can make a fair amount of sense to have multicolumn indexes where one columns is over an int8 and the other over the datatype requiring the gist index to be used. I've certainly done that. Yeah; the whole point of having these opclasses at all is to allow a GIST index to index a scalar column along with some GIST-only functionality. However, given the lack of field complaints, it seems likely that nobody is trying to do that on non-Intel architectures; or at least, not on a non-Intel architecture where the kernel is unwilling to mask the problem via trap emulation. So my feeling about it is we don't need to try to back-patch a fix. I'd like to see it fixed in HEAD, though. A larger issue is that we evidently have no buildfarm animals that are picky about alignment, or at least none that are running a modern-enough buildfarm script to be running the contrib/logical_decoding test. That seems like a significant gap. I don't want to volunteer to run a critter on my HPPA box: it's old enough, and eats enough electricity, that I no longer want to leave it on 24x7. Plus a lot of the time its response to a bus error is to lock up in a tight loop rather than report an error, so a failure wouldn't get reported usefully by the buildfarm anyway. Does anyone have an ARM or PPC box where they can configure the kernel not to mask misaligned fetches? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] btree_gist macaddr valgrind woes
Hi, After Heikki has fixed the bit btree_gist bugs my valgrind run shows a bug in macaddr. Presumably it's because the type is a fixed width type with a length of 6. I guess it's allocating stuff sizeof(macaddr) but then passes the result around as a Datum which doesn't work well. ==14219== Invalid read of size 8 ==14219==at 0x4C2BB50: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:882) ==14219==by 0x45FA01: heap_fill_tuple (heaptuple.c:248) ==14219==by 0x4629A7: index_form_tuple (indextuple.c:132) ==14219==by 0x48ED05: gistFormTuple (gistutil.c:611) ==14219==by 0x499801: gistBuildCallback (gistbuild.c:476) ==14219==by 0x530412: IndexBuildHeapScan (index.c:2460) ==14219==by 0x4991D4: gistbuild (gistbuild.c:209) ==14219==by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650) ==14219==by 0x52F816: index_build (index.c:1962) ==14219==by 0x52E79D: index_create (index.c:1083) ==14219==by 0x5E9253: DefineIndex (indexcmds.c:600) ==14219==by 0x7A4DED: ProcessUtilitySlow (utility.c:1149) ==14219== Address 0x67d37a0 is 8 bytes inside a block of size 12 client-defined ==14219==at 0x8FB83A: palloc0 (mcxt.c:698) ==14219==by 0x6B6C3C4: gbt_num_compress (btree_utils_num.c:31) ==14219==by 0x6B74BF1: gbt_macad_compress (btree_macaddr.c:113) ==14219==by 0x8D73E2: FunctionCall1Coll (fmgr.c:1298) ==14219==by 0x48EB39: gistcentryinit (gistutil.c:576) ==14219==by 0x48ECA1: gistFormTuple (gistutil.c:603) ==14219==by 0x499801: gistBuildCallback (gistbuild.c:476) ==14219==by 0x530412: IndexBuildHeapScan (index.c:2460) ==14219==by 0x4991D4: gistbuild (gistbuild.c:209) ==14219==by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650) ==14219==by 0x52F816: index_build (index.c:1962) ==14219==by 0x52E79D: index_create (index.c:1083) ==14219== ==14219== Invalid read of size 8 ==14219==at 0x4C2BA70: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:882) ==14219==by 0x45FA01: heap_fill_tuple (heaptuple.c:248) ==14219==by 0x4629A7: index_form_tuple (indextuple.c:132) ==14219==by 0x48ED05: gistFormTuple (gistutil.c:611) ==14219==by 0x48C755: gistSplit (gist.c:1314) ==14219==by 0x488ED4: gistplacetopage (gist.c:242) ==14219==by 0x48C025: gistinserttuples (gist.c:1134) ==14219==by 0x48BFAA: gistinserttuple (gist.c:1093) ==14219==by 0x48AC56: gistdoinsert (gist.c:750) ==14219==by 0x49985B: gistBuildCallback (gistbuild.c:490) ==14219==by 0x530412: IndexBuildHeapScan (index.c:2460) ==14219==by 0x4991D4: gistbuild (gistbuild.c:209) ==14219== Address 0x67dba58 is 8 bytes inside a block of size 12 client-defined ==14219==at 0x8FB6D2: palloc (mcxt.c:678) ==14219==by 0x6B74D04: gbt_macad_union (btree_macaddr.c:145) ==14219==by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324) ==14219==by 0x48D7C8: gistMakeUnionItVec (gistutil.c:198) ==14219==by 0x496E8E: gistunionsubkeyvec (gistsplit.c:64) ==14219==by 0x496F05: gistunionsubkey (gistsplit.c:91) ==14219==by 0x498981: gistSplitByKey (gistsplit.c:689) ==14219==by 0x48C423: gistSplit (gist.c:1270) ==14219==by 0x488ED4: gistplacetopage (gist.c:242) ==14219==by 0x48C025: gistinserttuples (gist.c:1134) ==14219==by 0x48BFAA: gistinserttuple (gist.c:1093) ==14219==by 0x48AC56: gistdoinsert (gist.c:750) ==14219== ==14250== Uninitialised byte(s) found during client check request ==14250==at 0x794E9F: PageAddItem (bufpage.c:314) ==14250==by 0x48CC95: gistfillbuffer (gistutil.c:42) ==14250==by 0x489CBC: gistplacetopage (gist.c:451) ==14250==by 0x48C025: gistinserttuples (gist.c:1134) ==14250==by 0x48C31C: gistfinishsplit (gist.c:1240) ==14250==by 0x48C0F5: gistinserttuples (gist.c:1159) ==14250==by 0x48BFAA: gistinserttuple (gist.c:1093) ==14250==by 0x48AC56: gistdoinsert (gist.c:750) ==14250==by 0x49985B: gistBuildCallback (gistbuild.c:490) ==14250==by 0x530412: IndexBuildHeapScan (index.c:2460) ==14250==by 0x4991D4: gistbuild (gistbuild.c:209) ==14250==by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650) ==14250== Address 0x6efbcfb is 19 bytes inside a block of size 40 client-defined ==14250==at 0x8FB83A: palloc0 (mcxt.c:698) ==14250==by 0x462954: index_form_tuple (indextuple.c:129) ==14250==by 0x48ED05: gistFormTuple (gistutil.c:611) ==14250==by 0x48C618: gistSplit (gist.c:1292) ==14250==by 0x488ED4: gistplacetopage (gist.c:242) ==14250==by 0x48C025: gistinserttuples (gist.c:1134) ==14250==by 0x48BFAA: gistinserttuple (gist.c:1093) ==14250==by 0x48AC56: gistdoinsert (gist.c:750) ==14250==by 0x49985B: gistBuildCallback (gistbuild.c:490) ==14250==by 0x530412: IndexBuildHeapScan (index.c:2460) ==14250==by 0x4991D4: gistbuild (gistbuild.c:209) ==14250==by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650) ==14250== Uninitialised value was created by a heap allocation ==14250==at 0x8FB6D2: palloc (mcxt.c:678) ==14250==by 0x6B6CFCE: gbt_var_key_copy
Re: [HACKERS] btree_gist macaddr valgrind woes
On 05/16/2014 01:28 PM, Andres Freund wrote: Hi, After Heikki has fixed the bit btree_gist bugs my valgrind run shows a bug in macaddr. Presumably it's because the type is a fixed width type with a length of 6. I guess it's allocating stuff sizeof(macaddr) but then passes the result around as a Datum which doesn't work well. The complaints seem to be coming from all the varlen data types, too, not just macaddr: ... ==14219==by 0x6B74BF1: gbt_macad_compress (btree_macaddr.c:113) ... ==14250==by 0x6B75C45: gbt_text_picksplit (btree_text.c:215) ... ==14280==by 0x6B76063: gbt_bytea_picksplit (btree_bytea.c:143) ... ==14292==by 0x6B76591: gbt_bit_union (btree_bit.c:173) ... I'm not seeing those valgrind warnings when I run it. Can you give more details on how to reproduce? - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
On 2014-05-16 17:08:40 +0300, Heikki Linnakangas wrote: On 05/16/2014 01:28 PM, Andres Freund wrote: Hi, After Heikki has fixed the bit btree_gist bugs my valgrind run shows a bug in macaddr. Presumably it's because the type is a fixed width type with a length of 6. I guess it's allocating stuff sizeof(macaddr) but then passes the result around as a Datum which doesn't work well. The complaints seem to be coming from all the varlen data types, too, not just macaddr: Hm, right. Didn't look very far yet. ... ==14219==by 0x6B74BF1: gbt_macad_compress (btree_macaddr.c:113) ... ==14250==by 0x6B75C45: gbt_text_picksplit (btree_text.c:215) ... ==14280==by 0x6B76063: gbt_bytea_picksplit (btree_bytea.c:143) ... ==14292==by 0x6B76591: gbt_bit_union (btree_bit.c:173) ... I'm not seeing those valgrind warnings when I run it. Can you give more details on how to reproduce? I've added a bit more support for valgrind, but I don't see that being relevant in those. Are you compiling with USE_VALGRIND? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
Heikki Linnakangas hlinnakan...@vmware.com writes: On 05/16/2014 01:28 PM, Andres Freund wrote: Presumably it's because the type is a fixed width type with a length of 6. I guess it's allocating stuff sizeof(macaddr) but then passes the result around as a Datum which doesn't work well. I've not tried valgrinding to check, but for macaddr I suspect the blame can be laid at the feet of gbt_num_compress, which is creating a datum of actual size 2 * tinfo-size (ie, 12 bytes for macaddr). This is later stored into an index column of declared type gbtreekey16, so the tuple assembly code is expecting the datum to have size 16. AFAICS there is no direct connection between the sizes the C code knows about and the sizes of the datatypes declared as STORAGE options in btree_gist--1.0.sql. Probably a reasonable fix would be to add a field to gbtree_ninfo that specifies the index datum size, and then make gbt_num_compress palloc0 that size rather than 2 * tinfo-size. The complaints seem to be coming from all the varlen data types, too, not just macaddr: Dunno what's the problem for the varlena types, but that's a completely separate code path. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
Andres Freund and...@2ndquadrant.com writes: After Heikki has fixed the bit btree_gist bugs my valgrind run shows a bug in macaddr. AFAICT none of these traces represent live bugs, but I've pushed fixes for them into HEAD. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
On 05/16/2014 06:43 PM, Tom Lane wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: On 05/16/2014 01:28 PM, Andres Freund wrote: Presumably it's because the type is a fixed width type with a length of 6. I guess it's allocating stuff sizeof(macaddr) but then passes the result around as a Datum which doesn't work well. I've not tried valgrinding to check, but for macaddr I suspect the blame can be laid at the feet of gbt_num_compress, which is creating a datum of actual size 2 * tinfo-size (ie, 12 bytes for macaddr). This is later stored into an index column of declared type gbtreekey16, so the tuple assembly code is expecting the datum to have size 16. Ah, I see. AFAICS there is no direct connection between the sizes the C code knows about and the sizes of the datatypes declared as STORAGE options in btree_gist--1.0.sql. Probably a reasonable fix would be to add a field to gbtree_ninfo that specifies the index datum size, and then make gbt_num_compress palloc0 that size rather than 2 * tinfo-size. ISTM the correct fix would be to define a gbtreekey12 data type and use that. That's not back-patchable though, and introducing a whole new type to save a few bytes is hardly worth it. What you did makes sense. The complaints seem to be coming from all the varlen data types, too, not just macaddr: Dunno what's the problem for the varlena types, but that's a completely separate code path. It seems to be a similar issue to what I fixed in the bit type earlier. When the lower+upper keys are stored as one varlen Datum, the padding bytes between them are not zeroed. With these datatypes (i.e. not varbit) it doesn't lead to any real errors, however, because the padding bytes are never read. Valgrind is just complaining that we're storing uninitialized data on disk, even though it's never looked at. Nevertheless, seems best to fix that, if only to silence Valgrind. (And you just beat me to it. Thanks!) - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
Heikki Linnakangas hlinnakan...@vmware.com writes: On 05/16/2014 06:43 PM, Tom Lane wrote: Dunno what's the problem for the varlena types, but that's a completely separate code path. It seems to be a similar issue to what I fixed in the bit type earlier. When the lower+upper keys are stored as one varlen Datum, the padding bytes between them are not zeroed. With these datatypes (i.e. not varbit) it doesn't lead to any real errors, however, because the padding bytes are never read. Valgrind is just complaining that we're storing uninitialized data on disk, even though it's never looked at. Yeah, I came to the same conclusions. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
Heikki Linnakangas hlinnakan...@vmware.com writes: ISTM the correct fix would be to define a gbtreekey12 data type and use that. That's not back-patchable though, and introducing a whole new type to save a few bytes is hardly worth it. What you did makes sense. BTW, the *real* problem with all this stuff is that the gbtreekeyNN types are declared as having int alignment, even though some of the opclasses store double-aligned types in them. I imagine it's possible to provoke bus errors on machines that are picky about alignment. The first column of an index is safe enough because index tuples will be double-aligned anyway, but it seems like there's a hazard for lower-order columns. This is something we cannot fix compatibly :-( regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] btree_gist macaddr valgrind woes
Hi, On 2014-05-16 15:31:52 -0400, Tom Lane wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: On 05/16/2014 06:43 PM, Tom Lane wrote: Dunno what's the problem for the varlena types, but that's a completely separate code path. It seems to be a similar issue to what I fixed in the bit type earlier. When the lower+upper keys are stored as one varlen Datum, the padding bytes between them are not zeroed. With these datatypes (i.e. not varbit) it doesn't lead to any real errors, however, because the padding bytes are never read. Valgrind is just complaining that we're storing uninitialized data on disk, even though it's never looked at. Yeah, I came to the same conclusions. Thanks for fixing, looks good here. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
