Re: Copy/Paste table(s) functions - git context patch

On Thu, 2011-10-13 at 06:22 +0200, Vladimir Kokovic wrote:
> Hi,
>
> I finally found the time to make a new version !
>
> > * When I copy a table, and then paste it, I can not paste it once more.
> > Why is that?
> Fixed
>

OK.

> > * If I paste a table and there is an error (for example, the table
> > Already exists), I can not paste it once more.
> Fixed
>

OK.

> > * If I paste a table, and click on the dialog asking me if I'm sure I
> > Want to copy the table here, I can not paste it once more.
> Fixed
>
> > * If I paste a table to the same schema I copied it from, and if I click
> > Cancel on the dialog asking me for a suffix, it keeps asking me for a
> > Suffix (wether I click OK or cancel, the behavior)
> Fixed
>

It still does. I copy table t1 in schema public, and paste it in schema
s1. If a table named t1 already exists in schema s1, I can click OK or
Cancel, if there's nothing in the textbox, it keeps asking me about a
prefix.

> > * If I paste a table to another schema and the copied it from, and that
> > The same table name exists in that schema, it asks me for a suffix. If
> > I click cancel, it still tries to paste the table (it should not, I
> > Clicked cancel).
> Fixed
>

Nope, I still have the issue.

> > * If I paste a table to another schema and the copied it from, and that
> > The same table name exists in that schema, it asks me for a suffix. If
> > I delete the suffix by default and click OK, it still tries to paste
> > The table (it should, it's obvious it would fail).

Still fails.

> > * If I paste a table and the table already exists, it asks me for a
> > Suffix. If I put ". Whatever", it fails because the table name is not
> > Quoted (I have a foo.bar.whatever CREATE TABLE, CREATE instead of
> > TABLE foo. "Bar.whatever")
> Fixed
>

OK.

> > * By the way, when it fails to create the table, it should not try to
> > Insert into it right after.
> Yes
>

OK.

> > * With drag and drop, the cursor changes too late ... and you should
> > Change the cursor, it looks non-standard).
> I do not know how to change the treectrl DND UI !
>
> > * I do not like at all the "Copy tables from the list ..." dialog. It
> > Should not display tables from every servers.
> This was the main reason why I started CopyPasteTables !
> I am DBA for multiple servers in different locations of a company.
>

Well, I don't pretend we shouldn't offer the possibility to copy a
table, and paste it into another database. You can already do this with
the "Copy a table" item. I don't think we should keep the dialog. All
actions done from a right click happens in one database, and I don't
think we should change this behaviour.

I also don't think we should keep the icon in the toolbar.

> > There are unexplained colors (green, blue, and red).
> > Checkboxes are not usable on servers and databases.
> > Unless you explain why, I think you should get rid of it, not fix it.
> Now, only schema and tables are colored.
>

That's not an explanation. Why do you use colors?

> > On the code itself:
> > * CtlCheckTreeView.cpp is a widget, that should not have any behavior
> > Specific to one window. At least, not in the way this is done now.
> I think the changes that were made do not affect the initial design
> and functionality.
>

Yeah, but that's not the point. Your patch adds code in it that is
specific to your dialog (for example, "frmCopyTables *frm =
(frmCopyTables *)copytables;"). I don't think this is a good design.
Maybe someone will need something like the new code in this file in the
future, and it should be able to do it without having to fuss with a
frmCopyTable object.

> > * To check if a table exists, you look into pg_tables. You should better
> > Browse the treeview.
> > * BTW, you already have the function to do it (findTable)
> Yes, but I need the function for a server which is not connected
> (without using a browser refresh).
>

I don't feel comfortable with this.

If I click in a database in the copy window, it opens the connection to
this database in the browser. So, if the user deconnects in the browser,
the copy window still shows the connection as alive, but we cannot see
any tables. It's really weird. It will confuse users.

> > * Messages should be in uppercase (Copied FROM ...).
> Fixed
>
> > I also do not understand this code in pgColumn.cpp:
> > @ @ @ @ -327.7 +328.8 PgColumn wxString:: GetDefinition ()
>
> > If ((sql == wxT ("integer") | | sql == wxT ("BIGINT") | |
> > Sql == wxT ("pg_catalog.integer") | | sql ==
> > WxT ("pg_catalog.bigint"))
> > - & & (GetDefault () == seqDefault1 | | GetDefault () ==
> > SeqDefault2))
> > + & & ((& & Sequence9! GetSerialSequence (). IsEmpty ()) | |
> > + (! Sequence9 & & (GetDefault () == seqDefault1 | | GetDefault () ==
> > SeqDefault2))))
> > {
> > If (sql.Right (6) == wxT ("BIGINT"))
> > Sql = wxT ("bigserial");
> Without this, serial fields are not displayed as serial type for versions 9.x
>

That doesn't explain the code. Or IOW, I still don't understand the
code.

> > Another issue: I launch a copy and disconnect during copy. After I
> > Reconnect, it crashes. But the table is there with its 6 million rows.
> > And sometimes I do not need to reconnect: it crashes during the copy.
> Fixed
>

I still have crashes from time to time while doing a copy.

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com