Re: actualized SQL/PSM patch

On 31/03/2008, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> Pavel,
>
> Honestly, I havn't dug into the real patch all that deeply but I did
> notice a few minor issues which I've listed out below. The bigger
> question I have for this patch, however, is just how close is it to
> PL/pgSQL? If the differences are minor and far between would it be
> more reasonable to just make PL/pgSQL play double-duty and have a flag
> somewhere to indicate when it should be in 'PL/pgPSM' mode?
>
> Thanks.

Hello,

thank you for time. I thing so plpgsql is too much different language
than plpgpsm - mainly there is different concept of catching errors,
cursor's declaration and operation and different statements. My tip:

gram.y - conformance 10%
pl_exec.c - conf. 40%
pl_func.c - conf 80% (diff in dump functions)
scan.l - conf. 99%

I can't to say so plpgpsm is an dialect of plpgsql. Minimally there
are different parser. I am sure so supported functions can be shared,
but it's mean really dramatic changes in plpgsql code. I belive so
separated languages will be more maintainable.

>
> #1: INSTALL.plpgpsm starts out saying:
> "Installation of PL/pgSQL"
> I'm guessing you just missed changing it. Also in there:
> "For installation any PL language you need superuser's rights."
> should probably read:
> For installation of any PL language you need superuser rights.
> Or just:
> To install any PL language you need to be the database superuser.
>
> #2: pl_comp.c has a similar issue in its comments:
> pl_comp.c as the top says "Compiler part of the PL/pgSQL" ..
> plpgpsm_compile Make an execution tree for a PL/pgSQL function.
> Should read 'PL/pgPSM' there.
>
> #3: pl_comp.c uses C++ style comments for something which I'm guessing
> you didn't actually intend to even be in the patch:
> //elog(ERROR, "zatim konec");
> in do_compile().
>
> #4: Again in pl_comp.c there are C++ style comments, this time for
> variables which can probably just be removed:
> // PLpgPSM_nsitem *nse;
> // char *cp[1];
>
> #5: In pl_exec.c, exec_stmt_open, again you have C++ style comments:
> // ToDo: Holdable cursors
>
> #6: In the "expected.out", for the 'fx()' function, the CONTEXT says:
> CONTEXT: compile of PL/pgSQL function "fx()" near line 2
> Even though it says "LANGUAGE plpgpsm", which seems rather odd.
>
> #7: gram.y also has in the comments "Parser for the PL/pgSQL" ..
>
> #8: plpgpsm_compile_error_callback() passes "PL/pgSQL" to errcontext(),
> probably the cause of #7 and fixing it and regenerating the expected
> output would probably work.
>
> #9: plerrcodes.h also has "PL/pgSQL error codes" in the comments at the
> top.
>
> #10: ditto for pl_exec.c "Executor for the PL/pgSQL" ..
>
> #11: more error-strings being passed with "PL/pgSQL" in it in pl_exec.c:
> in exec_stmt_prepare() and exec_prepare_plan(), exec_stmt_execute():
> eg:
> cannot COPY to/from client in PL/pgSQL
> cannot begin/end transactions in PL/pgSQL
> cannot manipulate cursors directly in PL/pgSQL
>
> #12: Also in the comments for plpgpsm_estate_setup are references to
> PL/pgSQL.
>
> #13: pl_funcs.c also says "Misc functions for the PL/pgSQL" ..
>
> #14: plpgpsqm_dumptree outputs:
> Execution tree of successfully compiled PL/pgSQL function
> Should be updated for PL/pgPSM
>
> #15: Header comment in pl_handler.c also says PL/pgSQL
>
> #16: Function-definition comment for plpgpsqm_call_handler also says
> PL/pgSQL
> ditto for plpgpsm_validator
>
> #17: Header comment in plpgpsm.h say PL/pgSQL, other comments later as
> well, such as for the PLpgPSM_plugin struct
>
> #18: Also for the header comment in scan.l
>

I'll correct it, thank you very much

Pavel
> Enjoy,
>

>
> Stephen
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
>
> iD8DBQFH8UGarzgMPqB3kigRAv2uAJ0RR2WA37Qx14Ty9mx3pzd6hbazqACfZaG1
> NRxCF2vC9+BbVlSHM9swc1A=
> =fFpD
> -----END PGP SIGNATURE-----
>
>