Re: Add support for SRF and returning composites to pl/tcl

From: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Karl Lehenbauer <karl(at)flightaware(dot)com>
Subject: Re: Add support for SRF and returning composites to pl/tcl
Date: 2017-01-08 04:57:45
Message-ID: 33035f75-26b0-29ed-7361-4c7f56a81471@BlueTreble.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1/6/17 2:45 PM, Tom Lane wrote:
> The only thing that seems significant is that we'd change the SQLSTATE
> for the "odd number of list items" error: pltcl_trigger_handler has
>
> (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
> errmsg("trigger's return list must have even number of elements")));
>
> and that would become
>
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("column name/value list must have even number of elements")));
>
> But that's probably fine; it's hard to believe anyone is depending on this
> particular case --- and I think the argument that this is legitimately
> a TRIGGER_PROTOCOL_VIOLATED case is a bit thin anyway.

Yeah, I forgot to mention that, but I did look at both cases and felt
the same.

> While I was checking the patch to verify that it didn't change any
> behavior, I noticed that it did, and there's a pre-existing bug here:
> pltcl_build_tuple_result is applying utf_e2u to the Tcl_GetString results,
> but surely that should be utf_u2e instead. Fortunately we haven't shipped
> this code yet.

Ugh.

For this patch lets just fix that (see attached). Moving forward, I
think it'd be good to improve this...

There's only 2 cases of Tcl_GetString that don't then call utf_u2e (or,
amusingly, UTF_U2E); perhaps we should just get rid of all direct use of
TclGetString and replace it with something like pltcl_get_utf() and
pltcl_get_ascii()?

Oh, hell, now I see that there's an actual difference between utf_* and
UTF_*. And AFAICT, those macros don't completely solve things either; if
you make multiple calls you're still leaking memory but wouldn't know
it. Granted, existing calls seem to all be properly wrapped, but that
seems pretty error prone.

Looking at plpython, it seems that it doesn't go through any of this
trouble: if you're using python 3 it just treats everything as if it
needs encoding, and the conversion routines handle freeing things.
AFAICT that's true for identifiers as well.

So I'm thinking that we should just forbid the use of direct Tcl string
functions and pass everything though our own functions that will handle
UTF8. There are a bunch of places where we're handing static C strings
(that are clearly plain ASCII) to TCL, so we should probably continue to
support that. But in the other direction I don't think it's worth it.

TCL does have a separate string append operation as well, so we'll need
to either provide that or do all the appending using our string
functions and then pass that along.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

Attachment Content-Type Size
pltcl_fix_build_tuple_encoding.patch text/plain 832 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2017-01-08 05:19:50 Re: merging some features from plpgsql2 project
Previous Message Ryan Murphy 2017-01-08 04:51:56 Re: Adding type info etc for inheritance errmsg: "child table is missing column ..."