Prev: [HACKERS] "0 0" return tag
Next: [HACKERS] LISTEN/NOTIFY versus encoding conversion
From: "David E. Wheeler" on 13 Feb 2010 17:25
On Feb 12, 2010, at 3:10 PM, Tim Bunce wrote:

> I've appended the POD documentation and attached the (rough but working)
> test script.
>
> I plan to release the module to CPAN in the next week or so.
>
> I'd greatly appreciate any feedback.

I like the idea overall, and anything that can simplify the interface is more than welcome. However:

* I'd rather not have to specify a signature for a non-polymorphic function.
* I'd like to be able to use Perl code to call the functions as discussed
previously, something like:

my $count_sql = SP->tl_activity_stats_sql(
[ statistic => $stat, person_id => $pid ],
$debug
);

For a Polymorphic function, perhaps it could be something like:

my $count = SP->call(
tl_activity_stats_sql => [qw(text[] int)],
[ statistic => $stat, person_id => $pid ],
$debug
);

The advantage here is that I'm not writing functions inside strings, and only provide the signature when I need to disambiguate between polymorphic variants.

Anyway, That's just interface arguing. The overall idea is sound and very much appreciated.

Best,

David


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: "David E. Wheeler" on 15 Feb 2010 14:52
On Feb 15, 2010, at 2:51 AM, Tim Bunce wrote:

> The signature doesn't just qualify the selection of the function,
> it also ensures appropriate interpretation of the arguments.
>
> I could allow call('foo', @args), which could be written call(foo => @args),
> but what should that mean in terms of the underlying behaviour?
>
> I think there are three practical options:
> a) treat it the same as call('foo(unknown...)', @args)

I believe that's basically what psql does. It's certainly what DBD::Pg does.

> b) treat it the same as call('foo(text...)', @args)

Probably not a great idea.

> c) instead of using a cached prepared query, build an SQL statement
> for every execution, which would naturally have to quote all values:
> my $args = join ",", map { ::quote_nullable($_) } @_;
> return ::spi_exec_query("select * from $spname($args)");
>
> I suspect there are subtle issues (that I'm unfamilar with) lurking here.
> I'd appreciate someone with greater understanding spelling out the issues
> and trade-offs in those options.

I'm pretty sure the implementation doesn't have to declare the types of anything:

sub AUTOLOAD {
my $self = shift;
our $AUTOLOAD;
(my $fn = $AUTOLOAD) =~ s/.*://;
my $prepared = spi_prepare(
'EXECUTE ' . quote_ident($fn) . '('
. join(', ', ('?') x @_)
. ')';
# Cache it and call it.
}

> Umm,
> tl_activity_stats_sql => [qw(text[] int)]
>
> seems to me longer and rather less visually appealing than
>
> 'tl_activity_stats_sql(text[], int)'

That would work, too. But either way, having to specify the signature would be the exception rather than the rule. You'd only need to do it when calling a polymorphic function with the same number of arguments as another polymorphic function.

>> and only provide the signature when I need to disambiguate between
>> polymorphic variants.
>
> Or need to qualify the type of the argument for some other reason, like
> passing an array reference.

I don't think it's necessary. I mean, if you're passed an array, you should of course pass it to PostgreSQL, but it can be anyarray.

> But perhaps we can agree on one of the options a/b/c above and then
> this issue will be less relevant. It's not like you'd be saving much
> typing:
>
> call('tl_activity_stats_sql', @args)
> call(tl_activity_stats_sql => @args)
> SP->tl_activity_stats_sql(@args)

No, but the latter is more Perlish.

> You could always add a trivial SP::AUTOLOAD wrapper function to your
> plperl.on_init code :)

Yeah yeah. I could even put one on CPAN. ;-P But where are you caching planned functions?

Best,

David


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: "David E. Wheeler" on 15 Feb 2010 17:58
On Feb 15, 2010, at 2:42 PM, Tim Bunce wrote:

> I've not really looked the the DBD::Pg code much so this seemed like a
> good excuse... It looks like the default is to call PQprepare() with
> paramTypes Oid values of 0.

Yes, IIRC, 0 == unknown as far as the server is concerned. It just tells the server to resolve it when it can.

> http://developer.postgresql.org/pgdocs/postgres/libpq-exec.html says
> "If paramTypes is NULL, or any particular element in the array is zero,
> the server assigns a data type to the parameter symbol in the same way
> it would do for an untyped literal string."

Right, exactly.

> But I don't know if that means it has the same semantics as using
> 'unknown' as a type to PL/Perl's spi_prepare(). The docs for
> spi_prepare() don't mention if type parameters are optional or what
> happens if they're omitted.
> http://developer.postgresql.org/pgdocs/postgres/plperl-builtins.html#PLPERL-DATABASE

Same as in SQL PREPARE, I'm sure. Ultimately that's what's doing the work, IIUC.

> Looking at the code I see spi_prepare() maps the provided arg type names
> to oids then calls SPI_prepare(). The docs for SPI_prepare() also don't
> mention if the type parameters are optional or what happens if they're omitted.
> The docs for the int nargs parameter say "number of input *parameters*"
> not "number of parameters that Oid *argtypes describes"
> http://developer.postgresql.org/pgdocs/postgres/spi-spi-prepare.html
>
> Guess I need to go and check the current behaviour... see below.

And like maybe a doc patch might be useful.

> I'm currently using:
>
> my $placeholders = join ",", map { '$'.$_ } 1..$arity;
> my $plan = spi_prepare("select * from $spname($placeholders)", @$arg_types) };

