Re: Increase pltcl test coverage

On 1/9/17 5:38 PM, Tom Lane wrote:
> Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> writes:
>> Hmm... I suspect there's more places where this could be a problem. For
>> example, pltcl_quote calls palloc, which could ereport as well.
>
> Yeah. I looked at that but couldn't get terribly excited about it,
> because AFAICS, Tcl in general is apt to fall over under sufficient
> memory pressure. There are too many API functions with no failure
> return provision, even though they clearly must do memory allocation
> under the hood. (The fact that they've even got ckalloc() tells you
> what their philosophy is here: they're content to PANIC on OOM.)

Uhm... wow. According to [1] that's going to result in TCL calling
abort(). I'm guessing there's no reasonable way for us to recognize a
TCL abort as something that we didn't need to panic on...

In any case, AFAICT there's still a bug here: if PG hits a memory error,
we'll ERROR, which is going to leave the interpreter right back in a bad
state for the rest of that session. That doesn't seem so good. It also
means a pltcl proc wouldn't get the chance to trap the error.

Though, since a memory error could just as likely come out of tcl, which
is going to panic us anyway, I guess it doesn't matter.

> I think pltcl should attempt to cover any error conditions that aren't
> purely out-of-memory ones, but in a quick scan I didn't see any other
> hazards.

Yeah, I think we're OK on that front.

I was hoping to establish a precedent for all the new TCL functions that
pltcl provides so it would be extremely unlikely that the returnnext bug
could be repeated. Putting them in a separate file with a nice comment
block would be another alternative.
--
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)