Hello pgsql-hackers,
Following a recent discussion on the JDBC list
(http://archives.postgresql.org/pgsql-jdbc/2004-05/msg00100.php) I'm
looking at modifying when unnamed statements received via a v3 protocol
Parse message are planned. The idea is to delay planning until the Bind
message arrives, so that the planner benefits from knowing the actual
parameter values in use. This means that clients such as JDBC can use an
unnamed Parse/Bind as a way to provide typed and/or binary parameter
values without running the risk of worsening the query plan.
As this is my first excursion into the backend code (and the first time
I've touched any C for about a year, for that matter) I'd like to get a
sanity check on what I'm doing..
So far my changes involve:
1. Add a ParamListInfo parameter to pg_plan_query(), pg_plan_queries(),
planner(); this provides actual parameter values or NULL if
unavailiable. All existing callers except execute_bind_message pass NULL.
2. In execute_parse_message, if parsing into the unnamed statement,
first parse the query. If there are any parameters present, don't plan
the query yet (set unnamed_stat_pstmt->plan_list = NULL). Otherwise,
parse and plan as normal.
3. In execute_bind_message, if the source statement is not planned
(plan_list == NULL), plan it, passing the concrete parameter values
received in the Bind message. The planning happens using the target
portal's memory context; the resulting plan_list is only used for this
portal and the source statement remains unplanned. Move the call to
PortalDefineQuery() so that it occurs after parameters have been
received and any planning has been done.
4. In planner(), if parameters are provided, replace Param nodes with
Const nodes in the query tree where appropriate.
For 4) I have a couple of options:
a) thread a ParamListInfo parameter through the planner code. Pass it
to eval_const_expressions(). Make eval_const_expressions() do the
Param->Const replacement where possible in the mutated tree it produces.
b) if parameters are provided, do the Param->Const replacement at the
start of planner() using a custom tree walker.
4a) appears more efficient as it doesn't need another intermediate tree
copy, but it requires threading the parameter values through many
planner functions. There are quite a few (20-30ish?) functions affected.
4b) appears simpler to implement but requires more tree copying at runtime.
...
Does this look like a reasonable approach to take? Any suggestions, or
gotchas I've missed?
-O
