Re: plperl features

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Bruce Momjian-2
Sergej Sergeev wrote:

>
> >Sergej Sergeev <[hidden email]> writes:
> >  
> >
> >>>What happens if you feed other pseudotypes, like cstring or
> >>>language_handler?  Shouldn't that be disallowed or something?
> >>>      
> >>>
> >
> >  
> >
> >>Other pseudo-types are disallowed (no-change)
> >>    
> >>
> >
> >No, because you diked out the check at lines 1452ff, rather than
> >upgrading it to something correct.
> >
> >I find the "fn_retispseudo" and "arg_is_p" flags pretty bogus anyway
> >since they fail to indicate *which* pseudotype it is.  You might as
> >well just test for the specific type OID.
> >
> > regards, tom lane
> >  
> >
> New patch. I have added the check pseudo-type argumetns.
> Specific type is substituted in runtime, during function call.

I can't apply this patch because the code has changed too much.  Would
you regenerate a patch against current CVS?

Also, this indenting seems wrong:

> ! /* Disallow pseudotype argument, except ANYELEMENT or ANYARRAY */
>   if (typeStruct->typtype == 'p')
> + if (procStruct->proargtypes[i] == ANYARRAYOID ||
> + procStruct->proargtypes[i] == ANYELEMENTOID)
> + /* okay */
> + prodesc->arg_is_p[i] = true;
> + else
>   {
>   free(prodesc->proname);
>   free(prodesc);

Putting an 'if' after an 'if' is just too strange.  Please make a more
complete fix that has proper block indenting.

Also, I don't think the arg_is_p variable is really the proper fix for
this, but I am unsure what to recomment.  Others?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  [hidden email]               |  (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/faq
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Tom Lane-2
Bruce Momjian <[hidden email]> writes:
> Also, I don't think the arg_is_p variable is really the proper fix for
> this, but I am unsure what to recomment.  Others?

The thing I didn't like about that was that it assumes there is only
one pseudotype behavior that is or ever will be interesting for plperl.

I think it'd probably make more sense to store an array of the parameter
type OIDs and then check for ANYELEMENT or ANYARRAY as such in the
places where the patch uses arg_is_p.

                        regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Bruce Momjian-2

Sergej, are you going to repost this patch?

---------------------------------------------------------------------------

Tom Lane wrote:

> Bruce Momjian <[hidden email]> writes:
> > Also, I don't think the arg_is_p variable is really the proper fix for
> > this, but I am unsure what to recomment.  Others?
>
> The thing I didn't like about that was that it assumes there is only
> one pseudotype behavior that is or ever will be interesting for plperl.
>
> I think it'd probably make more sense to store an array of the parameter
> type OIDs and then check for ANYELEMENT or ANYARRAY as such in the
> places where the patch uses arg_is_p.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  [hidden email]               |  (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 1: subscribe and unsubscribe commands go to [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Andrew Dunstan

This was the patch that I took the array processing piece from and
attempted to fix, since it was badly broken. However, I'm not happy
about any of the ways of doing it, and suspect I won't get it done for
8.1. I think we need that piece done before we look at ANYELEMENT/ANYARRAY.

cheers

andrew

Bruce Momjian wrote:

>Sergej, are you going to repost this patch?
>
>---------------------------------------------------------------------------
>
>Tom Lane wrote:
>  
>
>>Bruce Momjian <[hidden email]> writes:
>>    
>>
>>>Also, I don't think the arg_is_p variable is really the proper fix for
>>>this, but I am unsure what to recomment.  Others?
>>>      
>>>
>>The thing I didn't like about that was that it assumes there is only
>>one pseudotype behavior that is or ever will be interesting for plperl.
>>
>>I think it'd probably make more sense to store an array of the parameter
>>type OIDs and then check for ANYELEMENT or ANYARRAY as such in the
>>places where the patch uses arg_is_p.
>>
>> regards, tom lane
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 7: don't forget to increase your free space map settings
>>
>>    
>>
>
>  
>

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

               http://archives.postgresql.org
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Bruce Momjian-2

Do we need a TODO item?

---------------------------------------------------------------------------

Andrew Dunstan wrote:

>
> This was the patch that I took the array processing piece from and
> attempted to fix, since it was badly broken. However, I'm not happy
> about any of the ways of doing it, and suspect I won't get it done for
> 8.1. I think we need that piece done before we look at ANYELEMENT/ANYARRAY.
>
> cheers
>
> andrew
>
> Bruce Momjian wrote:
>
> >Sergej, are you going to repost this patch?
> >
> >---------------------------------------------------------------------------
> >
> >Tom Lane wrote:
> >  
> >
> >>Bruce Momjian <[hidden email]> writes:
> >>    
> >>
> >>>Also, I don't think the arg_is_p variable is really the proper fix for
> >>>this, but I am unsure what to recomment.  Others?
> >>>      
> >>>
> >>The thing I didn't like about that was that it assumes there is only
> >>one pseudotype behavior that is or ever will be interesting for plperl.
> >>
> >>I think it'd probably make more sense to store an array of the parameter
> >>type OIDs and then check for ANYELEMENT or ANYARRAY as such in the
> >>places where the patch uses arg_is_p.
> >>
> >> regards, tom lane
> >>
> >>---------------------------(end of broadcast)---------------------------
> >>TIP 7: don't forget to increase your free space map settings
> >>
> >>    
> >>
> >
> >  
> >
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  [hidden email]               |  (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 3: if posting/reading through Usenet, please send an appropriate
       subscribe-nomail command to [hidden email] so that your
       message can get through to the mailing list cleanly
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Andrew Dunstan


Bruce Momjian wrote:

>Do we need a TODO item?
>  
>

Sure, Maybe two:

. pass arrays natively instead of as text between plperl and postgres
. add support for polymorphic arguments and return types to plperl

cheers

andrew


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

               http://www.postgresql.org/docs/faq
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Bruce Momjian-2
Andrew Dunstan wrote:

>
>
> Bruce Momjian wrote:
>
> >Do we need a TODO item?
> >  
> >
>
> Sure, Maybe two:
>
> . pass arrays natively instead of as text between plperl and postgres
> . add support for polymorphic arguments and return types to plperl

Added to TODO:

        o Pass arrays natively instead of as text between plperl and postgres
        o Add support for polymorphic arguments and return types to plperl

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  [hidden email]               |  (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 4: Don't 'kill -9' the postmaster
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Sergej Sergeev
In reply to this post by Bruce Momjian-2
Bruce Momjian wrote:

>Sergej, are you going to repost this patch?
>  
>
Sorry for delaying.
Yes, I working on it, but I wait for decision about Andrew and Abhijit
patches.

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster
Reply | Threaded
Open this post in threaded view
|

Re: plperl features

Andrew Dunstan
Sergej Sergeev said:
> Bruce Momjian wrote:
>
>>Sergej, are you going to repost this patch?
>>
>>
> Sorry for delaying.
> Yes, I working on it, but I wait for decision about Andrew and Abhijit
> patches.
>

This is the polymorphic types plus perl to pg array patch, right?

I am not working on this right now (or anything else for 8.1) - the original
plperl array to pg array implementation was broken and I was not happy about
the fix I first came up with, and ran out of time to work on an acceptable
solution.

Neither is Abhijit, to the best of my knowledge - he has today submitted a
patch for SPI fetching via cursor, which should not affect this stuff.

We recently put the two items on the TODO list, as I understood from Joshua
that you guys weren't working on plperl at all in the 8.1 feature freeze
time frame.


cheers

andrew





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

               http://www.postgresql.org/docs/faq