Pavel Stehule <pavel.steh...@gmail.com> writes: > there is final Wojciech's patch
I looked this over a little bit, but I don't see an answer to the question I put on the commitfest entry: why is this being done in plpgsql, and not somewhere in the core code? The core has also got the concept of %TYPE, and could stand to have support for attaching [] notation to that. I'm not entirely sure if anything could be shared, but it would sure be worth looking at. I've wished for some time that %TYPE not be handled directly in read_datatype() at all, but rather in the core parse_datatype() function. This would require passing some sort of callback function to parse_datatype() to let it know what variables can be referenced in %TYPE, but we have parser callback functions just like that already. One benefit we could get that way is that the core meaning of %TYPE (type of a table field name) would still be available in plpgsql, if the name didn't match any declared plpgsql variable. However, assuming that we're sticking to just changing plpgsql for the moment ... I cannot escape the feeling that this is a large patch with a small patch struggling to get out. It should not require 500 net new lines of code to provide this functionality, when all we're doing is looking up the array type whose element type is the type the existing code can derive. I would have expected to see the grammar passing one extra flag to plpgsql_parse_cwordtype and plpgsql_parse_cwordrowtype routines that were little changed except for the addition of a step to find the array type. Another complaint is that the grammar changes assume that the first token of decl_datatype must be T_WORD or T_CWORD, which means that it fails for cases such as unreserved plpgsql keywords. This example should work, and does work in 9.1: create domain hint as int; create or replace function foo() returns void as $$ declare x hint; begin end $$ language plpgsql; but fails with this patch because "hint" is returned by the lexer as K_HINT not T_WORD. You might be able to get around that by adding a production with unreserved_keyword --- but I'm unsure that that would cover every case that worked before, and in any case I do not see the point of changing the grammar production at all. It gains almost nothing to have Bison parse the [] rather than doing it with C code in read_datatype(). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers