Обсуждение: [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

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