Sunday, June 1, 2008

Re: [HACKERS] explain doesn't work with execute using

*** ./src/backend/executor/spi.c.orig 2008-06-01 17:26:19.000000000 +0200
--- ./src/backend/executor/spi.c 2008-06-01 17:35:01.000000000 +0200
***************
*** 63,68 ****
--- 63,71 ----
static MemoryContext _SPI_execmem(void);
static MemoryContext _SPI_procmem(void);
static bool _SPI_checktuples(void);
+ static Portal _SPI_cursor_open(const char *name, SPIPlanPtr plan,
+ Datum *Values, const char *Nulls,
+ bool read_only, int pflags);


/* =================== interface functions =================== */
***************
*** 908,921 ****


/*
! * SPI_cursor_open()
! *
! * Open a prepared SPI plan as a portal
*/
! Portal
! SPI_cursor_open(const char *name, SPIPlanPtr plan,
Datum *Values, const char *Nulls,
! bool read_only)
{
CachedPlanSource *plansource;
CachedPlan *cplan;
--- 911,923 ----


/*
! * _SPI_cursor_open()
! * Open a prepared SPI plan as portal, allows set parameter's pflags
*/
! static Portal
! _SPI_cursor_open(const char *name, SPIPlanPtr plan,
Datum *Values, const char *Nulls,
! bool read_only, int pflags)
{
CachedPlanSource *plansource;
CachedPlan *cplan;
***************
*** 997,1003 ****
ParamExternData *prm = &paramLI->params[k];

prm->ptype = plan->argtypes[k];
! prm->pflags = 0;
prm->isnull = (Nulls && Nulls[k] == 'n');
if (prm->isnull)
{
--- 999,1005 ----
ParamExternData *prm = &paramLI->params[k];

prm->ptype = plan->argtypes[k];
! prm->pflags = pflags;
prm->isnull = (Nulls && Nulls[k] == 'n');
if (prm->isnull)
{
***************
*** 1130,1135 ****
--- 1132,1154 ----


/*
+ * SPI_cursor_open()
+ *
+ * Open a prepared SPI plan as a portal
+ */
+ Portal
+ SPI_cursor_open(const char *name, SPIPlanPtr plan,
+ Datum *Values, const char *Nulls,
+ bool read_only)
+ {
+ return _SPI_cursor_open(name, plan,
+ Values, Nulls,
+ read_only, 0);
+
+ }
+
+
+ /*
* SPI_cursor_open_with_args()
*
* Parse and plan a query and open it as a portal. Like SPI_execute_with_args,
***************
*** 1177,1183 ****
/* SPI_cursor_open expects to be called in procedure memory context */
_SPI_procmem();

! result = SPI_cursor_open(name, &plan, Values, Nulls, read_only);

/* And clean up */
_SPI_curid++;
--- 1196,1203 ----
/* SPI_cursor_open expects to be called in procedure memory context */
_SPI_procmem();

! /* all params has PARAM_FLAG_CONST flag */
! result = _SPI_cursor_open(name, &plan, Values, Nulls, read_only, PARAM_FLAG_CONST);

/* And clean up */
_SPI_curid++;
hello

2008/6/1 Tom Lane <tgl@sss.pgh.pa.us>:
> "Pavel Stehule" <pavel.stehule@gmail.com> writes:
>> I found following bug - using explain in stored procedures like:
>> ...
>> produce wrong result. Real plan is correct, etc variables are
>> substituted. Bud this explain show variables.
>
> This seems to be correctable with a one-line patch: make SPI_cursor_open
> set the CONST flag on parameters it puts into the portal (attached).
> I'm not entirely sure if it's a good idea or not --- comments?

We can do less invasive patch - it's much more ugly, but don't change
any other behave. I am afraid, so one-line patch can change behave of
explain statements in some cases where using variables is correct.

Regards
Pavel Stehule

>
> regards, tom lane
>
> Index: src/backend/executor/spi.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/backend/executor/spi.c,v
> retrieving revision 1.195
> diff -c -r1.195 spi.c
> *** src/backend/executor/spi.c 12 May 2008 20:02:00 -0000 1.195
> --- src/backend/executor/spi.c 1 Jun 2008 15:33:13 -0000
> ***************
> *** 997,1003 ****
> ParamExternData *prm = &paramLI->params[k];
>
> prm->ptype = plan->argtypes[k];
> ! prm->pflags = 0;
> prm->isnull = (Nulls && Nulls[k] == 'n');
> if (prm->isnull)
> {
> --- 997,1010 ----
> ParamExternData *prm = &paramLI->params[k];
>
> prm->ptype = plan->argtypes[k];
> ! /*
> ! * We mark the parameters as const. This has no effect for simple
> ! * execution of a plan, but if more planning happens within the
> ! * portal (eg via EXPLAIN), the effect will be to treat the
> ! * parameters as constants. This is good and correct as long as
> ! * no plan generated inside the portal is used outside it.
> ! */
> ! prm->pflags = PARAM_FLAG_CONST;
> prm->isnull = (Nulls && Nulls[k] == 'n');
> if (prm->isnull)
> {
>

No comments: