Re: pl/python custom datatype parsers

On 04/02/11 17:19, Hitoshi Harada wrote:
> 2011/1/28 Jan Urbański <wulczer@wulczer.org>:
>> On 23/12/10 15:15, Jan Urbański wrote:
>>> Here's a patch implementing custom parsers for data types mentioned in
>>> http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
>>> an incremental patch on top of the plpython-refactor patch sent eariler.
>>
>> Updated to master.
>
> I reviewed this for some time today.

Thank you.

> The patch applies with hunks, compiles and tests are passed, though it
> looks like not having additional test along with it.

I added a simple test. I had to add an expected file for the case when
hstore is compiled without PL/Python integration.

> - in hstore_plpython.c,
>   PLyParsers parsers = {
>       .in = hstore_to_dict,
>       .out = dict_to_hstore
>   };
>   I'm not sure if this coding style is used anywhere in the core.
> Isn't this the C99 style?

Ooops, you're right. Fixed.

> - You need define custom variable class to use this feature.
> plpython.hstore = 'public.hstore'. I wonder why it's called
> plpython[u].hstore = 'public.hstore' (with 'u') because the language
> is called "plpythonu".

I think plpython.hstore was what showed up in discussion... I'd be fine
with calling the variable plpythonu.hstore, if that's the consensus.

> - typo in plpython.h,
>   Types for parsres functions that ...

Fixed.

> - I tried the sample you mention upthread,
>   regression=# select pick_one('a=>3, b=>4', 'b');
>   ERROR:  TypeError: string indices must be integers
>   CONTEXT:  PL/Python function "pick_one"
>
>   My python is 2.4.3 again.

Hm, this means that the hstore has not been transformed into a Python
dict, but into a string, which is what happens if you *don't* have
plpython hstore integration enabled. I think that was because of an
issue with my changes to hstore's Makefile, that made it compile without
Python support, even if the sources were configured with --with-python.

There's also a gotcha: if you set plpython.hstore to 'public.hstore',
you will have to DROP (or CREATE OR REPLACE again) all functions that
accept or return hstores, because their I/O routines are already cached.
Not sure how big of a problem that is (or how to fix it in an elegant
manner). Making the parameter PGC_POSTMASTER is an easy solution... but
not very nice.

> That's it for now. It is an exciting feature and plpython will be the
> first language to think of when you're building "object database" if
> this feature is in. The design here will affect following pl/perl and
> other so it is important enough to discuss.

Yes, I ended up writing this patch as a PoC of how you can integrate
procedural languages with arbitrary addon modules, so it would be good
to have a discussion about the general mechanisms. I'm aware that this
discussion, and subsequently this patch, might be punted to 9.2
(although that would be a shame).

Cheers,
Jan