Hi

On Tue, Feb 23, 2016 at 5:24 PM, Neel Patel <neel.pa...@enterprisedb.com>
wrote:

> Hi,
>
> Please find attached patch file for the following three nodes.
>
>    - Foreign Data Wrappers
>    - Foreign Servers
>    - User Mapping
>
> With this patch, we have implemented "Dependencies", "Dependent" tab and
> proper comments has been added.
>
> Do review it and let us know for any comments.
>

This seems to be nearly ready now. Some feedback below - note that a couple
of the issues may be caused by infrastructure code, in which case please do
fix them, but feel free to put them in a different patch:

- When adding a User Mapping, you cannot specify an empty option, e.g. a
blank password.

- When granting USAGE to PUBLIC on a foreign server, the WITH GRANT OPTION
is set, despite not being selected. e.g. adding "U" permissions for
"public" results in:

GRANT ALL ON FOREIGN SERVER redis_server TO public;
GRANT ALL ON FOREIGN SERVER redis_server TO public WITH GRANT OPTION;

- Why all the extra blank lines in this SQL? I would expect to see only 2,
between the ALTER/COMMENT/GRANT sections.

====
ALTER SERVER redis_server
    VERSION 'Fooo';

COMMENT ON SERVER redis_server
    IS 'Redis Server x';






GRANT ALL ON FOREIGN SERVER redis_server TO public;
GRANT ALL ON FOREIGN SERVER redis_server TO public WITH GRANT OPTION;


====

- Error messages from the server are not displayed properly - e.g:

invalid option "server"
HINT:  Valid options in this context are: <none>

Is displayed as:

invalid option "server" HINT: Valid options in this context are:

(Notice the lack of "<none>")

- A foreign server object is not listed as being dependent upon the FDW
it's defined as using (but, pgAdmin 3 gets this wrong too).

- The "Options" tabs have a hint message of "Please enter some value!",
whether the focus is in the Option or Value field. Please fix to display:

Please enter an option name.

or

Please enter a value.

Based on the current focus. Note also that there should not be an
exclamation mark.

Thanks!

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Reply via email to