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

From Stephen Frost
Subject Re: actualized SQL/PSM patch
Date
Msg-id 20080331195506.GK4999@tamriel.snowman.net
Whole thread Raw
Responses Re: actualized SQL/PSM patch
List pgsql-hackers
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
minorand far between would it be more reasonable to just make PL/pgSQL play double-duty and have a flag somewhere to
indicatewhen 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
languageyou 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()"
nearline 2Even 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
regeneratingthe expectedoutput 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/pgSQLcannot 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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: POSIX shared memory support
Next
From: Tom Lane
Date:
Subject: Re: pgkill