public inbox for [email protected]  
help / color / mirror / Atom feed
From: Akshay Joshi <[email protected]>
To: Andrej Antonov <[email protected]>
Cc: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: patch: fixing dublicates foreign-tables
Date: Tue, 8 Dec 2015 17:31:10 +0530
Message-ID: <CANxoLDfvidhCn6_VctVVWWYpQwNNeivChbxvzt3RXK=LecH2aw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CA+OCxowu1E6ObUsYea+uxYa5rx+2HQg0Z7byOGX5hwpa0r-Crg@mail.gmail.com>
	<CANxoLDc2mVbjvE3gz6-+NzyBcq0CdmM2_hNNinT2hhGa_E0=9w@mail.gmail.com>
	<CANxoLDe=xsFLwxEhP5XvEym2jsOHEanyPUW9JgRnwJ7Oy0LaJg@mail.gmail.com>
	<[email protected]>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi Andrej

Your patch looks good to me. During testing I came across one more issue
when user select the existing foreign table and click on Properties menu
entry in the "Foreign Server" combo box is blank. I have fixed the issue
and attached is the patch with combined fix.

*Issue *:- As per the code "SetValue()" method of the wxComboBox is called
in case of existing foreign table, but in the documentation of the wxWidget
it is mentioned that "For a combobox with wxCB_READONLY style the string
must be in the combobox choices list otherwise the call to SetValue() is
ignored"

*Fix*:- Insert the value of Foreign Servers before calling the "SetValue()"
method.


On Tue, Dec 8, 2015 at 4:23 PM, Andrej Antonov <[email protected]> wrote:

> hello, Akshay!
>
> screenshot:
>
> "Screenshot from 2015-12-08 13-43-54 with-labels.png" (as attachment file).
>
> script to reproduce:
>
>
>
>         CREATE FOREIGN DATA WRAPPER postgresql
>           VALIDATOR postgresql_fdw_validator;
>
>         CREATE SERVER film_server FOREIGN DATA WRAPPER postgresql OPTIONS
> (host 'foo', dbname 'foodb', port '5432');
>
>         CREATE FOREIGN TABLE films1 (
>             code        char(5) NOT NULL,
>             title       varchar(40) NOT NULL,
>             did         integer NOT NULL,
>             date_prod   date,
>             kind        varchar(10),
>             len         interval hour to minute
>         )
>         SERVER film_server;
>
>         CREATE FOREIGN TABLE films2 (
>             code        char(5) NOT NULL,
>             title       varchar(40) NOT NULL,
>             did         integer NOT NULL,
>             date_prod   date,
>             kind        varchar(10),
>             len         interval hour to minute
>         )
>         SERVER film_server;
>
>         COMMENT ON FOREIGN TABLE films1 IS 'description A';
>         COMMENT ON FOREIGN TABLE films2 IS 'description B';
>
>         COMMENT ON COLUMN films1.code IS 'descr code A';
>         COMMENT ON COLUMN films1.title IS 'descr title A';
>         COMMENT ON COLUMN films1.did IS 'descr did A';
>         COMMENT ON COLUMN films2.code IS 'descr code B';
>         COMMENT ON COLUMN films2.title IS 'descr title B';
>         COMMENT ON COLUMN films2.did IS 'descr did B';
>
>
>
> Akshay Joshi писал 2015-12-08 13:21:
>
>> Hi Andrej
>>
>> Can you please share the reproduction step of the issue that you have
>> fixed.
>>
>> On Tue, Dec 8, 2015 at 11:15 AM, Akshay Joshi
>> <[email protected]> wrote:
>>
>> On Tue, Dec 8, 2015 at 11:09 AM, Dave Page <[email protected]>
>>> wrote:
>>>
>>> Akshay, can you review this please?
>>>>
>>>
>>> Sure, Dave.
>>>
>>> 2015-12-07 19:47 GMT+05:30 Andrej Antonov <[email protected]>:
>>>>
>>>>> fixing dublicates foreign-tables (see attachment file).
>>>>>
>>>>> this email -- it is just copy of pull-request
>>>>> https://github.com/postgres/pgadmin3/pull/11 [1] .
>>>>>
>>>>> thanks in advance.
>>>>>
>>>>> --
>>>>> Андрей Антонов,
>>>>> инженер-программист отдела
>>>>>
>>>> информационных технологий и
>>>> программирования,
>>>>
>>>>> компания «Импульс М»
>>>>>
>>>>> --
>>>>> Sent via pgadmin-hackers mailing list
>>>>>
>>>> ([email protected])
>>>>
>>>>> To make changes to your subscription:
>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers [2]
>>>>>
>>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com [3]
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com [4]
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>> --
>>>
>>> AKSHAY JOSHI
>>> PRINCIPAL SOFTWARE ENGINEER
>>>
>>> Phone: +91 20-3058-9517
>>> Mobile: +91 976-788-8246
>>>
>>
>> --
>>
>> AKSHAY JOSHI
>> PRINCIPAL SOFTWARE ENGINEER
>>
>> Phone: +91 20-3058-9517
>> Mobile: +91 976-788-8246
>>
>> Links:
>> ------
>> [1] https://github.com/postgres/pgadmin3/pull/11
>> [2] http://www.postgresql.org/mailpref/pgadmin-hackers
>> [3] http://pgsnake.blogspot.com
>> [4] http://www.enterprisedb.com
>>
>
> --
> Андрей Антонов,
> инженер-программист отдела информационных технологий и программирования,
> компания «Импульс М»
>



