Re: [HACKERS] SQL procedures

2017-11-22 19:01 GMT+01:00 Peter Eisentraut <
peter(dot)eisentraut(at)2ndquadrant(dot)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
>
>