On Mon, Jan 23, 2012 at 7:13 PM, Matthew Draper <matthew@trebex.net> wrote:
> On 19/01/12 20:28, Hitoshi Harada wrote:
>>> (Now it occurred to me that forgetting the #include parse_func.h might
>>> hit this breakage..., so I'll fix it here and continue to test, but if
>>> you'll fix it yourself, let me know)
>>
>> I fixed it here and it now works with my environment.
>
> Well spotted; that's exactly what I'd done. :/
>
>
>> The regression tests pass, the feature seems working as aimed, but it
>> seems to me that it needs more test cases and documentation. For the
>> tests, I believe at least we need "ambiguous" case given upthread, so
>> that we can ensure to keep compatibility. For the document, it should
>> describe the name resolution rule, as stated in the patch comment.
>
> Attached are a new pair of patches, fixing the missing include (and the
> other warning), plus addressing the above.
>
> I'm still not sure whether to just revise (almost) all the SQL function
> examples to use parameter names, and declare them the "right" choice; as
> it's currently written, named parameters still seem rather second-class.
>
Agreed. The patch seems ok, except an example I've just found.
db1=# create function t(a int, t t) returns int as $$ select t.a $$
language sql;
CREATE FUNCTION
db1=# select t(0, row(1, 2)::t);t
---1
(1 row)
Should we throw an error in such ambiguity? Or did you make it happen
intentionally? If latter, we should also mention the rule in the
manual.
Other than that, the patch looks good to me.
Thanks,
--
Hitoshi Harada
