Thread: Support of INHERIT in existing tables (8.2+ releases)

Support of INHERIT in existing tables (8.2+ releases)

From
Guillaume Lelarge
Date:
Hi,

I don't know if you have read this page
(http://www.postgresonline.com/journal/index.php?/archives/59-How-to-Inherit,-Unherit-and-Merge-Inherit.html)
but it complains on the lack of inherit support for existing tables.

So, here is a patch to add support on adding inherited tables on already
existing tables. The feature is available since PostgreSQL 8.2 release.

Tested on Linux. It works fine there. I didn't check Win32 platform
because I didn't change any xrc files :) Didn't check on Mac OS X either
cause I don't have one (btw, I'm following ebay's Mac Mini list to get
one soon).

Regards.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: pgadmin/include/schema/pgTable.h
===================================================================
--- pgadmin/include/schema/pgTable.h    (revision 7393)
+++ pgadmin/include/schema/pgTable.h    (working copy)
@@ -72,6 +72,7 @@
     long GetInheritedTableCount() { if (inheritedTableCount < 0) UpdateInheritance(); return inheritedTableCount; }
     wxString GetInheritedTables() { GetInheritedTableCount(); return inheritedTables; }
     wxString GetQuotedInheritedTables() { GetInheritedTableCount(); return quotedInheritedTables; }
+    wxArrayString GetInheritedTablesOidList() { GetInheritedTableCount(); return inheritedTablesOidList; }
     wxArrayString GetQuotedInheritedTablesList() { GetInheritedTableCount(); return quotedInheritedTablesList; }
     wxString GetCoveringIndex(ctlTree *browser, const wxString &collist);
     pgCollection *GetColumnCollection(ctlTree *browser);
@@ -124,7 +125,7 @@
     long inheritedTableCount;
     wxString quotedInheritedTables, inheritedTables, primaryKey, quotedPrimaryKey,
         primaryKeyName, primaryKeyColNumbers, tablespace;
-    wxArrayString quotedInheritedTablesList;
+    wxArrayString quotedInheritedTablesList, inheritedTablesOidList;
     slSet *replicationSet;
     OID tablespaceOid;
 };
Index: pgadmin/include/dlg/dlgTable.h
===================================================================
--- pgadmin/include/dlg/dlgTable.h    (revision 7393)
+++ pgadmin/include/dlg/dlgTable.h    (working copy)
@@ -56,7 +56,7 @@
     wxString GetItemConstraintType(ctlListView *list, long pos);
     bool hasPK;

-    wxArrayString previousColumns, previousConstraints;
+    wxArrayString previousColumns, previousConstraints, previousTables;
     wxArrayString tableOids, inheritedTableOids;
     wxTreeItemId columnsItem, constraintsItem;

Index: pgadmin/schema/pgTable.cpp
===================================================================
--- pgadmin/schema/pgTable.cpp    (revision 7393)
+++ pgadmin/schema/pgTable.cpp    (working copy)
@@ -511,7 +511,7 @@
 {
     // not checked so far
     pgSet *props=ExecuteSet(
-        wxT("SELECT c.relname , nspname\n")
+        wxT("SELECT c.oid, c.relname , nspname\n")
         wxT("  FROM pg_inherits i\n")
         wxT("  JOIN pg_class c ON c.oid = i.inhparent\n")
         wxT("  JOIN pg_namespace n ON n.oid=c.relnamespace\n")
@@ -533,6 +533,7 @@
                     + qtIdent(props->GetVal(wxT("relname")));
             quotedInheritedTablesList.Add(GetQuotedSchemaPrefix(props->GetVal(wxT("nspname")))
                     + qtIdent(props->GetVal(wxT("relname"))));
+            inheritedTablesOidList.Add(props->GetVal(wxT("oid")));
             props->MoveNext();
             inheritedTableCount++;
         }
Index: pgadmin/dlg/dlgTable.cpp
===================================================================
--- pgadmin/dlg/dlgTable.cpp    (revision 7393)
+++ pgadmin/dlg/dlgTable.cpp    (working copy)
@@ -157,14 +157,19 @@
         if (table->GetTablespaceOid() != 0)
             cbTablespace->SetKey(table->GetTablespaceOid());

+        inheritedTableOids=table->GetInheritedTablesOidList();
+
         wxArrayString qitl=table->GetQuotedInheritedTablesList();
         size_t i;
         for (i=0 ; i < qitl.GetCount() ; i++)
+        {
+            previousTables.Add(qitl.Item(i));
             lbTables->Append(qitl.Item(i));
+        }

-        btnAddTable->Disable();
-        lbTables->Disable();
-        cbTables->Disable();
+        btnAddTable->Enable(connection->BackendMinimumVersion(8, 2));
+        lbTables->Enable(connection->BackendMinimumVersion(8, 2));
+        cbTables->Enable(connection->BackendMinimumVersion(8, 2));
         chkHasOids->Enable(table->GetHasOids() && connection->BackendMinimumVersion(8, 0));
         cbTablespace->Enable(connection->BackendMinimumVersion(7, 5));

@@ -278,11 +283,30 @@
         // create mode
         btnChangeCol->Hide();

+        // Add the default tablespace
+        cbTablespace->Insert(_("<default tablespace>"), 0, (void *)0);
+        cbTablespace->SetSelection(0);
+    }
+
+    if (connection->BackendMinimumVersion(8,2) || !table)
+    {
         wxString systemRestriction;
         if (!settings->GetShowSystemObjects())
         systemRestriction =
             wxT("   AND ") + connection->SystemNamespaceRestriction(wxT("n.nspname"));
-
+
+        if (table)
+        {
+            wxString oids = table->GetOidStr();
+            int i;
+            for (i=0 ; i < (int)inheritedTableOids.GetCount() ; i++)
+            {
+                oids += wxT(", ") + inheritedTableOids.Item(i);
+            }
+            if (oids.Length() > 0)
+                systemRestriction += wxT(" AND c.oid NOT IN (") + oids + wxT(")");
+        }
+
         pgSet *set=connection->ExecuteSet(
             wxT("SELECT c.oid, c.relname , nspname\n")
             wxT("  FROM pg_class c\n")
@@ -302,10 +326,6 @@
             }
             delete set;
         }
-
-        // Add the default tablespace
-        cbTablespace->Insert(_("<default tablespace>"), 0, (void *)0);
-        cbTablespace->SetSelection(0);
     }

     FillConstraint();
@@ -511,7 +531,7 @@
         wxArrayString tmpDef=previousColumns;
         wxString tmpsql;

