Re: [Postgres-xc-developers] Param handling refactoring - Part 1

[Ashutosh B. <ash...@en...>]

Hi Amit,
I am looking at this patch.

First I looked at the testcase added. It seems testing the parameter
handling for queries arising from plpgsql functions. The function
prm_func() seems to be doing that. Can you please add some comments in this
function specifying what is being tested in various sets of statements in
function. Also, it seems to be using two tables prm_emp1 and prm_emp2. The
first one is being used to populate the other one and a variable inside the
function. Later only the other is being used. Can we just use a single
table instead of two? Can we use an existing volatile function instead of a
new one like prm_volfunc()?

On Tue, Feb 26, 2013 at 9:51 AM, Amit Khandekar <
ami...@en...> wrote:

> There has been errors like :
> "Cannot find parameter $4" or
> "Bind supplies 4 parameters while Prepare needs 8 parameters" that we
> have been getting for specific scenarios. These scenarios come up in
> plpgsql functions. This is the root cause:
>
> If PLpgSQL_datum.dtype is not a simple type (PLPGSQL_DTYPE_VAR), the
> parameter types (ParamExternData.ptype) for such plpgsql functions are
> not set until when the values are actually populated. Example of such
> variables is record variable without %rowtype specification. The
> ParamListInfo.paramFetch hook function is called when needed to fetch
> the such parameter types. In the XC function
> pgxc_set_remote_parameters(), we do not consider this, and we check
> only the ParamExternData.ptype to see if parameters are present, and
> end up with lesser parameters than the actual parameters, sometimes
> even ending up with 0 parameter types.
>
> During trigger support implementation, it was discovered that due to
> this issue,
> the NEW.field or OLD.field cannot be used directly in SQL statements.
>
> Actually we don't even need parameter types to be set at plan time in
> XC. We only need them at the BIND message. There, we can anyway infer
> the types from the tuple descriptor. So the attached patch removes all
> the places where parameter types are set, and derives them when the
> BIND data row is built.
>
> I have not touched the SetRemoteStatementName function in this patch.
> There can be scenarios where user calls PREPARE using parameter types,
> and in such cases it is better to use these parameters in
> SetRemoteStatementName() being called from BuildCachedPlan with
> non-NULL boundParams. Actually use of parameter types during PREPARE
> and rebuilding cached plans etc will be dealt further after this one.
> So, I haven't removed param types altogether.
>
> We also need to know whether the parameters are supplied through
> source data plan (DMLs) or they are external. So added a field
> has_internal_params in RemoteQuery to make this difference explicit.
> Data row and parameters types are built in a different manner for DMLs
> and non-DMLs.
>
> Moved the datarow generation function from execTuples.c to execRemote.c .
>
> Regressions
> -----------------
>
> There is a parameter related error in plpgsql.sql test, which does not
> occur now, so corrected the expected output. It still does not show
> the exact output because of absence of trigger support.
>
> Added new test xc_params.sql which would be further extended later.
>
>
> ------------------------------------------------------------------------------
> Everyone hates slow websites. So do we.
> Make your web apps faster with AppDynamics
> Download AppDynamics Lite for free today:
> http://p.sf.net/sfu/appdyn_d2d_feb
> _______________________________________________
> Postgres-xc-developers mailing list
> Pos...@li...
> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>
>


-- 
Best Wishes,
Ashutosh Bapat
EntepriseDB Corporation
The Enterprise Postgres Company