On 31/03/2008, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> 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?
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;
> #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
> #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():
> 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
> #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
> 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
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
> -----END PGP SIGNATURE-----
In response to
pgsql-hackers by date
|Next:||From: Michael Paesold||Date: 2008-04-01 10:25:14|
|Subject: Re: [HACKERS] How embarrassing: optimization of a one-shot query doesn't work|
|Previous:||From: NikhilS||Date: 2008-04-01 08:40:02|
|Subject: Re: [email@example.com: Re: [BUGS] Problem identifying constraints which should not be inherited]|