-- 
*Akshay Joshi*
*Principal Software Engineer *



*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*


-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Attachments:

  [image/png] Foreign_Server.png (31.5K, 3-Foreign_Server.png)
  download | view image

  [application/octet-stream] 0001-pgForeignTableFactory-CreateObjects-use-AND-des_V2.patch (2.2K, 4-0001-pgForeignTableFactory-CreateObjects-use-AND-des_V2.patch)
  download | inline diff:
diff --git a/pgadmin/dlg/dlgForeignTable.cpp b/pgadmin/dlg/dlgForeignTable.cpp
index e827469..393ac0c 100644
--- a/pgadmin/dlg/dlgForeignTable.cpp
+++ b/pgadmin/dlg/dlgForeignTable.cpp
@@ -117,6 +117,24 @@ int dlgForeignTable::Go(bool modal)
 	btnAdd->Disable();
 	btnRemove->Disable();
 
+	if (connection)
+	{
+		pgSet *set = connection->ExecuteSet(
+			wxT("SELECT srvname\n")
+			wxT("  FROM pg_foreign_server\n")
+			wxT("  ORDER BY srvname"));
+		if (set)
+		{
+			while (!set->Eof())
+			{
+				wxString srvname = set->GetVal(wxT("srvname"));
+				cbForeignServer->Append(srvname);
+				set->MoveNext();
+			}
+			delete set;
+		}
+	}
+
 	if (foreigntable)
 	{
 		// Edit Mode
@@ -185,21 +203,6 @@ int dlgForeignTable::Go(bool modal)
 		// Create mode
 		cbOwner->Append(wxEmptyString);
 		cbOwner->Disable();
-
-		pgSet *set = connection->ExecuteSet(
-		                 wxT("SELECT srvname\n")
-		                 wxT("  FROM pg_foreign_server\n")
-		                 wxT("  ORDER BY srvname"));
-		if (set)
-		{
-			while (!set->Eof())
-			{
-				wxString srvname = set->GetVal(wxT("srvname"));
-				cbForeignServer->Append(srvname);
-				set->MoveNext();
-			}
-			delete set;
-		}
 		cbForeignServer->SetSelection(0);
 	}
 
diff --git a/pgadmin/schema/pgForeignTable.cpp b/pgadmin/schema/pgForeignTable.cpp
index a02a18e..e057855 100644
--- a/pgadmin/schema/pgForeignTable.cpp
+++ b/pgadmin/schema/pgForeignTable.cpp
@@ -360,7 +360,8 @@ pgObject *pgForeignTableFactory::CreateObjects(pgCollection *collection, ctlTree
 	                wxT("  FROM pg_class c\n")
 	                wxT("  JOIN pg_foreign_table ft ON c.oid=ft.ftrelid\n")
 	                wxT("  LEFT OUTER JOIN pg_foreign_server fs ON ft.ftserver=fs.oid\n")
-	                wxT("  LEFT OUTER JOIN pg_description des ON (des.objoid=c.oid AND des.classoid='pg_class'::regclass)\n")
+	                wxT("  LEFT OUTER JOIN pg_description des ON (des.objoid=c.oid AND des.classoid='pg_class'::regclass\n")
+	                wxT("  AND des.objsubid=0)\n")
 	                wxT(" WHERE c.relnamespace = ") + collection->GetSchema()->GetOidStr() + wxT("\n")
 	                + restriction +
 	                wxT(" ORDER BY c.relname");


view thread (6+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: patch: fixing dublicates foreign-tables
  In-Reply-To: <CANxoLDfvidhCn6_VctVVWWYpQwNNeivChbxvzt3RXK=LecH2aw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox