2017-11-22 19:01 GMT+01:00 Peter Eisentraut < peter.eisentr...@2ndquadrant.com>:
> On 11/20/17 16:25, Andrew Dunstan wrote: > > I've been through this fairly closely, and I think it's pretty much > > committable. The only big item that stands out for me is the issue of > > OUT parameters. > > I figured that that's something that would come up. I had intentionally > prohibited OUT parameters for now so that we can come up with something > for them without having to break any backward compatibility. > > From reading some of the references so far, I think it could be > sufficient to return a one-row result set and have the drivers adapt the > returned data into their interfaces. The only thing that would be a bit > weird about that is that a CALL command with OUT parameters would return > a result set and a CALL command without any OUT parameters would return > CommandComplete instead. Maybe that's OK. > T-SQL procedures returns data or OUT variables. I remember, it was very frustrating Maybe first result can be reserved for OUT variables, others for multi result set Regards Pavel > > Default Privileges docs: although FUNCTIONS and ROUTINES are equivalent > > here, we should probably advise using ROUTINES, as FUNCTIONS could > > easily be take to mean "functions but not procedures". > > OK, I'll update the documentation a bit more. > > > CREATE/ALTER PROCEDURE: It seems more than a little odd to allow > > attributes that are irrelevant to procedures in these statements. The > > docco states "for compatibility with ALTER FUNCTION" but why do we want > > such compatibility if it's meaningless? If we can manage it without too > > much violence I'd prefer to see an exception raised if these are used. > > We can easily be more restrictive here. I'm open to suggestions. There > is a difference between options that don't make sense for procedures > (e.g., RETURNS NULL ON NULL INPUT), which are prohibited, and those that > might make sense sometime, but are currently not used. But maybe that's > too confusing and we should just prohibit options that are not currently > used. > > > In create_function.sgml, we have: > > > > If a schema name is included, then the function is created in the > > specified schema. Otherwise it is created in the current schema. > > - The name of the new function must not match any existing function > > + The name of the new function must not match any existing > > function or procedure > > with the same input argument types in the same schema. However, > > functions of different argument types can share a name (this is > > called <firstterm>overloading</firstterm>). > > > > The last sentence should probably say "functions and procedures of > > different argument types" There's a similar issue in > create_procedure.sqml. > > fixing that > > > In grant.sgml, there is: > > > > + The <literal>FUNCTION</literal> syntax also works for > aggregate > > + functions. Or use <literal>ROUTINE</literal> to refer to a > > function, > > + aggregate function, or procedure regardless of what it is. > > > > > > I would replace "Or" by "Alternatively,". I think it reads better that > way. > > fixing that > > > In functions.c, there is: > > > > /* Should only get here for VOID functions */ > > - Assert(fcache->rettype == VOIDOID); > > + Assert(fcache->rettype == InvalidOid || fcache->rettype > > == VOIDOID); > > fcinfo->isnull = true; > > result = (Datum) 0; > > > > The comment should also refer to procedures. > > fixing that > > > It took me a minute to work out what is going on with the new code in > > aclchk.c:objectsInSchemaToOids(). It probably merits a comment or two. > > improving that > > > We should document where returned values in PL procedures are ignored > > (plperl, pltcl) and where they are not (plpython, plpgsql). Maybe we > > should think about possibly being more consistent here, too. > > Yeah, suggestions? I think it makes sense in PL/pgSQL and PL/Python to > disallow return values that would end up being ignored, because the only > way a return value could arise is if user code explicit calls > RETURN/return. However, in Perl, the return value is the result of the > last statement, so prohibiting a return value would be very > inconvenient. (Don't know about Tcl.) So maybe the current behavior > makes sense. Documentation is surely needed. > > > The context line here looks odd: > > > > CREATE PROCEDURE test_proc2() > > LANGUAGE plpythonu > > AS $$ > > return 5 > > $$; > > CALL test_proc2(); > > ERROR: PL/Python procedure did not return None > > CONTEXT: PL/Python function "test_proc2" > > > > Perhaps we need to change plpython_error_callback() so that "function" > > isn't hardcoded. > > fixing that > > -- > Peter Eisentraut http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services > >
