Re: [PATCHES] [PATCH] 5 plperl patches

2004-10-15 Thread Bruce Momjian 

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
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faqs/FAQ.html

Re: [PATCHES] [PATCH] 5 plperl patches

2004-10-11 Thread Bruce Momjian 
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
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings

Re: [PATCHES] [PATCH] 5 plperl patches

2004-10-02 Thread Andrew Dunstan 
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.)
 


---(end of broadcast)---
TIP 8: explain analyze is your friend

[PATCHES] [PATCH] 5 plperl patches

2004-10-01 Thread Abhijit Menon-Sen 
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
--- plperl.c~   2004-10-02 04:12:05.189765562 +0530
+++ plperl.c2004-10-02 04:12:28.017002164 +0530
@@ -1270,6 +1270,7 @@ compile_plperl_function(Oid fn_oid, bool
int proname_len;
plperl_proc_desc *prodesc = NULL;
int i;
+   SV  **svp;
 
/* We'll need the pg_proc tuple in any case... */
procTup = SearchSysCache(PROCOID,
@@ -1292,12 +1293,12 @@ compile_plperl_function(Oid fn_oid, bool
/
 * Lookup the internal proc name in the hashtable
 /
-   if (hv_exists(plperl_proc_hash, internal_proname, proname_len))
+   svp = hv_fetch(plperl_proc_hash, internal_proname, proname_len, FALSE);
+   if (svp)
{
booluptodate;
 
-   prodesc = (plperl_proc_desc *) SvIV(*hv_fetch(plperl_proc_hash,
- 
internal_proname, proname_len, 0));
+   prodesc = (plperl_proc_desc *) SvIV(*svp);
 
/
 * If it's present, must check whether it's still up to date.
@@ -1625,7 +1626,7 @@ plperl_hash_from_tuple(HeapTuple tuple, 
if (attdata)
hv_store(array, attname, strlen(attname), newSVpv(attdata, 0), 
0);
else
-   hv_store(array, attname, strlen(attname), newSVpv("undef", 0), 
0);
+   hv_store(array, attname, strlen(attname), newSV(0), 0);
}
return array;
 }
--- plperl.c~   2004-10-02 04:14:43.768721344 +0530
+++ plperl.c2004-10-02 04:14:53.325733327 +0530
@@ -451,7 +451,7 @@ plperl_get_keys(HV *hv)
hv_iterinit(hv);
while ((val = hv_iternextsv(hv, (char **) &key, &klen)))
{
-   av_store(ret, key_count, eval_pv(key, TRUE));
+   av_store(ret, key_count, newSVpv(key, 0));
key_count++;
}
hv_iterinit(hv);
@@ -484,11 +484,8 @@ plperl_get_key(AV *keys, int index)
 static char *
 plperl_get_elem(HV *hash, char *key)
 {
-   SV**svp;
-
-   if (hv_exists_ent(hash, eval_pv(key, TRUE), FALSE))
-   svp = hv_fetch(hash, key, strlen(key), FALSE);
-   else
+   SV **svp = hv_fetch(hash, key, strlen(key), FALSE);
+   if (!svp)
{
elog(ERROR, "plperl: key '%s' not found", key);
return NULL;
@@ -998,7 +995,8 @@ plperl_func_handler(PG_FUNCTION_ARGS)
g_attr_num = tupdesc->natts;
 
for (i = 0; i < tupdesc->natts; i++)
-   av_store(g_column_keys, i + 1, 
eval_pv(SPI_fname(tupdesc, i + 1), TRUE));
+   av_store(g_column_keys, i + 1,
+newSVpv(SPI_fname(tupdesc, i+1), 0));
 
slot = TupleDescGetSlot(tupdesc);
funcctx->slot = slot;
--- plperl.c~   2004-10-02 04:15:16.737864847 +0530
+++ plperl.c2004-10-02 04:16:36.108850361 +0530
@@ -1519,7 +1519,7 @@ static SV  *
 plperl_build_tuple_argument(HeapTuple tuple, TupleDesc tupdesc)
 {
int i;
-   SV *output;
+   HV *hv;
Datum   attr;
boolisnull;
char   *attname;
@@ -1527,31 +1527,22 @@ plperl_build_tuple_argument(HeapTuple tu
HeapTuple   typeTup;
Oid typoutput;
Oid typioparam;
+   int namelen;
 
-   output = sv_2mortal(newSVpv