Ah, yeah, that's better, but I do think you should use quote_ident() on the function name.

> and it turns out that spi_prepare is happy to prepare a statement with
> more placeholders than there are types provided.

Types or args?

> I'm a little nervous of relying on that undocumented behaviour.
> Hopefully someone can clarify if that's expected behaviour.

It's what I would expect, but I'm not an authority on this stuff.

> So, anyway, I've now extended the code so the parenthesis and types
> aren't needed. Thanks for prompting the investigation :)

Yay!

>> I don't think it's necessary. I mean, if you're passed an array, you
>> should of course pass it to PostgreSQL, but it can be anyarray.
>
> Sure, you can pass an array in encoded string form, no problem.
> But specifying in the signature a type that includes [] enables
> you to use a perl array _reference_ and let call() look after
> encoding it for you.
>
> I did it that way round, rather than checking all the args for refs on
> every call, as it felt safer, more efficient, and more extensible.

IIRC (again, sorry), that's what DBD::Pg does: It checks all the args and turns an array into an SQL array, without regard to specified types.

>> No, but the latter is more Perlish.
>
> True. You can't specify a schema though, and the 'SP' is somewhat
> artificial. Still, I'm coming round to the idea :)

What about `SP->schema::function_name()`? Agreed that SP is artificial, but there needs to be some kind of handle for AUTOLOAD to wrap itself around. Maybe a singleton object instead? (I was kind of thinking of SP as that, anyway:

use constant SP => 'PostgreSQL::PLPerl';

)

>> Yeah yeah. I could even put one on CPAN. ;-P
>
> I think it only needs this (untested):
>
> package SP;
> sub AUTOLOAD { our $AUTOLOAD =~ s/^SP:://; shift; call($AUTOLOAD, @_); }

Yep. Might be nice sugar to just throw in your module anyway.

> I could either add an extra module (PostgreSQL::PLPerl::Call::SP)
> or add a fancy import hook like:
>
> use PostgreSQL::PLPerl::Call qw(:AUTOLOAD => 'SP');

The latter is nice, as then the DBA can specify the name of package/global object.

Best,

David


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: "David E. Wheeler" on 16 Feb 2010 12:11
On Feb 16, 2010, at 4:08 AM, Tim Bunce wrote:

>> Yes, IIRC, 0 == unknown as far as the server is concerned. It just
>> tells the server to resolve it when it can.
>
> An extra source of puzzlement is that the oid of the 'unknown' type is
> 705 not 0, and the unknown type isn't discussed in the docs (as far as I
> could see).

