Re: Issues for named/mixed function notation patch

2009/10/2 Jeff Davis <pgsql(at)j-davis(dot)com>:
> On Thu, 2009-10-01 at 17:56 +1000, Brendan Jurd wrote:
>> I've had a look through the documentation and cleaned up a few things.
>
> Thanks, that is helpful. I have included that along with some other
> comment updates in the attached patch.
>
> Paval,
>
> In ProcedureCreate(), you match the argument names to see if anything
> changed (in which case you raise an error). The code for that looks more
> complex than I expected because it keeps track of the two argument lists
> using different array indexes (i and j). Is there a reason it you can't
> just iterate through with something like:
>
>  if (p_oldargmodes[i] == PROARGMODE_OUT ||
>      p_oldargmodes[i] == PROARGMODE_TABLE)
>    continue;
>
>  if (strcmp(p_oldargnames[i], p_names[i]) != 0)
>    elog(ERROR, ...

I testing visible interface from outside.

from outside the following functions are same:

foo1(in a1, in a2, in a3, out a1, out a2, out a3)
foo2(in a1, out a1, in a2, out a2, in a3, out a3)

so the used test is immune to this change.

>
> I'm oversimplifying, of course, but I don't understand why you need both
> i and j. Also, there is some unnecessarily verbose code like:
>
>  if (p_modes == NULL || (p_modes != NULL ...
>

when p_modes is null,then all arguments are input. So input parameter
is when p_modes is null (all parameters are input) or is "in",
"inout", "variadic".

> In func_get_detail(), there is:
>
>  /* shouldn't happen, FuncnameGetCandidates messed up */
>  if (best_candidate->ndargs > pform->pronargdefaults)
>    elog(ERROR, "not enough default arguments");
>

best_candidate->ndargs ~ use n default values, it have to be less or
equal than declared defaults in pgproc.

> Why is it only an error if ndargs is greater? Shouldn't they be equal?
>

ndargs == pronargdefaults is correct - it means, use all declared
defaults. But, raise exception when you would to use more defaults
than is declared.

>  /*
>   * Actually only first nargs field of best_candidate->args is valid,
>   * Now, we have to refresh last ndargs items.
>   */
>
> Can you explain the above comment?
>

this is not good formulation. It means, so in this moment we processed
nargs fields, and we have to process others ndargs fields. But i named
or mixed notation, the processed fields should not be first nargs
fields (like positional notation). There should be a gap, that are
filled by defaults. Gaps are identified via bitmap created on cycle
above. In this cycle is processed positional parameters (with
increasing i) and named parameters. Because positional parameters have
to be front of named parameters, then we don't need increase i when
process named p.,

> Please review Brendan and my patches and combine them with your patch.
>

yes, I'll go on this evening, thank you.

Pavel

> Regards,
>        Jeff Davis
>