Re: Issues for named/mixed function notation patch

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Brendan Jurd <direvus(at)gmail(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bernd Helmle <mailings(at)oopsware(dot)de>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Issues for named/mixed function notation patch
Date: 2009-10-02 08:29:48
Message-ID: 1254472188.31660.68.camel@jdavis
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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.


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)

if (strcmp(p_oldargnames[i], p_names[i]) != 0)
elog(ERROR, ...

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 ...

In func_get_detail(), there is:

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

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

* 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?

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

Jeff Davis

Attachment Content-Type Size
named-mixed.patch text/x-patch 9.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2009-10-02 09:13:43 Re: Hot Standby on git
Previous Message Heikki Linnakangas 2009-10-02 08:26:18 Re: Hot Standby on git