Yes, I noticed that, too. Greg, do you know the answer to that?

>>> http://developer.postgresql.org/pgdocs/postgres/libpq-exec.html saysGuess I need to go and check the current behaviour... see below.
>>
>> And like maybe a doc patch might be useful.
>
> I would be great if someone who understood

Can any SPI experts chime in here? It seems that the ability to omit types for parameters in spi_prepare() is undocumented. Is that officially okay?

> These appear to be identical in behaviour:
>
> spi_prepare("select * from foo($1,$2)", 'unknown', 'unknown');
> spi_prepare("select * from foo($1,$2)", 'unknown')
> spi_prepare("select * from foo($1,$2)")

Ah, interesting.

> Wouldn't work unless you'd installed an AUTOLOAD function into each
> schema:: package that you wanted to use. (schema->SP::function_name()
> could be made to work but that's just too bizzare :)

Maybe SP->schema('public')->function_name()? I kind of like the idea of objects created for specific schemas, though (as in your example). Maybe that, too, is something that could be specified in the `use`statement. Or maybe `SP::schema->function`? That's kind of nice, keeps things encapsulated under SP. You could then do the identifier quoting, too. The downside is that, once loaded, the schema package names would be locked down. If I created a new schema in the connection, SP wouldn't know about it.

> Something like that is probably best. I've made PostgreSQL::PLPerl::Call
> export both &call and &SP where SP is a constant containing the name
> of a class (PostgreSQL::PLPerl::Call::SP) that just has an AUTOLOAD.

Cool, thanks!

From the docs:

> Immediately after the function name, in parenthesis, a comma separated list of
> type names can be given. For example:
>
> 'pi()'
> 'generate_series(int,int)'
> 'array_cat(int[], int[])'
> 'myschema.myfunc(date, float8)'

It could also just be 'pi', no?

> Functions with C<varadic> arguments can be called with a fixed number of
> arguments by repeating the type name in the signature the same number of times.

I assume that type names can be omitted her, too, yes?

> $pi = SP->pi();
> $seqn = SP->nextval($sequence_name);
>
> Using this form you can't easily specify a schema name or argument types, and
> you can't call varadic functions.

Why not?

Also, I notice a few `==head`s. I think that's one too many "="s.

> You can take this approach further by specifying some of the arguments in the
> anonymous subroutine so they don't all have to be provided in the call:
>
> $some_func = sub { call('some_func(int, date[], int)', $foo, shift, $debug) };
> ...
> $val = $some_func->(\@dates);

Currying! :-)

> If the function was executed in scalar context then an exception will be thrown
> if more than one row is returned. For example:

Someone's going to want an iterator object/cursor. :-P

> For varadic functions, separate plans are created and cached for each distinct
> number of arguments the function is called with.

Why?

> Functions with a varadic argument can't be called with no values for that
> argument. You'll get a "function ... does not exist" error. This appears to be
> a PostgreSQL limitation.

Hrm. Worth enquiring about.

So, is this on GitHub yet? That way I can submit patches.

Best,

David


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: "David E. Wheeler" on 16 Feb 2010 12:51
On Feb 16, 2010, at 9:43 AM, Richard Huxton wrote:

> Perhaps it would be better to be explicit about what's going on?
> SEARCHPATH->function()
> SCHEMA('public')->function2()
>
> Or did "SP" mean "Stored Procedure"?

Yes.

> On a (kind of) related note, it might be worthwhile to mention search_path in the docs and point out it has the same pros/cons as unix file paths.

+1. It's a little like file paths and a little like name spaces, without quite being either one.

Best,

David


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 |  Next  |  Last
Pages: 1 2 3
Prev: [HACKERS] "0 0" return tag
Next: [HACKERS] LISTEN/NOTIFY versus encoding conversion