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.

#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

                Enjoy,

                        Stephen

Attachment: signature.asc
Description: Digital signature

Reply via email to