-        // Build a tmeporary list of ADD COLUMNs, and fixup the list to remove
+        // Build a temporary list of ADD COLUMNs, and fixup the list to remove
         for (pos=0; pos < lstColumns->GetItemCount() ; pos++)
         {
             definition = lstColumns->GetText(pos, 3);
@@ -555,10 +575,34 @@
         AppendNameChange(sql);
         AppendOwnerChange(sql, wxT("TABLE ") + tabname);

+        tmpDef=previousTables;
+        tmpsql.Empty();
+
+        // Build a temporary list of INHERIT tables, and fixup the list to remove
+        for (pos = 0 ; pos < (int)lbTables->GetCount() ; pos++)
+        {
+            definition = lbTables->GetString(pos);
+            index = tmpDef.Index(definition);
+            if (index < 0)
+                tmpsql += wxT("ALTER TABLE ") + table->GetQuotedFullIdentifier()
+                    +  wxT(" INHERIT ") + qtIdent(definition) + wxT(";\n");
+            else
+                tmpDef.RemoveAt(index);
+        }
+
+        for (index = 0 ; index < (int)tmpDef.GetCount() ; index++)
+        {
+            definition = tmpDef.Item(index);
+            sql += wxT("ALTER TABLE ") + table->GetQuotedFullIdentifier()
+                +  wxT(" NO INHERIT ") + qtIdent(definition) + wxT(";\n");
+        }
+        // Add the ADD COLUMNs...
+        sql += tmpsql;
+
         tmpDef=previousConstraints;
         tmpsql.Empty();

-        // Build a tmeporary list of ADD CONSTRAINTs, and fixup the list to remove
+        // Build a temporary list of ADD CONSTRAINTs, and fixup the list to remove
         for (pos=0; pos < lstConstraints->GetItemCount() ; pos++)
         {
             wxString conname= qtIdent(lstConstraints->GetItemText(pos));

Re: Support of INHERIT in existing tables (8.2+ releases)

From
"Dave Page"
Date:
On Fri, Jul 11, 2008 at 11:30 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
>
>  (btw, I'm following ebay's Mac Mini list to get one soon).

:-)

Make sure you don't get anything older than OS X 10.4 (Tiger). No real
need to spend extra to get Leopard though (unless you want to).


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

Re: Support of INHERIT in existing tables (8.2+ releases)

From
"Dave Page"
Date:
On Fri, Jul 11, 2008 at 11:30 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

> So, here is a patch to add support on adding inherited tables on already
> existing tables. The feature is available since PostgreSQL 8.2 release.
>
> Tested on Linux. It works fine there. I didn't check Win32 platform because
> I didn't change any xrc files :) Didn't check on Mac OS X either cause I
> don't have one (btw, I'm following ebay's Mac Mini list to get one soon).

Hmm, testing on 8.3.3 on Windows, I created parent and child table,
then tried to add parent to child's inherit list separately and got:

ERROR:  syntax error at or near "from"
LINE 1: ALTER TABLE child ADD COLUMN id Inherited from table ...

The actual SQL generated is:

ALTER TABLE child ADD COLUMN id Inherited from table parent;
ALTER TABLE child INHERIT parent;

Shouldn't it just be doing:

ALTER TABLE child INHERIT parent;


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

Re: Support of INHERIT in existing tables (8.2+ releases)

From
Guillaume Lelarge
Date:
Dave Page a écrit :
> On Fri, Jul 11, 2008 at 11:30 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>
>> So, here is a patch to add support on adding inherited tables on already
>> existing tables. The feature is available since PostgreSQL 8.2 release.
>>
>> Tested on Linux. It works fine there. I didn't check Win32 platform because
>> I didn't change any xrc files :) Didn't check on Mac OS X either cause I
>> don't have one (btw, I'm following ebay's Mac Mini list to get one soon).
>
> Hmm, testing on 8.3.3 on Windows, I created parent and child table,
> then tried to add parent to child's inherit list separately and got:
>
> ERROR:  syntax error at or near "from"
> LINE 1: ALTER TABLE child ADD COLUMN id Inherited from table ...
>
> The actual SQL generated is:
>
> ALTER TABLE child ADD COLUMN id Inherited from table parent;
> ALTER TABLE child INHERIT parent;
>
> Shouldn't it just be doing:
>
> ALTER TABLE child INHERIT parent;
>

Yeah, you're right. I had this issue during my tests and I'm sure I
fixed it. Don't know why it isn't in the patch I sent. Sorry about this.
I can't check right now but I will take care of this issue this afternoon.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com

Re: Support of INHERIT in existing tables (8.2+ releases)

From
Guillaume Lelarge
Date:
Guillaume Lelarge a écrit :
> Dave Page a écrit :
>> On Fri, Jul 11, 2008 at 11:30 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>>
>>> So, here is a patch to add support on adding inherited tables on already
>>> existing tables. The feature is available since PostgreSQL 8.2 release.
>>>
>>> Tested on Linux. It works fine there. I didn't check Win32 platform
>>> because
>>> I didn't change any xrc files :) Didn't check on Mac OS X either cause I
>>> don't have one (btw, I'm following ebay's Mac Mini list to get one
>>> soon).
>>
>> Hmm, testing on 8.3.3 on Windows, I created parent and child table,
>> then tried to add parent to child's inherit list separately and got:
>>
>> ERROR:  syntax error at or near "from"
>> LINE 1: ALTER TABLE child ADD COLUMN id Inherited from table ...
>>
>> The actual SQL generated is:
>>
>> ALTER TABLE child ADD COLUMN id Inherited from table parent;
>> ALTER TABLE child INHERIT parent;
>>
>> Shouldn't it just be doing:
>>
>> ALTER TABLE child INHERIT parent;
>>
>
> Yeah, you're right. I had this issue during my tests and I'm sure I
> fixed it. Don't know why it isn't in the patch I sent. Sorry about this.
> I can't check right now but I will take care of this issue this afternoon.
>

This patch fixes the issue you found.

