Re: [HACKERS] plpgsql versus SPI plan abstraction

2013-01-30 Thread Pavel Stehule
2013/1/30 Tom Lane t...@sss.pgh.pa.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.1358539...@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@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql versus SPI plan abstraction

2013-01-30 Thread Jan UrbaƄski

On 30/01/13 22:23, Tom Lane wrote:

BTW, I'm also wondering if it's really necessary for plpython/plpy_spi.c
to be looking into spi_priv.h ...


As far as I can tell, it's not necessary, spi.h would be perfectly fine.

Cheers,
Jan


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers