Обсуждение: btree_gist macaddr valgrind woes

Поиск
Список
Период
Сортировка

btree_gist macaddr valgrind woes

От
Andres Freund
Дата:
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 (btree_utils_var.c:82)
==14250==    by 0x6B6D4F1: gbt_var_bin_union (btree_utils_var.c:256)
==14250==    by 0x6B6DF47: gbt_var_picksplit (btree_utils_var.c:502)
==14250==    by 0x6B75C45: gbt_text_picksplit (btree_text.c:215)
==14250==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14250==    by 0x497C38: gistUserPicksplit (gistsplit.c:433)
==14250==    by 0x4989C8: gistSplitByKey (gistsplit.c:697)
==14250==    by 0x48C423: gistSplit (gist.c:1270)
==14250==    by 0x488ED4: gistplacetopage (gist.c:242)
==14250==    by 0x48C025: gistinserttuples (gist.c:1134)
==14250==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14250== 
==14259== Uninitialised byte(s) found during client check request
==14259==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14259==    by 0x48CC95: gistfillbuffer (gistutil.c:42)
==14259==    by 0x489CBC: gistplacetopage (gist.c:451)
==14259==    by 0x48C025: gistinserttuples (gist.c:1134)
==14259==    by 0x48C31C: gistfinishsplit (gist.c:1240)
==14259==    by 0x48C0F5: gistinserttuples (gist.c:1159)
==14259==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14259==    by 0x48AC56: gistdoinsert (gist.c:750)
==14259==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14259==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14259==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14259==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14259==  Address 0x69928c3 is 19 bytes inside a block of size 40 client-defined
==14259==    at 0x8FB83A: palloc0 (mcxt.c:698)
==14259==    by 0x462954: index_form_tuple (indextuple.c:129)
==14259==    by 0x48ED05: gistFormTuple (gistutil.c:611)
==14259==    by 0x48C618: gistSplit (gist.c:1292)
==14259==    by 0x488ED4: gistplacetopage (gist.c:242)
==14259==    by 0x48C025: gistinserttuples (gist.c:1134)
==14259==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14259==    by 0x48AC56: gistdoinsert (gist.c:750)
==14259==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14259==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14259==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14259==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14259==  Uninitialised value was created by a heap allocation
==14259==    at 0x8FB6D2: palloc (mcxt.c:678)
==14259==    by 0x6B6CFCE: gbt_var_key_copy (btree_utils_var.c:82)
==14259==    by 0x6B6D4F1: gbt_var_bin_union (btree_utils_var.c:256)
==14259==    by 0x6B6DF47: gbt_var_picksplit (btree_utils_var.c:502)
==14259==    by 0x6B75C45: gbt_text_picksplit (btree_text.c:215)
==14259==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14259==    by 0x497C38: gistUserPicksplit (gistsplit.c:433)
==14259==    by 0x4989C8: gistSplitByKey (gistsplit.c:697)
==14259==    by 0x48C423: gistSplit (gist.c:1270)
==14259==    by 0x488ED4: gistplacetopage (gist.c:242)
==14259==    by 0x48C025: gistinserttuples (gist.c:1134)
==14259==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14259== 
==14269== Uninitialised byte(s) found during client check request
==14269==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14269==    by 0x48CC95: gistfillbuffer (gistutil.c:42)
==14269==    by 0x489CBC: gistplacetopage (gist.c:451)
==14269==    by 0x48C025: gistinserttuples (gist.c:1134)
==14269==    by 0x48C31C: gistfinishsplit (gist.c:1240)
==14269==    by 0x48C0F5: gistinserttuples (gist.c:1159)
==14269==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14269==    by 0x48AC56: gistdoinsert (gist.c:750)
==14269==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14269==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14269==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14269==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14269==  Address 0x6866163 is 19 bytes inside a block of size 40 client-defined
==14269==    at 0x8FB83A: palloc0 (mcxt.c:698)
==14269==    by 0x462954: index_form_tuple (indextuple.c:129)
==14269==    by 0x48ED05: gistFormTuple (gistutil.c:611)
==14269==    by 0x48C618: gistSplit (gist.c:1292)
==14269==    by 0x488ED4: gistplacetopage (gist.c:242)
==14269==    by 0x48C025: gistinserttuples (gist.c:1134)
==14269==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14269==    by 0x48AC56: gistdoinsert (gist.c:750)
==14269==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14269==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14269==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14269==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14269==  Uninitialised value was created by a heap allocation
==14269==    at 0x8FB6D2: palloc (mcxt.c:678)
==14269==    by 0x6B6CFCE: gbt_var_key_copy (btree_utils_var.c:82)
==14269==    by 0x6B6D4F1: gbt_var_bin_union (btree_utils_var.c:256)
==14269==    by 0x6B6DF47: gbt_var_picksplit (btree_utils_var.c:502)
==14269==    by 0x6B75C45: gbt_text_picksplit (btree_text.c:215)
==14269==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14269==    by 0x497C38: gistUserPicksplit (gistsplit.c:433)
==14269==    by 0x4989C8: gistSplitByKey (gistsplit.c:697)
==14269==    by 0x48C423: gistSplit (gist.c:1270)
==14269==    by 0x488ED4: gistplacetopage (gist.c:242)
==14269==    by 0x48C025: gistinserttuples (gist.c:1134)
==14269==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14269== 
==14280== Uninitialised byte(s) found during client check request
==14280==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14280==    by 0x48CC95: gistfillbuffer (gistutil.c:42)
==14280==    by 0x489CBC: gistplacetopage (gist.c:451)
==14280==    by 0x48C025: gistinserttuples (gist.c:1134)
==14280==    by 0x48C31C: gistfinishsplit (gist.c:1240)
==14280==    by 0x48C0F5: gistinserttuples (gist.c:1159)
==14280==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14280==    by 0x48AC56: gistdoinsert (gist.c:750)
==14280==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14280==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14280==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14280==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14280==  Address 0x695d36f is 15 bytes inside a block of size 24 client-defined
==14280==    at 0x8FB83A: palloc0 (mcxt.c:698)
==14280==    by 0x462954: index_form_tuple (indextuple.c:129)
==14280==    by 0x48ED05: gistFormTuple (gistutil.c:611)
==14280==    by 0x48C618: gistSplit (gist.c:1292)
==14280==    by 0x488ED4: gistplacetopage (gist.c:242)
==14280==    by 0x48C025: gistinserttuples (gist.c:1134)
==14280==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14280==    by 0x48AC56: gistdoinsert (gist.c:750)
==14280==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14280==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14280==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14280==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14280==  Uninitialised value was created by a heap allocation
==14280==    at 0x8FB6D2: palloc (mcxt.c:678)
==14280==    by 0x6B6D344: gbt_var_node_truncate (btree_utils_var.c:204)
==14280==    by 0x6B6E04B: gbt_var_picksplit (btree_utils_var.c:520)
==14280==    by 0x6B76063: gbt_bytea_picksplit (btree_bytea.c:143)
==14280==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14280==    by 0x497C38: gistUserPicksplit (gistsplit.c:433)
==14280==    by 0x4989C8: gistSplitByKey (gistsplit.c:697)
==14280==    by 0x48C423: gistSplit (gist.c:1270)
==14280==    by 0x488ED4: gistplacetopage (gist.c:242)
==14280==    by 0x48C025: gistinserttuples (gist.c:1134)
==14280==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14280==    by 0x48AC56: gistdoinsert (gist.c:750)
==14280== 
==14292== Uninitialised byte(s) found during client check request
==14292==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14292==    by 0x48966C: gistplacetopage (gist.c:357)
==14292==    by 0x48C025: gistinserttuples (gist.c:1134)
==14292==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14292==    by 0x48AC56: gistdoinsert (gist.c:750)
==14292==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14292==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14292==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14292==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14292==    by 0x52F816: index_build (index.c:1962)
==14292==    by 0x52E79D: index_create (index.c:1083)
==14292==    by 0x5E9253: DefineIndex (indexcmds.c:600)
==14292==  Address 0x67e408f is 15 bytes inside a block of size 40 client-defined
==14292==    at 0x8FB6D2: palloc (mcxt.c:678)
==14292==    by 0x48D0D3: gistfillitupvec (gistutil.c:133)
==14292==    by 0x489569: gistplacetopage (gist.c:325)
==14292==    by 0x48C025: gistinserttuples (gist.c:1134)
==14292==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14292==    by 0x48AC56: gistdoinsert (gist.c:750)
==14292==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14292==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14292==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14292==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14292==    by 0x52F816: index_build (index.c:1962)
==14292==    by 0x52E79D: index_create (index.c:1083)
==14292==  Uninitialised value was created by a heap allocation
==14292==    at 0x8FB6D2: palloc (mcxt.c:678)
==14292==    by 0x6B6D344: gbt_var_node_truncate (btree_utils_var.c:204)
==14292==    by 0x6B6D6DA: gbt_var_union (btree_utils_var.c:325)
==14292==    by 0x6B76591: gbt_bit_union (btree_bit.c:173)
==14292==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14292==    by 0x48D7C8: gistMakeUnionItVec (gistutil.c:198)
==14292==    by 0x496E8E: gistunionsubkeyvec (gistsplit.c:64)
==14292==    by 0x496F05: gistunionsubkey (gistsplit.c:91)
==14292==    by 0x498981: gistSplitByKey (gistsplit.c:689)
==14292==    by 0x48C423: gistSplit (gist.c:1270)
==14292==    by 0x488ED4: gistplacetopage (gist.c:242)
==14292==    by 0x48C025: gistinserttuples (gist.c:1134)
==14292== 
==14292== Uninitialised byte(s) found during client check request
==14292==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14292==    by 0x48CC95: gistfillbuffer (gistutil.c:42)
==14292==    by 0x489CBC: gistplacetopage (gist.c:451)
==14292==    by 0x48C025: gistinserttuples (gist.c:1134)
==14292==    by 0x48C31C: gistfinishsplit (gist.c:1240)
==14292==    by 0x48C0F5: gistinserttuples (gist.c:1159)
==14292==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14292==    by 0x48AC56: gistdoinsert (gist.c:750)
==14292==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14292==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14292==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14292==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14292==  Address 0x6801e57 is 15 bytes inside a block of size 24 client-defined
==14292==    at 0x8FB83A: palloc0 (mcxt.c:698)
==14292==    by 0x462954: index_form_tuple (indextuple.c:129)
==14292==    by 0x48ED05: gistFormTuple (gistutil.c:611)
==14292==    by 0x48C755: gistSplit (gist.c:1314)
==14292==    by 0x488ED4: gistplacetopage (gist.c:242)
==14292==    by 0x48C025: gistinserttuples (gist.c:1134)
==14292==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14292==    by 0x48AC56: gistdoinsert (gist.c:750)
==14292==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14292==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14292==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14292==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14292==  Uninitialised value was created by a heap allocation
==14292==    at 0x8FB6D2: palloc (mcxt.c:678)
==14292==    by 0x6B6D344: gbt_var_node_truncate (btree_utils_var.c:204)
==14292==    by 0x6B6E028: gbt_var_picksplit (btree_utils_var.c:519)
==14292==    by 0x6B765D5: gbt_bit_picksplit (btree_bit.c:184)
==14292==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14292==    by 0x497C38: gistUserPicksplit (gistsplit.c:433)
==14292==    by 0x4989C8: gistSplitByKey (gistsplit.c:697)
==14292==    by 0x48C423: gistSplit (gist.c:1270)
==14292==    by 0x488ED4: gistplacetopage (gist.c:242)
==14292==    by 0x48C025: gistinserttuples (gist.c:1134)
==14292==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14292==    by 0x48AC56: gistdoinsert (gist.c:750)
==14292== 
==14299== Uninitialised byte(s) found during client check request
==14299==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14299==    by 0x48966C: gistplacetopage (gist.c:357)
==14299==    by 0x48C025: gistinserttuples (gist.c:1134)
==14299==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14299==    by 0x48AC56: gistdoinsert (gist.c:750)
==14299==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14299==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14299==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14299==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14299==    by 0x52F816: index_build (index.c:1962)
==14299==    by 0x52E79D: index_create (index.c:1083)
==14299==    by 0x5E9253: DefineIndex (indexcmds.c:600)
==14299==  Address 0x6877827 is 15 bytes inside a block of size 40 client-defined
==14299==    at 0x8FB6D2: palloc (mcxt.c:678)
==14299==    by 0x48D0D3: gistfillitupvec (gistutil.c:133)
==14299==    by 0x489569: gistplacetopage (gist.c:325)
==14299==    by 0x48C025: gistinserttuples (gist.c:1134)
==14299==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14299==    by 0x48AC56: gistdoinsert (gist.c:750)
==14299==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14299==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14299==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14299==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14299==    by 0x52F816: index_build (index.c:1962)
==14299==    by 0x52E79D: index_create (index.c:1083)
==14299==  Uninitialised value was created by a heap allocation
==14299==    at 0x8FB6D2: palloc (mcxt.c:678)
==14299==    by 0x6B6D344: gbt_var_node_truncate (btree_utils_var.c:204)
==14299==    by 0x6B6D6DA: gbt_var_union (btree_utils_var.c:325)
==14299==    by 0x6B76591: gbt_bit_union (btree_bit.c:173)
==14299==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14299==    by 0x48D7C8: gistMakeUnionItVec (gistutil.c:198)
==14299==    by 0x496E8E: gistunionsubkeyvec (gistsplit.c:64)
==14299==    by 0x496F05: gistunionsubkey (gistsplit.c:91)
==14299==    by 0x498981: gistSplitByKey (gistsplit.c:689)
==14299==    by 0x48C423: gistSplit (gist.c:1270)
==14299==    by 0x488ED4: gistplacetopage (gist.c:242)
==14299==    by 0x48C025: gistinserttuples (gist.c:1134)
==14299== 
==14299== Uninitialised byte(s) found during client check request
==14299==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14299==    by 0x48CC95: gistfillbuffer (gistutil.c:42)
==14299==    by 0x489CBC: gistplacetopage (gist.c:451)
==14299==    by 0x48C025: gistinserttuples (gist.c:1134)
==14299==    by 0x48C31C: gistfinishsplit (gist.c:1240)
==14299==    by 0x48C0F5: gistinserttuples (gist.c:1159)
==14299==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14299==    by 0x48AC56: gistdoinsert (gist.c:750)
==14299==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14299==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14299==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14299==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14299==  Address 0x6891bf7 is 15 bytes inside a block of size 24 client-defined
==14299==    at 0x8FB83A: palloc0 (mcxt.c:698)
==14299==    by 0x462954: index_form_tuple (indextuple.c:129)
==14299==    by 0x48ED05: gistFormTuple (gistutil.c:611)
==14299==    by 0x48C755: gistSplit (gist.c:1314)
==14299==    by 0x488ED4: gistplacetopage (gist.c:242)
==14299==    by 0x48C025: gistinserttuples (gist.c:1134)
==14299==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14299==    by 0x48AC56: gistdoinsert (gist.c:750)
==14299==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14299==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14299==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14299==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14299==  Uninitialised value was created by a heap allocation
==14299==    at 0x8FB6D2: palloc (mcxt.c:678)
==14299==    by 0x6B6D344: gbt_var_node_truncate (btree_utils_var.c:204)
==14299==    by 0x6B6E028: gbt_var_picksplit (btree_utils_var.c:519)
==14299==    by 0x6B765D5: gbt_bit_picksplit (btree_bit.c:184)
==14299==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14299==    by 0x497C38: gistUserPicksplit (gistsplit.c:433)
==14299==    by 0x4989C8: gistSplitByKey (gistsplit.c:697)
==14299==    by 0x48C423: gistSplit (gist.c:1270)
==14299==    by 0x488ED4: gistplacetopage (gist.c:242)
==14299==    by 0x48C025: gistinserttuples (gist.c:1134)
==14299==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14299==    by 0x48AC56: gistdoinsert (gist.c:750)
==14299== 
==14310== Uninitialised byte(s) found during client check request
==14310==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14310==    by 0x48CC95: gistfillbuffer (gistutil.c:42)
==14310==    by 0x489CBC: gistplacetopage (gist.c:451)
==14310==    by 0x48C025: gistinserttuples (gist.c:1134)
==14310==    by 0x48C31C: gistfinishsplit (gist.c:1240)
==14310==    by 0x48C0F5: gistinserttuples (gist.c:1159)
==14310==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14310==    by 0x48AC56: gistdoinsert (gist.c:750)
==14310==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14310==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14310==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14310==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14310==  Address 0x6ab666f is 23 bytes inside a block of size 48 client-defined
==14310==    at 0x8FB83A: palloc0 (mcxt.c:698)
==14310==    by 0x462954: index_form_tuple (indextuple.c:129)
==14310==    by 0x48ED05: gistFormTuple (gistutil.c:611)
==14310==    by 0x48C618: gistSplit (gist.c:1292)
==14310==    by 0x488ED4: gistplacetopage (gist.c:242)
==14310==    by 0x48C025: gistinserttuples (gist.c:1134)
==14310==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14310==    by 0x48AC56: gistdoinsert (gist.c:750)
==14310==    by 0x49985B: gistBuildCallback (gistbuild.c:490)
==14310==    by 0x530412: IndexBuildHeapScan (index.c:2460)
==14310==    by 0x4991D4: gistbuild (gistbuild.c:209)
==14310==    by 0x8D813B: OidFunctionCall3Coll (fmgr.c:1650)
==14310==  Uninitialised value was created by a heap allocation
==14310==    at 0x8FB6D2: palloc (mcxt.c:678)
==14310==    by 0x6B6CFCE: gbt_var_key_copy (btree_utils_var.c:82)
==14310==    by 0x6B6D4F1: gbt_var_bin_union (btree_utils_var.c:256)
==14310==    by 0x6B6DF47: gbt_var_picksplit (btree_utils_var.c:502)
==14310==    by 0x6B76D27: gbt_numeric_picksplit (btree_numeric.c:235)
==14310==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14310==    by 0x497C38: gistUserPicksplit (gistsplit.c:433)
==14310==    by 0x4989C8: gistSplitByKey (gistsplit.c:697)
==14310==    by 0x48C423: gistSplit (gist.c:1270)
==14310==    by 0x488ED4: gistplacetopage (gist.c:242)
==14310==    by 0x48C025: gistinserttuples (gist.c:1134)
==14310==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14310== 
==14325== Uninitialised byte(s) found during client check request
==14325==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14325==    by 0x48966C: gistplacetopage (gist.c:357)
==14325==    by 0x48C025: gistinserttuples (gist.c:1134)
==14325==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14325==    by 0x48AC56: gistdoinsert (gist.c:750)
==14325==    by 0x488B81: gistinsert (gist.c:132)
==14325==    by 0x8D7964: FunctionCall6Coll (fmgr.c:1437)
==14325==    by 0x4C3EB8: index_insert (indexam.c:226)
==14325==    by 0x647F8B: ExecInsertIndexTuples (execUtils.c:1103)
==14325==    by 0x65BC55: ExecInsert (nodeModifyTable.c:274)
==14325==    by 0x65CE33: ExecModifyTable (nodeModifyTable.c:1025)
==14325==    by 0x639EC2: ExecProcNode (execProcnode.c:377)
==14325==  Address 0x6df3d6b is 35 bytes inside a block of size 96 client-defined
==14325==    at 0x8FB6D2: palloc (mcxt.c:678)
==14325==    by 0x48D0D3: gistfillitupvec (gistutil.c:133)
==14325==    by 0x489569: gistplacetopage (gist.c:325)
==14325==    by 0x48C025: gistinserttuples (gist.c:1134)
==14325==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14325==    by 0x48AC56: gistdoinsert (gist.c:750)
==14325==    by 0x488B81: gistinsert (gist.c:132)
==14325==    by 0x8D7964: FunctionCall6Coll (fmgr.c:1437)
==14325==    by 0x4C3EB8: index_insert (indexam.c:226)
==14325==    by 0x647F8B: ExecInsertIndexTuples (execUtils.c:1103)
==14325==    by 0x65BC55: ExecInsert (nodeModifyTable.c:274)
==14325==    by 0x65CE33: ExecModifyTable (nodeModifyTable.c:1025)
==14325==  Uninitialised value was created by a heap allocation
==14325==    at 0x8FB6D2: palloc (mcxt.c:678)
==14325==    by 0x6B6CFCE: gbt_var_key_copy (btree_utils_var.c:82)
==14325==    by 0x6B6D64F: gbt_var_union (btree_utils_var.c:309)
==14325==    by 0x6B769AF: gbt_numeric_union (btree_numeric.c:137)
==14325==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14325==    by 0x48D7C8: gistMakeUnionItVec (gistutil.c:198)
==14325==    by 0x496E8E: gistunionsubkeyvec (gistsplit.c:64)
==14325==    by 0x496F05: gistunionsubkey (gistsplit.c:91)
==14325==    by 0x498DB5: gistSplitByKey (gistsplit.c:777)
==14325==    by 0x48C423: gistSplit (gist.c:1270)
==14325==    by 0x488ED4: gistplacetopage (gist.c:242)
==14325==    by 0x48C025: gistinserttuples (gist.c:1134)
==14325== 
==14325== Uninitialised byte(s) found during client check request
==14325==    at 0x794E9F: PageAddItem (bufpage.c:314)
==14325==    by 0x48CC95: gistfillbuffer (gistutil.c:42)
==14325==    by 0x489CBC: gistplacetopage (gist.c:451)
==14325==    by 0x48C025: gistinserttuples (gist.c:1134)
==14325==    by 0x48C31C: gistfinishsplit (gist.c:1240)
==14325==    by 0x48C0F5: gistinserttuples (gist.c:1159)
==14325==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14325==    by 0x48AC56: gistdoinsert (gist.c:750)
==14325==    by 0x488B81: gistinsert (gist.c:132)
==14325==    by 0x8D7964: FunctionCall6Coll (fmgr.c:1437)
==14325==    by 0x4C3EB8: index_insert (indexam.c:226)
==14325==    by 0x647F8B: ExecInsertIndexTuples (execUtils.c:1103)
==14325==  Address 0x7134203 is 35 bytes inside a block of size 48 client-defined
==14325==    at 0x8FB83A: palloc0 (mcxt.c:698)
==14325==    by 0x462954: index_form_tuple (indextuple.c:129)
==14325==    by 0x48ED05: gistFormTuple (gistutil.c:611)
==14325==    by 0x48C755: gistSplit (gist.c:1314)
==14325==    by 0x488ED4: gistplacetopage (gist.c:242)
==14325==    by 0x48C025: gistinserttuples (gist.c:1134)
==14325==    by 0x48BFAA: gistinserttuple (gist.c:1093)
==14325==    by 0x48AC56: gistdoinsert (gist.c:750)
==14325==    by 0x488B81: gistinsert (gist.c:132)
==14325==    by 0x8D7964: FunctionCall6Coll (fmgr.c:1437)
==14325==    by 0x4C3EB8: index_insert (indexam.c:226)
==14325==    by 0x647F8B: ExecInsertIndexTuples (execUtils.c:1103)
==14325==  Uninitialised value was created by a heap allocation
==14325==    at 0x8FB6D2: palloc (mcxt.c:678)
==14325==    by 0x6B6CFCE: gbt_var_key_copy (btree_utils_var.c:82)
==14325==    by 0x6B6D64F: gbt_var_union (btree_utils_var.c:309)
==14325==    by 0x6B769AF: gbt_numeric_union (btree_numeric.c:137)
==14325==    by 0x8D74D0: FunctionCall2Coll (fmgr.c:1324)
==14325==    by 0x48D7C8: gistMakeUnionItVec (gistutil.c:198)
==14325==    by 0x496E8E: gistunionsubkeyvec (gistsplit.c:64)
==14325==    by 0x496F05: gistunionsubkey (gistsplit.c:91)
==14325==    by 0x498DB5: gistSplitByKey (gistsplit.c:777)
==14325==    by 0x48C423: gistSplit (gist.c:1270)
==14325==    by 0x488ED4: gistplacetopage (gist.c:242)
==14325==    by 0x48C025: gistinserttuples (gist.c:1134)
==14325== 

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



Re: btree_gist macaddr valgrind woes

От
Heikki Linnakangas
Дата:
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



Re: btree_gist macaddr valgrind woes

От
Andres Freund
Дата:
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



Re: btree_gist macaddr valgrind woes

От
Tom Lane
Дата:
Heikki Linnakangas <hlinnakangas@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



Re: btree_gist macaddr valgrind woes

От
Tom Lane
Дата:
Andres Freund <andres@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



Re: btree_gist macaddr valgrind woes

От
Heikki Linnakangas
Дата:
On 05/16/2014 06:43 PM, Tom Lane wrote:
> Heikki Linnakangas <hlinnakangas@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



Re: btree_gist macaddr valgrind woes

От
Tom Lane
Дата:
Heikki Linnakangas <hlinnakangas@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



Re: btree_gist macaddr valgrind woes

От
Tom Lane
Дата:
Heikki Linnakangas <hlinnakangas@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



Re: btree_gist macaddr valgrind woes

От
Andres Freund
Дата:
Hi,

On 2014-05-16 15:31:52 -0400, Tom Lane wrote:
> Heikki Linnakangas <hlinnakangas@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



Re: btree_gist macaddr valgrind woes

От
Tom Lane
Дата:
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.16rows=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



Re: btree_gist macaddr valgrind woes

От
Heikki Linnakangas
Дата:
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



Re: btree_gist macaddr valgrind woes

От
Andres Freund
Дата:
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



Re: btree_gist macaddr valgrind woes

От
Tom Lane
Дата:
Andres Freund <andres@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



Re: btree_gist macaddr valgrind woes

От
Heikki Linnakangas
Дата:
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



Re: btree_gist macaddr valgrind woes

От
Alvaro Herrera
Дата:
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 Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services



Re: btree_gist macaddr valgrind woes

От
Bruce Momjian
Дата:
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  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +