Re: pgAdmin for Greenplum database?

On Thu, Feb 19, 2009 at 9:29 PM, Chuck McDevitt <cmcdevitt(at)greenplum(dot)com> wrote:
> Revised patch. Here is the diff from 7597 on existing files. Next e-mail will be the new files.
>
> P.s. There is some extra stuff in the patch, because I didn't realize that there were a significant number of "tab" characters in the svn repository. I did an "untabify" on the files, so converted tabs to 4 spaces. This caused some extra differences with files that had tabs in the svn repository.

Hi Chuck,

The patch looks pretty good to me - just a few issues/queries below.
I've only eyeballed it and built it so far though - I'm in the process
of downloading Greenplum so I can test more thoroughly, though it's a
slow download at the moment (btw; you might want to have you're
website tell the user a password is being emailed - it looks like
nothing has happened when you first register).

- There is a block of commented out code in dlgExtTable which looks
like it was copied and pasted from dlgTable. It almost looks like you
started to write a more comprehensive GUI but then decided to just use
a textbox instead. What would be seen in the textbox? We would
normally only provide a UI like that for 'free form' properties such
as a function body, not for 'rigid form' properties such as those
defining a table.

- In pgPartitions.cpp, GetSQL should return the ALTER TABLE statement
that might be used to create the partition, not an untranslated
explanation of how the user should create the partition. The reverse
engineered SQL we display on the main window is intended as a teaching
tool as well as a convenience for those that like to use the SQL.

- Can you please rename pgPartition/pgExtTable/pgResQueue to gpXXX? We
use edbXXX for EnterpriseDB-specific objects and should follow that
convention for Greenplum.

- There are no dialogues for displaying/editing/creating partitions or
resource queues. Is there a reason for this? Are those items a user
will never create but must be able to browse? As a rule (to which
there are just two closely related exceptions) we want to add complete
support for object types from the outset.

- pgPartition.h (or gpPartition.h) is missing from precomp.h.

- The test in isGpApp() is pretty flimsy. Can you call pg_dump --help
and look for the --gp-syntax option in the help output?

- There's a change in embed-xrc.bat that probably shouldn't be in the patch!

- On frmOptions, the labels 'Greenplum help path' and 'Greenplum bin
path' don't fit, and are likely to be even longer in some
translations. We need to use 'GP help path' and 'GP bin path' I guess.

- The text on the Display tab on frmOptions should be changed from
'ResQueues/resource queues' to 'Resource queues'. The dual naming is
only used for roles where login roles == users and group roles ==
groups in older versions of PG.

- Please document the new options in docs/en_US/options-tab1.html. A
screenshot would help as well, though I can do that if you don't have
XP.

Regards, Dave.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com