Re: actualized SQL/PSM patch - Mailing list pgsql-hackers

On 31/03/2008, Stephen Frost <sfrost@snowman.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-----
>
>