On 31/03/2008, Stephen Frost [EMAIL PROTECTED] 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-
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
