Re: Convert pltcl from strings to objects

On 2/23/16 6:04 AM, Victor Wagner wrote:
> On Mon, 22 Feb 2016 17:57:36 -0600
> Jim Nasby <Jim.Nasby@BlueTreble.com> wrote:
>
>> On 2/18/16 6:26 AM, Victor Wagner wrote:
>> (BTW, I also disagree about using a Tcl list for prodesc. That's an
>> object in a *Postgres* hash table; as such it needs to be managed by
>> Postgres, not Tcl. AFAIK the Postgres ResourceOwner stuff would be
>> the correct way to do that (but I'm not exactly an expert on that
>> area).
>
> As far as I understand ResourceOwner is completely different garbage
> collection scheme, than reference counting. ResourceOwner mechanism lets
> free everything associated with transaction when transaction is over.

Ahh, right.

> Here we have another case. prodesc is a global thing. And it is shared
> between different operations. Problem was that there is no partcular
> owner, and we have to wait when last operation which deals with it
> would finish. It looks like perfect job for reference counting.

I've just tried to wrap my head around what's going on with prodesc and
failed... specifically, I don't understand this claim in the comment:

* Add the proc description block to the hashtable.  Note we do not
* attempt to free any previously existing prodesc block.  !!This is
* annoying, but necessary since there could be active calls using
* the old prodesc.!!

What else could be referencing it? I realize it's stored in
pltcl_proc_htab, but AFAICT that's backend-local. So I don't understand
what else could be referencing it.

I suspect this code predates conveniences that have been added to
Postgres over the years and that the whole way this stuff is being
cached should be overhauled to do whatever plpgsql does now...

> Actually, it doesn't matter much, because it wastes some memory only on
> procedure change, and typically in the heavy loaded production
> environments server side code doesn't change too frequently.

... but this is the reason it's maybe not a big priority. Though,
prodesc does appear to be a few hundred bytes, and this is something
that will affect *every* backend that has executed a procedure that then
gets modified.


> Moreover. errorcode and errorinfo information travel behind the scenes
> and cannot break any code which doesn't try to explicitely access it.

Yeah, Karl will be looking into this as part of a separate patch.


> I've mean, that there is no need to be to aggressive in the type
> conversion. You'll never know if Tcl code would actually access some
> tuple elements or not. Let's leave conversion to the time of actual use
> of values.

Will also be looked at as part of a separate patch.

> Please, send updated patch to the list in this thread, so it would
> appear in the commitfest and I can mark your patch as "ready for
> committer".

Done!
--
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