Skip site navigation (1) Skip section navigation (2)

Re: plpgsql versus SPI plan abstraction

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: plpgsql versus SPI plan abstraction
Date: 2013-01-30 21:29:47
Message-ID: CAFj8pRDch78PNU81vfLYkZ9pg4-7hkTXKwyBMM2=_hpvBgTV8Q@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
2013/1/30 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> I looked into the odd behavior noted recently on pgsql-novice that
> the error context stack reported by plpgsql could differ between
> first and subsequent occurrences of the same error:
> http://www.postgresql.org/message-id/26370.1358539743@sss.pgh.pa.us
>
> This seems to be specific to errors that are detected at plan time for a
> potentially-simple expression.  The example uses "1/0" which throws an
> error when eval_const_expressions tries to simplify it.  From plpgsql's
> viewpoint, the error happens when it tries to use GetCachedPlan() to get
> a plan tree that it can check for simple-ness.  In this situation, we
> have not pushed _SPI_error_callback onto the error context stack, so the
> line it might contribute to the context report doesn't show up.
> However, exec_simple_check_plan is set up to mark the PLpgSQL_expr as
> non-simple at the outset, so that when it loses control due to the
> error, that's how the already-cached PLpgSQL_expr is marked.  Thus, on a
> subsequent execution, we don't go through there but just pass off
> control to SPI_execute_plan --- and it *does* set up _SPI_error_callback
> before calling GetCachedPlan().  So now you get the additional line of
> context.
>
> There doesn't seem to be a comparable failure mode before 9.2, because
> in previous releases planning would always occur before we created a
> CachedPlanSource at all; so the failure would leave plpgsql still
> without a cached PLpgSQL_expr, and the behavior would be consistent
> across tries.
>
> My first thought about fixing this was to export _SPI_error_callback
> so that plpgsql could push it onto the context stack before doing
> GetCachedPlan.  But that's just another piercing of the veil of
> modularity.  What seems like a better solution is to export a SPI
> wrapper of GetCachedPlan() that pushes the callback locally.  With a
> bit more work (a wrapper to get the CachedPlanSource list) we could
> also stop letting pl_exec.c #include spi_priv.h, which is surely a
> modularity disaster from the outset.
>
> Does anyone see a problem with back-patching such a fix into 9.2,
> so as to get rid of the context stack instability there?
>

this is clean bug, so please, back-patch it in 9.2.

Regards

Pavel

> BTW, I'm also wondering if it's really necessary for plpython/plpy_spi.c
> to be looking into spi_priv.h ...
>
>                         regards, tom lane
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


In response to

pgsql-hackers by date

Next:From: Bruce MomjianDate: 2013-01-30 21:35:11
Subject: Re: pg_ctl idempotent option
Previous:From: Kevin GrittnerDate: 2013-01-30 21:28:36
Subject: Re: autovacuum not prioritising for-wraparound tables

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group