Re: First test of the Database Designer

On Sat, Jun 11, 2011 at 3:06 AM, Guillaume Lelarge
<guillaume(at)lelarge(dot)info>wrote:

> Hi,
>
> I tried Luis's repo this morning, and here are the things I've found
> that still need some work before reaching the first goal:
>
> * I have an assertion each time I launch pgAdmin2 from your repo:
> Debug: ./src/common/cmdline.cpp(446): assert "i != (-1)" failed in
> Found(): unknown switch
>

Well this error happens when this source code is executed:
wxCmdLineParser cmdParser(cmdLineDesc, argc, argv);
if (cmdParser.Parse() != 0)
return false;

if ((cmdParser.Found(wxT("q")) && cmdParser.Found(wxT("qc"))) ||
(cmdParser.Found(wxT("S")) && cmdParser.Found(wxT("Sc"))) ||
(cmdParser.Found(wxT("d")) && cmdParser.Found(wxT("dc"))))
....
here an assertion is raised, I always got this assert and I just ignored it,
because I believe isn't related to my source code and this is probably a
side effect from that xiul/gsoc2010 merge with gleu/ticket and the merge
with pgadmin3 repo at github, and in fact I don't merge mi repo with
original pgadmin3 from start of my coding at gsoc, because that process make
me loss a lot of time with conflicts in parts of pgAdmin source code that I
didn't modify in any way, in fact my source code is very isolate right now.

> * I should be able to right click on an empty schema to create a table
>

Good idea, I don't thought on this because I'm not focused on usability
right now, just I want everything works as good as possible with so little
time to do many things.

> * What is the "short name" of a table?
>

That is part of the automatic fk naming subsystem, I explain here:

Table A [source of fk] ---------< Table B [destination of fk]

- At beginning of fk, all pk or uk column from source table are added to
destination table.
- Each of this columns needs a name, an automatic one at the beginning.
- To create this name there are two options:
source table name + _ + original column name
or
source short table name (if exists) + _ + original column name

- But if user choose to use their own name he can do it, and then auto
naming subsystem for this column is off, later if he user likes it, he is
able to turn it on by right click on that column.

- Constraint name from fk when autonaming is used, is taken from short name
too.

In little words this is just a way of helping user to have and good
automatic naming convention if he likes to use, but if he doesn't like he
just edit column name and constraint name, and auto naming subsystem turn
off.

* UI of new table dialog:
> * the "Generate Short Name" button should not be there
> * focus should be on the first textbox
> * OK should be the default button (iow, hitting enter should trigger
> it)
> * Cancel should behave like any cancel button (iow, hitting esc should
> trigger it)
>

Well again, I'm not focused on usability right now I just wanted everything
works fine, but in my testing:

- Focus is on first texbox at least on my development environment.
- Enter at first textbox failed, right, I'll fixed it.
- Right now pressing esc trigger cancel at least on my development
environment.
- What should I do with generate short name? change position or delete it?

> * UI of relationships
> * it should be possible to create relationships between existing
> columns
>

This is a new feature that requires changing the subsystem of relationship
creation, and I didn't do at first goal as I told you at beginning only
fixing of old source code and not adding new features, but if you want it
I'll be glad of doing it but this probably will affect my proposal schedule
and I need your confirmation. Should I do this feature?

> * adding a char(n) type doesn't ask for its length
>

I'll fix it.

> * moreover, the list of datatype seems weird to me... you should list
> all available types and when the user choose a type with a length
> and/or a precision, it should ask them (it's already like that for
> varchar, but not for char, numeric, etc)
>

This list is just as we agree at last year, some important datatypes and
list with all when user need more.
What should I modify the list? or How do you believe it should be done?

I'll fix precision of types.

* I can't use the del key when I select an object (table and
> relationship)
>

Keyboard shorcuts is a new feature (need modification of framework, and
figures is not as easy as adding a new event to some class), I didn't do
this as I explain you above. Again, I'll be glad of doing it but this
probably will affect my proposal schedule and I need your confirmation.
Should I do this feature?

> * if I create a relationship between t1 and t2, and then between t2 and
> t1, once you start moving one of these tables, the two relationships
> appear as just one
>

As I explain you right now angle lines don't allow me to have this kind of
realtionships and a new kind of lines (orthogonal lines) should be
implemented for this reason. Again, I'll be glad of doing it but this
probably will affect my proposal schedule and I need your confirmation.
Should I do this feature?

> * if the user asks for a new design, the sql textbox should be empty too
>

I'll fix that.

> * UI of relationship contextual menu
> * you should only keep one item of "1:1" and "1:M", it's one or the
> other, so you must not offer both with a checkbox
>
I'll fix that.

> * rather than menu items "Foreign Key from Primary Key", and "Foreign
> Key from Unique Key", you should have a sub menu entitled "Foreign
> Key From", and, for the items in the submenu, you should have
> "Primary Key", and "Unique Constraint "+name of each unique
> constraint...

I'll implement this.

this way, the user knows quickly which constraint is
> used, and could change it quickly... and you have one less dialog
>
> * in a table box, there is a "Indexes" item never used. It should
> disappeared.
>

You're right, index part right now are out of scope but in a future them
should be coded.

> * I can move a table outside of the drawing area, and I can never get it
> back
>

That's right but only for bottom and right sides, because I would implement
a function that auto resize canva if user go in any of this direction, but
right now that function was harder to code that I believes and is no
implemented, because this I'll disallow moving figures to this sides.

> Even if it's a long list of complaints, I didn't really hit a bug like I
> did in the previous GSoC, so that's quite good. I could even draw my
> test schema almost completely (almost because Luis's repo doesn't
> support yet self referencing table).

Self foreign key is a new feature that needs orthogonal lines features to be
implemented.

> That test schema always ended with
> a crash of pgAdmin. That doesn't happen any more, and it's good. But
> it's not good enough to be commitable
>

I agree with you at this point is not commitable becuase last patch will be
the really commitable one, but as you explain me, I'll make this patch
commitable.

> So, Luis, you need to work on this list of items, and fix them. Once
> that's done, I'll get another look at your code.
>
> Thanks.
>

Regards, Luis.