On 3 January 2013 13:49, Ali Dar <ali.munir.dar@gmail.com> wrote:
> Find attached an initial patch for ALTER RENAME RULE feature. Please note
> that it does not have any documentation yet.
>
Hi,
I just got round to looking at this. All-in-all it looks OK. I just
have a few more review comments, in addition to Stephen's comment
about renaming SELECT rules...
This compiler warning should be fixed with another #include:
alter.c:107:4: warning: implicit declaration of function ‘RenameRewriteRule’
In gram.y, I think you can just use qualified_name instead of
makeRangeVarFromAnyName().
In RenameRewriteRule(), I think it's worth doing a check to ensure
that the relation actually is a table or a view (you might have some
other relation kind at that point in the code). If the user
accidentally types the name of an index, say, instead of a table, then
it is better to throw an error saying "xxx is not a table or a view"
rather than reporting that the rule doesn't exist.
I think this could probably use some simple regression tests to test
both the success and failure cases.
It would be nice to extend psql tab completion to support this too,
although perhaps that could be done as a separate patch.
Don't forget the docs!
Regards,
Dean