Re: [pgAdmin4][Patch]: Foreign Data Wrapper

Hi

On Tue, Feb 23, 2016 at 5:24 PM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)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