Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix constant_position for prepared statement #12

Closed
wants to merge 1 commit into from

Conversation

dblugeon
Copy link

@dblugeon dblugeon commented Mar 9, 2017

Hello Dalibo Team.
This PR permit to store the constant position for variables in prepared Statement in this pgqsEntry store.

To reproduce the problem :

create a table with 2 or more columns.
create a program which use a prepared statement (I used a java program) for an select query.

select * from pg_qualstat()

with previous version :
you can see null or -X for constant_position.
If you rexecute the test program, the constant_position decrement.

with the pr version
you will have constant_position

The constant_position will permit to have an explain plan for prepared statement in powa-web with the good order for variable.

Regards

with this commit,
this pgqsEntry store the constant position for variables in preparedStatement
@rjuju
Copy link
Member

rjuju commented Mar 10, 2017

Hello,

You're absolutely right, the position is lost for a parameter (which can happen for a prepared statement, but also when using extended protocol).

However, I think your fix isn't right, since it always (not only when the position is -1) overrides the const position with the var position, the var being the column used in the OpExpr).

Unfortunately, I don't think this is something that can be fixed in pg_qualstats. The right fix would be to teach postgres to give to the Const position the original parameter position. Following patch seems to do it:

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index b19380e1b1..848c29da57 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2444,6 +2444,7 @@ eval_const_expressions_mutator(Node *node,
                            int16       typLen;
                            bool        typByVal;
                            Datum       pval;
+                           Const      *c;
 
                            Assert(prm->ptype == param->paramtype);
                            get_typlenbyval(param->paramtype,
@@ -2452,13 +2453,16 @@ eval_const_expressions_mutator(Node *node,
                                pval = prm->value;
                            else
                                pval = datumCopy(prm->value, typByVal, typLen);
-                           return (Node *) makeConst(param->paramtype,
+                           c = makeConst(param->paramtype,
                                                      param->paramtypmod,
                                                      param->paramcollid,
                                                      (int) typLen,
                                                      pval,
                                                      prm->isnull,
                                                      typByVal);
+                           c->location = param->location;
+
+                           return (Node *) c;
                        }
                    }
                }

I'll propose something similar on postgres mailing list, I don't know if it'll be accepted or backported.

@rjuju
Copy link
Member

rjuju commented Jun 8, 2017

Hello again!

Sorry for the late answer. I did proposed the patch on -hackers (see https://www.postgresql.org/message-id/20170311220932.GJ15188@nol.local), but it was unfortunately ignored. I guess you could try to answer it if you want to attract some interest, but I have no idea if that'd be eventually accepted.

@rjuju
Copy link
Member

rjuju commented Jul 15, 2021

I have a good news here. I faced the same problem recently and bumped the topic again and the patch was finally merged upstream, see postgres/postgres@be850f1.

This will however be part of psotgres 15, so it unfortunately won't be usable before ~ september 2022.

@rjuju rjuju closed this Jul 15, 2021
@dblugeon
Copy link
Author

Hello,
Thank you for this. :)

@dblugeon dblugeon deleted the patch-1 branch July 15, 2021 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants