Thread: [PATCH] 5 plperl patches
I have attached 5 patches (split up for ease of review) to plperl.c. 1. Two minor cleanups: - We don't need to call hv_exists+hv_fetch; we should just check the return value of hv_fetch. - newSVpv("undef",0) is the string "undef", not a real undef. 2. This should fix the bug Andrew Dunstan described in a recent -hackers post. It replaces three bogus "eval_pv(key, 0)" calls with newSVpv, and eliminates another redundant hv_exists+hv_fetch pair. 3. plperl_build_tuple_argument builds up a string of Perl code to create a hash representing the tuple. This patch creates the hash directly. 4. Another minor cleanup: replace a couple of av_store()s with av_push. 5. Analogous to #3 for plperl_trigger_build_args. This patch removes the static sv_add_tuple_value function, which does much the same as two other utility functions defined later, and merges the functionality into plperl_hash_from_tuple. I have tested the patches to the best of my limited ability, but I would appreciate it very much if someone else could review and test them too. (Thanks to Andrew and David Fetter for their help with some testing.) -- ams
Attachment
This also passes my modest testing, including the error case mentioned the other day, and appears to make sense. (I did notice a small case of non-postgres indentation, but I assume we'll have another run of pg_indent before the next beta?) cheers andrew Abhijit Menon-Sen wrote: >I have attached 5 patches (split up for ease of review) to plperl.c. > >1. Two minor cleanups: > > - We don't need to call hv_exists+hv_fetch; we should just check the > return value of hv_fetch. > - newSVpv("undef",0) is the string "undef", not a real undef. > >2. This should fix the bug Andrew Dunstan described in a recent -hackers > post. It replaces three bogus "eval_pv(key, 0)" calls with newSVpv, > and eliminates another redundant hv_exists+hv_fetch pair. > >3. plperl_build_tuple_argument builds up a string of Perl code to create > a hash representing the tuple. This patch creates the hash directly. > >4. Another minor cleanup: replace a couple of av_store()s with av_push. > >5. Analogous to #3 for plperl_trigger_build_args. This patch removes the > static sv_add_tuple_value function, which does much the same as two > other utility functions defined later, and merges the functionality > into plperl_hash_from_tuple. > >I have tested the patches to the best of my limited ability, but I would >appreciate it very much if someone else could review and test them too. > >(Thanks to Andrew and David Fetter for their help with some testing.) > > > >
Andrew Dunstan wrote: > > This also passes my modest testing, including the error case mentioned > the other day, and appears to make sense. (I did notice a small case of > non-postgres indentation, but I assume we'll have another run of > pg_indent before the next beta?) No, we typically do not run pgindent more than once during beta, though we could change that if people wanted. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Patch applied. Thanks. --------------------------------------------------------------------------- Abhijit Menon-Sen wrote: > I have attached 5 patches (split up for ease of review) to plperl.c. > > 1. Two minor cleanups: > > - We don't need to call hv_exists+hv_fetch; we should just check the > return value of hv_fetch. > - newSVpv("undef",0) is the string "undef", not a real undef. > > 2. This should fix the bug Andrew Dunstan described in a recent -hackers > post. It replaces three bogus "eval_pv(key, 0)" calls with newSVpv, > and eliminates another redundant hv_exists+hv_fetch pair. > > 3. plperl_build_tuple_argument builds up a string of Perl code to create > a hash representing the tuple. This patch creates the hash directly. > > 4. Another minor cleanup: replace a couple of av_store()s with av_push. > > 5. Analogous to #3 for plperl_trigger_build_args. This patch removes the > static sv_add_tuple_value function, which does much the same as two > other utility functions defined later, and merges the functionality > into plperl_hash_from_tuple. > > I have tested the patches to the best of my limited ability, but I would > appreciate it very much if someone else could review and test them too. > > (Thanks to Andrew and David Fetter for their help with some testing.) > > -- ams [ Attachment, skipping... ] [ Attachment, skipping... ] [ Attachment, skipping... ] [ Attachment, skipping... ] [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faqs/FAQ.html -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073