I have one question about inherited columns display in the table
properties dialog. There's a "Inherited from" column on the Columns tab.
It shows the word "inherited" when the column is inherited. But I
initialy thought it would show the table name of the inherited column.
Is it a bug? (btw, I checked in 1.8.4, it works the same (or it bugs the
same :) ))


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: pgadmin/include/schema/pgTable.h
===================================================================
--- pgadmin/include/schema/pgTable.h    (revision 7393)
+++ pgadmin/include/schema/pgTable.h    (working copy)
@@ -72,6 +72,7 @@
     long GetInheritedTableCount() { if (inheritedTableCount < 0) UpdateInheritance(); return inheritedTableCount; }
     wxString GetInheritedTables() { GetInheritedTableCount(); return inheritedTables; }
     wxString GetQuotedInheritedTables() { GetInheritedTableCount(); return quotedInheritedTables; }
+    wxArrayString GetInheritedTablesOidList() { GetInheritedTableCount(); return inheritedTablesOidList; }
     wxArrayString GetQuotedInheritedTablesList() { GetInheritedTableCount(); return quotedInheritedTablesList; }
     wxString GetCoveringIndex(ctlTree *browser, const wxString &collist);
     pgCollection *GetColumnCollection(ctlTree *browser);
@@ -124,7 +125,7 @@
     long inheritedTableCount;
     wxString quotedInheritedTables, inheritedTables, primaryKey, quotedPrimaryKey,
         primaryKeyName, primaryKeyColNumbers, tablespace;
-    wxArrayString quotedInheritedTablesList;
+    wxArrayString quotedInheritedTablesList, inheritedTablesOidList;
     slSet *replicationSet;
     OID tablespaceOid;
 };
Index: pgadmin/include/dlg/dlgTable.h
===================================================================
--- pgadmin/include/dlg/dlgTable.h    (revision 7393)
+++ pgadmin/include/dlg/dlgTable.h    (working copy)
@@ -56,7 +56,7 @@
     wxString GetItemConstraintType(ctlListView *list, long pos);
     bool hasPK;

-    wxArrayString previousColumns, previousConstraints;
+    wxArrayString previousColumns, previousConstraints, previousTables;
     wxArrayString tableOids, inheritedTableOids;
     wxTreeItemId columnsItem, constraintsItem;

Index: pgadmin/schema/pgTable.cpp
===================================================================
--- pgadmin/schema/pgTable.cpp    (revision 7393)
+++ pgadmin/schema/pgTable.cpp    (working copy)
@@ -511,7 +511,7 @@
 {
     // not checked so far
     pgSet *props=ExecuteSet(
-        wxT("SELECT c.relname , nspname\n")
+        wxT("SELECT c.oid, c.relname , nspname\n")
         wxT("  FROM pg_inherits i\n")
         wxT("  JOIN pg_class c ON c.oid = i.inhparent\n")
         wxT("  JOIN pg_namespace n ON n.oid=c.relnamespace\n")
@@ -533,6 +533,7 @@
                     + qtIdent(props->GetVal(wxT("relname")));
             quotedInheritedTablesList.Add(GetQuotedSchemaPrefix(props->GetVal(wxT("nspname")))
                     + qtIdent(props->GetVal(wxT("relname"))));
+            inheritedTablesOidList.Add(props->GetVal(wxT("oid")));
             props->MoveNext();
             inheritedTableCount++;
         }
Index: pgadmin/dlg/dlgTable.cpp
===================================================================
--- pgadmin/dlg/dlgTable.cpp    (revision 7393)
+++ pgadmin/dlg/dlgTable.cpp    (working copy)
@@ -157,14 +157,19 @@
         if (table->GetTablespaceOid() != 0)
             cbTablespace->SetKey(table->GetTablespaceOid());

+        inheritedTableOids=table->GetInheritedTablesOidList();
+
         wxArrayString qitl=table->GetQuotedInheritedTablesList();
         size_t i;
         for (i=0 ; i < qitl.GetCount() ; i++)
+        {
+            previousTables.Add(qitl.Item(i));
             lbTables->Append(qitl.Item(i));
+        }

-        btnAddTable->Disable();
-        lbTables->Disable();
-        cbTables->Disable();
+        btnAddTable->Enable(connection->BackendMinimumVersion(8, 2));
+        lbTables->Enable(connection->BackendMinimumVersion(8, 2));
+        cbTables->Enable(connection->BackendMinimumVersion(8, 2));
         chkHasOids->Enable(table->GetHasOids() && connection->BackendMinimumVersion(8, 0));
         cbTablespace->Enable(connection->BackendMinimumVersion(7, 5));

@@ -278,11 +283,30 @@
         // create mode
         btnChangeCol->Hide();

+        // Add the default tablespace
+        cbTablespace->Insert(_("<default tablespace>"), 0, (void *)0);
+        cbTablespace->SetSelection(0);
+    }
+
+    if (connection->BackendMinimumVersion(8,2) || !table)
+    {
         wxString systemRestriction;
         if (!settings->GetShowSystemObjects())
         systemRestriction =
             wxT("   AND ") + connection->SystemNamespaceRestriction(wxT("n.nspname"));
-
+
+        if (table)
+        {
+            wxString oids = table->GetOidStr();
+            int i;
+            for (i=0 ; i < (int)inheritedTableOids.GetCount() ; i++)
+            {
+                oids += wxT(", ") + inheritedTableOids.Item(i);
+            }
+            if (oids.Length() > 0)
+                systemRestriction += wxT(" AND c.oid NOT IN (") + oids + wxT(")");
+        }
+
         pgSet *set=connection->ExecuteSet(
             wxT("SELECT c.oid, c.relname , nspname\n")
             wxT("  FROM pg_class c\n")
@@ -302,10 +326,6 @@
             }
             delete set;
         }
-
-        // Add the default tablespace
-        cbTablespace->Insert(_("<default tablespace>"), 0, (void *)0);
-        cbTablespace->SetSelection(0);
     }

     FillConstraint();
@@ -511,28 +531,31 @@
         wxArrayString tmpDef=previousColumns;
         wxString tmpsql;

-        // Build a tmeporary list of ADD COLUMNs, and fixup the list to remove
+        // Build a temporary list of ADD COLUMNs, and fixup the list to remove
         for (pos=0; pos < lstColumns->GetItemCount() ; pos++)
         {
-            definition = lstColumns->GetText(pos, 3);
-            if (definition.IsEmpty())
+            if (lstColumns->GetText(pos, 2).IsEmpty())
             {
-                definition=qtIdent(lstColumns->GetText(pos)) + wxT(" ") + lstColumns->GetText(pos, 1);
-                index=tmpDef.Index(definition);
-                if (index < 0)
-                    tmpsql += wxT("ALTER TABLE ") + table->GetQuotedFullIdentifier()
-                        +  wxT(" ADD COLUMN ") + definition + wxT(";\n");
-            }
-            else
-            {
-                tmpsql += definition;
-
-                pgColumn *column=(pgColumn*) StrToLong(lstColumns->GetText(pos, 6));
-                if (column)
+                definition = lstColumns->GetText(pos, 3);
+                if (definition.IsEmpty())
                 {
-                    index=tmpDef.Index(column->GetQuotedIdentifier()
-                                + wxT(" ") + column->GetDefinition());
+                    definition=qtIdent(lstColumns->GetText(pos)) + wxT(" ") + lstColumns->GetText(pos, 1);
+                    index=tmpDef.Index(definition);
+                    if (index < 0)
+                        tmpsql += wxT("ALTER TABLE ") + table->GetQuotedFullIdentifier()
+                            +  wxT(" ADD COLUMN ") + definition + wxT(";\n");
                 }
+                else
+                {
+                    tmpsql += definition;
+
+                    pgColumn *column=(pgColumn*) StrToLong(lstColumns->GetText(pos, 6));
+                    if (column)
+                    {
+                        index=tmpDef.Index(column->GetQuotedIdentifier()
+                                    + wxT(" ") + column->GetDefinition());
+                    }
+                }
             }
             if (index >= 0)
                 tmpDef.RemoveAt(index);
@@ -555,10 +578,34 @@
         AppendNameChange(sql);
         AppendOwnerChange(sql, wxT("TABLE ") + tabname);

+        tmpDef=previousTables;
+        tmpsql.Empty();
+
+        // Build a temporary list of INHERIT tables, and fixup the list to remove
+        for (pos = 0 ; pos < (int)lbTables->GetCount() ; pos++)
+        {
+            definition = lbTables->GetString(pos);
+            index = tmpDef.Index(definition);
+            if (index < 0)
+                tmpsql += wxT("ALTER TABLE ") + table->GetQuotedFullIdentifier()
+                    +  wxT(" INHERIT ") + qtIdent(definition) + wxT(";\n");
+            else
+                tmpDef.RemoveAt(index);
+        }
+
+        for (index = 0 ; index < (int)tmpDef.GetCount() ; index++)
+        {
+            definition = tmpDef.Item(index);
+            sql += wxT("ALTER TABLE ") + table->GetQuotedFullIdentifier()
+                +  wxT(" NO INHERIT ") + qtIdent(definition) + wxT(";\n");
+        }
+        // Add the ADD COLUMNs...
+        sql += tmpsql;
+
         tmpDef=previousConstraints;
         tmpsql.Empty();

-        // Build a tmeporary list of ADD CONSTRAINTs, and fixup the list to remove
+        // Build a temporary list of ADD CONSTRAINTs, and fixup the list to remove
         for (pos=0; pos < lstConstraints->GetItemCount() ; pos++)
         {
             wxString conname= qtIdent(lstConstraints->GetItemText(pos));

Re: Support of INHERIT in existing tables (8.2+ releases)

From
"Dave Page"
Date:
On Mon, Jul 14, 2008 at 8:09 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> This patch fixes the issue you found.

Seems to add another though :-(

..\..\src\common\string.cpp(2501): assert "nIndex < m_nCount" failed
in wxArrayString::RemoveAt(): bad index in wxArrayString::Remove

Call stack:
[00] wxGUIAppTraitsBase::ShowAssertDialog
    c:\pgbuild\wxwidgets\src\common\appcmn.cpp:635
[01] ShowAssertDialog
    c:\pgbuild\wxwidgets\src\common\appbase.cpp:836
[02] wxAppConsole::OnAssertFailure
    c:\pgbuild\wxwidgets\src\common\appbase.cpp:445
[03] wxOnAssert
    c:\pgbuild\wxwidgets\src\common\appbase.cpp:713
[04] wxArrayString::RemoveAt
    c:\pgbuild\wxwidgets\src\common\string.cpp:2501
[05] dlgTable::GetSql
    c:\pgbuild\buildtrees\pgadmin3\pgadmin\dlg\dlgtable.cpp:562
[06] dlgTable::CheckChange
    c:\pgbuild\buildtrees\pgadmin3\pgadmin\dlg\dlgtable.cpp:974
[07] dlgTable::OnAddTable
    c:\pgbuild\buildtrees\pgadmin3\pgadmin\dlg\dlgtable.cpp:1018
[08] wxAppConsole::HandleEvent
    c:\pgbuild\wxwidgets\src\common\appbase.cpp:323
[09] wxEvtHandler::ProcessEventIfMatches
    c:\pgbuild\wxwidgets\src\common\event.cpp:1232
[10] wxEventHashTable::HandleEvent
    c:\pgbuild\wxwidgets\src\common\event.cpp:906
[11] wxEvtHandler::ProcessEvent
    c:\pgbuild\wxwidgets\src\common\event.cpp:1292
[12] wxWindowBase::TryParent
    c:\pgbuild\wxwidgets\src\common\wincmn.cpp:2661
[13] wxEvtHandler::ProcessEvent
    c:\pgbuild\wxwidgets\src\common\event.cpp:1306
[14] wxWindowBase::TryParent
    c:\pgbuild\wxwidgets\src\common\wincmn.cpp:2661
[15] wxEvtHandler::ProcessEvent
    c:\pgbuild\wxwidgets\src\common\event.cpp:1306
[16] wxWindowBase::TryParent
    c:\pgbuild\wxwidgets\src\common\wincmn.cpp:2661
[17] wxEvtHandler::ProcessEvent
    c:\pgbuild\wxwidgets\src\common\event.cpp:1306
[18] wxControl::ProcessCommand
    c:\pgbuild\wxwidgets\src\msw\control.cpp:321
[19] wxButton::SendClickEvent
    c:\pgbuild\wxwidgets\src\msw\button.cpp:476
[20] wxButton::MSWCommand
    c:\pgbuild\wxwidgets\src\msw\button.cpp:505

> I have one question about inherited columns display in the table properties
> dialog. There's a "Inherited from" column on the Columns tab. It shows the
> word "inherited" when the column is inherited. But I initialy thought it
> would show the table name of the inherited column. Is it a bug? (btw, I
> checked in 1.8.4, it works the same (or it bugs the same :) ))

Yes, I think you're correct - that does seem like a bug. Can you fix
that while you're in there?

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

Re: Support of INHERIT in existing tables (8.2+ releases)

From
Guillaume Lelarge
Date:
Dave Page a écrit :
> On Mon, Jul 14, 2008 at 8:09 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> This patch fixes the issue you found.
>
> Seems to add another though :-(
>
> ..\..\src\common\string.cpp(2501): assert "nIndex < m_nCount" failed
> in wxArrayString::RemoveAt(): bad index in wxArrayString::Remove
>
> Call stack:
> [00] wxGUIAppTraitsBase::ShowAssertDialog
>     c:\pgbuild\wxwidgets\src\common\appcmn.cpp:635
> [01] ShowAssertDialog
>     c:\pgbuild\wxwidgets\src\common\appbase.cpp:836
> [02] wxAppConsole::OnAssertFailure
>     c:\pgbuild\wxwidgets\src\common\appbase.cpp:445
> [03] wxOnAssert
>     c:\pgbuild\wxwidgets\src\common\appbase.cpp:713
> [04] wxArrayString::RemoveAt
>     c:\pgbuild\wxwidgets\src\common\string.cpp:2501
> [05] dlgTable::GetSql
>     c:\pgbuild\buildtrees\pgadmin3\pgadmin\dlg\dlgtable.cpp:562
> [06] dlgTable::CheckChange
>     c:\pgbuild\buildtrees\pgadmin3\pgadmin\dlg\dlgtable.cpp:974
> [07] dlgTable::OnAddTable
>     c:\pgbuild\buildtrees\pgadmin3\pgadmin\dlg\dlgtable.cpp:1018
> [08] wxAppConsole::HandleEvent
>     c:\pgbuild\wxwidgets\src\common\appbase.cpp:323
> [09] wxEvtHandler::ProcessEventIfMatches
>     c:\pgbuild\wxwidgets\src\common\event.cpp:1232
> [10] wxEventHashTable::HandleEvent
>     c:\pgbuild\wxwidgets\src\common\event.cpp:906
> [11] wxEvtHandler::ProcessEvent
>     c:\pgbuild\wxwidgets\src\common\event.cpp:1292
> [12] wxWindowBase::TryParent
>     c:\pgbuild\wxwidgets\src\common\wincmn.cpp:2661
> [13] wxEvtHandler::ProcessEvent
>     c:\pgbuild\wxwidgets\src\common\event.cpp:1306
> [14] wxWindowBase::TryParent
>     c:\pgbuild\wxwidgets\src\common\wincmn.cpp:2661
> [15] wxEvtHandler::ProcessEvent
>     c:\pgbuild\wxwidgets\src\common\event.cpp:1306
> [16] wxWindowBase::TryParent
>     c:\pgbuild\wxwidgets\src\common\wincmn.cpp:2661
> [17] wxEvtHandler::ProcessEvent
>     c:\pgbuild\wxwidgets\src\common\event.cpp:1306
> [18] wxControl::ProcessCommand
>     c:\pgbuild\wxwidgets\src\msw\control.cpp:321
> [19] wxButton::SendClickEvent
>     c:\pgbuild\wxwidgets\src\msw\button.cpp:476
> [20] wxButton::MSWCommand
>     c:\pgbuild\wxwidgets\src\msw\button.cpp:505
>

Sorry to answer so late, it was a lot of work to get everything working.
Anyways, fixed.

>> I have one question about inherited columns display in the table properties
>> dialog. There's a "Inherited from" column on the Columns tab. It shows the
>> word "inherited" when the column is inherited. But I initialy thought it
>> would show the table name of the inherited column. Is it a bug? (btw, I
>> checked in 1.8.4, it works the same (or it bugs the same :) ))
>
> Yes, I think you're correct - that does seem like a bug. Can you fix
> that while you're in there?
>

Fixed in the same patch. Don't know if you want a backpatch for 1.8?

Complete patch available at:
   http://developer.pgadmin.org/~guillaume/inherit82+_20080810.patch.bz2


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com

Re: Support of INHERIT in existing tables (8.2+ releases)

From
"Dave Page"
Date:
On Sun, Aug 10, 2008 at 6:55 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

> Sorry to answer so late, it was a lot of work to get everything working.
> Anyways, fixed.

No worries. Got the following warning:

./dlg/dlgTable.cpp: In member function 'void
dlgTable::OnAddTable(wxCommandEvent&)':
./dlg/dlgTable.cpp:1021: warning: unused variable 'row'

> Fixed in the same patch. Don't know if you want a backpatch for 1.8?

Thanks. I don't expect we're ever going to release another 1.8 - do you?

The only thing that looks a little odd with this patch is that the Add
button is always enabled, even if there is no table selected. That is
actually the existing behaviour, but it's a little more obvious now
the button is enabled when editing existing tables as well. Should we
fix that? My gut feeling is yes.

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

Re: Support of INHERIT in existing tables (8.2+ releases)

From
Guillaume Lelarge
Date:
Dave Page a écrit :
> On Sun, Aug 10, 2008 at 6:55 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>
>> Sorry to answer so late, it was a lot of work to get everything working.
>> Anyways, fixed.
>
> No worries. Got the following warning:
>
> ./dlg/dlgTable.cpp: In member function 'void
> dlgTable::OnAddTable(wxCommandEvent&)':
> ./dlg/dlgTable.cpp:1021: warning: unused variable 'row'
>

Fixed.

>> Fixed in the same patch. Don't know if you want a backpatch for 1.8?
>
> Thanks. I don't expect we're ever going to release another 1.8 - do you?
>

There's already 4 fixes for an 1.8.5. And this one.

Moreover, 1.10 is not expected before PostgreSQL 8.4 (march 2009 IIRC).
I think it would be better to have another 1.8. Not now, but there's
still like 7 months to go before next release. Not sure we won't need
another minor release.

> The only thing that looks a little odd with this patch is that the Add
> button is always enabled, even if there is no table selected. That is
> actually the existing behaviour, but it's a little more obvious now
> the button is enabled when editing existing tables as well. Should we
> fix that? My gut feeling is yes.
>

hehe... I saw it but wasn't sure if I should work on that too :)

Anyways, fixed.

New patch available at:
   http://developer.pgadmin.org/~guillaume/inherit82+_20080811.patch.bz2


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com

Re: Support of INHERIT in existing tables (8.2+ releases)

From
"Dave Page"
Date:
On Mon, Aug 11, 2008 at 1:50 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

> There's already 4 fixes for an 1.8.5. And this one.
>
> Moreover, 1.10 is not expected before PostgreSQL 8.4 (march 2009 IIRC). I
> think it would be better to have another 1.8. Not now, but there's still
> like 7 months to go before next release. Not sure we won't need another
> minor release.

<grumble>We rarely release in the second-half of the PG cycle. Still,
go ahead and backpatch if you like.

>> The only thing that looks a little odd with this patch is that the Add
>> button is always enabled, even if there is no table selected. That is
>> actually the existing behaviour, but it's a little more obvious now
>> the button is enabled when editing existing tables as well. Should we
>> fix that? My gut feeling is yes.
>>
>
> hehe... I saw it but wasn't sure if I should work on that too :)
>
> Anyways, fixed.

Cool, thanks. I'm not going to review it again - please apply.

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

Re: Support of INHERIT in existing tables (8.2+ releases)

From
Guillaume Lelarge
Date:
Dave Page a écrit :
> On Mon, Aug 11, 2008 at 1:50 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> [...]
>>> The only thing that looks a little odd with this patch is that the Add
>>> button is always enabled, even if there is no table selected. That is
>>> actually the existing behaviour, but it's a little more obvious now
>>> the button is enabled when editing existing tables as well. Should we
>>> fix that? My gut feeling is yes.
>>>
>> hehe... I saw it but wasn't sure if I should work on that too :)
>>
>> Anyways, fixed.
>
> Cool, thanks. I'm not going to review it again - please apply.
>

Thanks, done :)


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com