Обсуждение: Materialized View Patch File

On Wed, May 29, 2013 at 10:40 AM, Neel Patel
<neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the updated patch after fixing some of the issues and comments
> given by Ashesh.

I haven't looked at the code, but judging by the error I got when
first clicking on a view on a PG 9.2 server, you forgot to make this
code version-dependent:

2013-05-31 13:25:57 QUERY  : Scalar query (localhost:5432): SELECT
count(*) FROM pg_matviews WHERE matviewname = 'pem.avail_servers' AND
schemaname = 'pem'
2013-05-31 13:25:57 ERROR  : ERROR:  relation "pg_matviews" does not exist
LINE 1: SELECT count(*) FROM pg_matviews WHERE matviewname = 'pem.av...
                             ^
It then goes on to give the view definition as:

-- MATERIALIZED View: pem.avail_servers

-- DROP MATERIALIZED VIEW pem.avail_servers;

CREATE MATERIALIZED VIEW pem.avail_servers AS
 SELECT s.id, s.description, s.server, s.port, s.database, s.ssl, s.serviceid,
...
...

(as a side note, the first line for a real mat view should be: "--
Materialized View: pem.avail_servers")

--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Hi

On Mon, Jun 3, 2013 at 7:56 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Thanks for update.
>
> Yes it is a bug, I have not put the version check.
>
> Below are the changes.
>  --- Put the PG version check for mat views
>  --- In SQL Pane window change the the description of the view from --
> MATERIALIZED View: xxx TO
>        -- Materialized View: xxx
>
> Please find the updated patch in attachement.

Thanks - a few more comments now I've tested further:

- The tab should be called "Materialization" (unless someone has a better idea)

- I can still enable the "Materialized View" option on a 9.2 database
on the dialogue.

- I can't select a tablespace from the drop down menu, but I can type
a name in. Should be select-only - see dlgTable.

- The vacuum options UI design should match the existing one on
dlgTable (it's a nice design, but consistency comes first).

- Sidenote, because it will be irrelevant, s/Toastable/Toast table/

--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

On Mon, Jun 3, 2013 at 7:29 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
>
>
> On Mon, Jun 3, 2013 at 5:20 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Hi
>>
>> On Mon, Jun 3, 2013 at 7:56 AM, Neel Patel <neel.patel@enterprisedb.com>
>> wrote:
>> > Hi Dave,
>> >
>> > Thanks for update.
>> >
>> > Yes it is a bug, I have not put the version check.
>> >
>> > Below are the changes.
>> >  --- Put the PG version check for mat views
>> >  --- In SQL Pane window change the the description of the view from --
>> > MATERIALIZED View: xxx TO
>> >        -- Materialized View: xxx
>> >
>> > Please find the updated patch in attachement.
>>
>> Thanks - a few more comments now I've tested further:
>>
>> - The tab should be called "Materialization" (unless someone has a better
>> idea)
>>
> I will fix it.
>
>>
>> - I can still enable the "Materialized View" option on a 9.2 database
>> on the dialogue.
>>
>
> I will fix it.
>
>>
>> - I can't select a tablespace from the drop down menu, but I can type
>> a name in. Should be select-only - see dlgTable.
>
>
> I will fix it.
>
>>
>>
>> - The vacuum options UI design should match the existing one on
>> dlgTable (it's a nice design, but consistency comes first).
>
>
> Can we change dlgTable with new UI design ?

Not for this release, it's too late.


--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Hi

On Mon, Jun 10, 2013 at 7:23 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> We can combine the Materialization Tab and auto-vacuum tab.
>
> To make it consistent with other UI I would suggest to have one Tab called
> "Materialization" and in that tab we have another three Tabs.
>
> 1. Options ( Or we can have another name ) ---- For setting the option for
> materialized view
> 2. Table
> 3. Toast Table

There are only two options aren't there (Materialised: Yes/no and fill
factor)? If so, I'd have one tab with those options at the top, then a
"Vacuum Options" panel below that with 2 tabs for the vacuum stuff. I
think that'll fit just fine.


> On Wed, Jun 5, 2013 at 1:20 PM, Neel Patel <neel.patel@enterprisedb.com>
> wrote:
>>
>> Hi Dave,
>>
>> Please find the patch in attachement with below changes.
>>
>>  --- Changed the UI design and make it consistence with dlgTable.
>>
>> When user creates the view then user has option to create Normal View Or
>> Materialized View.
>> User is not allowed to change the "Security Barrier" and "Materialized
>> View" at the same time because "Security Barrier" option is for Normal View.
>>
>> So we put the below message
>>
>> "Security barrier and materialized view can not be enbale at the same
>> time"
>>
>> Let me know if any modification is required.
>>
>> Thanks,
>> Neel Patel
>>
>>
>> On Tue, Jun 4, 2013 at 12:02 AM, Dave Page <dave.page@enterprisedb.com>
>> wrote:
>>>
>>> On Mon, Jun 3, 2013 at 7:29 PM, Neel Patel <neel.patel@enterprisedb.com>
>>> wrote:
>>> >
>>> >
>>> > On Mon, Jun 3, 2013 at 5:20 PM, Dave Page <dave.page@enterprisedb.com>
>>> > wrote:
>>> >>
>>> >> Hi
>>> >>
>>> >> On Mon, Jun 3, 2013 at 7:56 AM, Neel Patel
>>> >> <neel.patel@enterprisedb.com>
>>> >> wrote:
>>> >> > Hi Dave,
>>> >> >
>>> >> > Thanks for update.
>>> >> >
>>> >> > Yes it is a bug, I have not put the version check.
>>> >> >
>>> >> > Below are the changes.
>>> >> >  --- Put the PG version check for mat views
>>> >> >  --- In SQL Pane window change the the description of the view from
>>> >> > --
>>> >> > MATERIALIZED View: xxx TO
>>> >> >        -- Materialized View: xxx
>>> >> >
>>> >> > Please find the updated patch in attachement.
>>> >>
>>> >> Thanks - a few more comments now I've tested further:
>>> >>
>>> >> - The tab should be called "Materialization" (unless someone has a
>>> >> better
>>> >> idea)
>>> >>
>>> > I will fix it.
>>> >
>>> >>
>>> >> - I can still enable the "Materialized View" option on a 9.2 database
>>> >> on the dialogue.
>>> >>
>>> >
>>> > I will fix it.
>>> >
>>> >>
>>> >> - I can't select a tablespace from the drop down menu, but I can type
>>> >> a name in. Should be select-only - see dlgTable.
>>> >
>>> >
>>> > I will fix it.
>>> >
>>> >>
>>> >>
>>> >> - The vacuum options UI design should match the existing one on
>>> >> dlgTable (it's a nice design, but consistency comes first).
>>> >
>>> >
>>> > Can we change dlgTable with new UI design ?
>>>
>>> Not for this release, it's too late.
>>>
>>>
>>> --
>>> Dave Page
>>> Chief Architect, Tools & Installers
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>
>>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Hi

Per our discussion earlier, please put the Materialized: Yes/No option
at the top of the Materialization tab, and below that add a tabset
with three tabs for Options, and the heap and toast Vacuum options.

Thanks.

On Mon, Jun 10, 2013 at 10:16 AM, Neel Patel
<neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> We have below three options.
>
> 1. Materialized Yes/No
> 2. TableSpace
> 3. Fill Factor
>
> I have checked with three options and below that two tabs so for that we
> need to increase little height of the dialog to fit in. If so then we will
> implement that and after detail testing will send you the patch along with
> all the fixes.
>
> Thanks,
> Neel Patel
>
>
> On Mon, Jun 10, 2013 at 12:27 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Hi
>>
>> On Mon, Jun 10, 2013 at 7:23 AM, Neel Patel <neel.patel@enterprisedb.com>
>> wrote:
>> > Hi Dave,
>> >
>> > We can combine the Materialization Tab and auto-vacuum tab.
>> >
>> > To make it consistent with other UI I would suggest to have one Tab
>> > called
>> > "Materialization" and in that tab we have another three Tabs.
>> >
>> > 1. Options ( Or we can have another name ) ---- For setting the option
>> > for
>> > materialized view
>> > 2. Table
>> > 3. Toast Table
>>
>> There are only two options aren't there (Materialised: Yes/no and fill
>> factor)? If so, I'd have one tab with those options at the top, then a
>> "Vacuum Options" panel below that with 2 tabs for the vacuum stuff. I
>> think that'll fit just fine.
>>
>>
>> > On Wed, Jun 5, 2013 at 1:20 PM, Neel Patel <neel.patel@enterprisedb.com>
>> > wrote:
>> >>
>> >> Hi Dave,
>> >>
>> >> Please find the patch in attachement with below changes.
>> >>
>> >>  --- Changed the UI design and make it consistence with dlgTable.
>> >>
>> >> When user creates the view then user has option to create Normal View
>> >> Or
>> >> Materialized View.
>> >> User is not allowed to change the "Security Barrier" and "Materialized
>> >> View" at the same time because "Security Barrier" option is for Normal
>> >> View.
>> >>
>> >> So we put the below message
>> >>
>> >> "Security barrier and materialized view can not be enbale at the same
>> >> time"
>> >>
>> >> Let me know if any modification is required.
>> >>
>> >> Thanks,
>> >> Neel Patel
>> >>
>> >>
>> >> On Tue, Jun 4, 2013 at 12:02 AM, Dave Page <dave.page@enterprisedb.com>
>> >> wrote:
>> >>>
>> >>> On Mon, Jun 3, 2013 at 7:29 PM, Neel Patel
>> >>> <neel.patel@enterprisedb.com>
>> >>> wrote:
>> >>> >
>> >>> >
>> >>> > On Mon, Jun 3, 2013 at 5:20 PM, Dave Page
>> >>> > <dave.page@enterprisedb.com>
>> >>> > wrote:
>> >>> >>
>> >>> >> Hi
>> >>> >>
>> >>> >> On Mon, Jun 3, 2013 at 7:56 AM, Neel Patel
>> >>> >> <neel.patel@enterprisedb.com>
>> >>> >> wrote:
>> >>> >> > Hi Dave,
>> >>> >> >
>> >>> >> > Thanks for update.
>> >>> >> >
>> >>> >> > Yes it is a bug, I have not put the version check.
>> >>> >> >
>> >>> >> > Below are the changes.
>> >>> >> >  --- Put the PG version check for mat views
>> >>> >> >  --- In SQL Pane window change the the description of the view
>> >>> >> > from
>> >>> >> > --
>> >>> >> > MATERIALIZED View: xxx TO
>> >>> >> >        -- Materialized View: xxx
>> >>> >> >
>> >>> >> > Please find the updated patch in attachement.
>> >>> >>
>> >>> >> Thanks - a few more comments now I've tested further:
>> >>> >>
>> >>> >> - The tab should be called "Materialization" (unless someone has a
>> >>> >> better
>> >>> >> idea)
>> >>> >>
>> >>> > I will fix it.
>> >>> >
>> >>> >>
>> >>> >> - I can still enable the "Materialized View" option on a 9.2
>> >>> >> database
>> >>> >> on the dialogue.
>> >>> >>
>> >>> >
>> >>> > I will fix it.
>> >>> >
>> >>> >>
>> >>> >> - I can't select a tablespace from the drop down menu, but I can
>> >>> >> type
>> >>> >> a name in. Should be select-only - see dlgTable.
>> >>> >
>> >>> >
>> >>> > I will fix it.
>> >>> >
>> >>> >>
>> >>> >>
>> >>> >> - The vacuum options UI design should match the existing one on
>> >>> >> dlgTable (it's a nice design, but consistency comes first).
>> >>> >
>> >>> >
>> >>> > Can we change dlgTable with new UI design ?
>> >>>
>> >>> Not for this release, it's too late.
>> >>>
>> >>>
>> >>> --
>> >>> Dave Page
>> >>> Chief Architect, Tools & Installers
>> >>> EnterpriseDB: http://www.enterprisedb.com
>> >>> The Enterprise PostgreSQL Company
>> >>>
>> >>> Blog: http://pgsnake.blogspot.com
>> >>> Twitter: @pgsnake
>> >>
>> >>
>> >
>>
>>
>>
>> --
>> Dave Page
>> Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Hi

On Thu, Jun 13, 2013 at 9:23 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the updated patch file for Materialized View with below changes.
>
>  ---- Materialized View UI changes
>  ---- Added Option for WITH DATA
>  ---- Some of the bug fixes

Much better :-)

The only issue that I can find is that if I create a matview, and give
it some custom vacuum properties, when I re-open the properties
dialogue, I cannot edit any of the auto vacuum properties - they're
all greyed out.

*But*, it doesn't happen with all views - and in a (admittedly brief)
test, I couldn't see what caused it. I see the problem with "gerp",
but not "foo" or "ruletest":

CREATE MATERIALIZED VIEW foo
WITH (
  FILLFACTOR=12,
  autovacuum_enabled=true,
  autovacuum_analyze_threshold=10,
  toast.autovacuum_enabled=true
) AS
 SELECT pg_class.relname,
    pg_class.relnamespace,
    pg_class.reltype,
    pg_class.reloftype,
    pg_class.relowner,
    pg_class.relam,
    pg_class.relfilenode,
    pg_class.reltablespace,
    pg_class.relpages,
    pg_class.reltuples,
    pg_class.relallvisible,
    pg_class.reltoastrelid,
    pg_class.reltoastidxid,
    pg_class.relhasindex,
    pg_class.relisshared,
    pg_class.relpersistence,
    pg_class.relkind,
    pg_class.relnatts,
    pg_class.relchecks,
    pg_class.relhasoids,
    pg_class.relhaspkey,
    pg_class.relhasrules,
    pg_class.relhastriggers,
    pg_class.relhassubclass,
    pg_class.relispopulated,
    pg_class.relfrozenxid,
    pg_class.relminmxid,
    pg_class.relacl,
    pg_class.reloptions
   FROM pg_class
WITH DATA;

ALTER TABLE foo
  OWNER TO postgres;

CREATE MATERIALIZED VIEW gerp
WITH (
  FILLFACTOR=12,
  autovacuum_enabled=true,
  autovacuum_vacuum_threshold=40
) AS
 SELECT pg_class.oid,
    pg_class.relname
   FROM pg_class
WITH DATA;

ALTER TABLE gerp
  OWNER TO postgres;

CREATE MATERIALIZED VIEW ruletest
WITH (
  autovacuum_enabled=true,
  autovacuum_vacuum_threshold=23,
  autovacuum_vacuum_cost_delay=15,
  toast.autovacuum_enabled=true,
  toast.autovacuum_freeze_min_age=500000
) AS
 SELECT pg_class.relname,
    pg_class.relnamespace,
    pg_class.reltype,
    pg_class.reloftype,
    pg_class.relowner,
    pg_class.relam,
    pg_class.relfilenode,
    pg_class.reltablespace,
    pg_class.relpages,
    pg_class.reltuples,
    pg_class.relallvisible,
    pg_class.reltoastrelid,
    pg_class.reltoastidxid,
    pg_class.relhasindex,
    pg_class.relisshared,
    pg_class.relpersistence,
    pg_class.relkind,
    pg_class.relnatts,
    pg_class.relchecks,
    pg_class.relhasoids,
    pg_class.relhaspkey,
    pg_class.relhasrules,
    pg_class.relhastriggers,
    pg_class.relhassubclass,
    pg_class.relispopulated,
    pg_class.relfrozenxid,
    pg_class.relminmxid,
    pg_class.relacl,
    pg_class.reloptions
   FROM pg_class
WITH DATA;

ALTER TABLE ruletest
  OWNER TO postgres;

Aside from that issue, I think it's just about done :-)

--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Committed with a couple of minor changes:

- Add the word "MATERIALIZED" to the COMMENT SQL where appropriate.

- List the comment after all the other properties, per our standard.

Thanks!!

On Fri, Jun 14, 2013 at 7:02 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
>
> On Thu, Jun 13, 2013 at 5:41 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Hi
>>
>> On Thu, Jun 13, 2013 at 9:23 AM, Neel Patel <neel.patel@enterprisedb.com>
>> wrote:
>> > Hi Dave,
>> >
>> > Please find the updated patch file for Materialized View with below
>> > changes.
>> >
>> >  ---- Materialized View UI changes
>> >  ---- Added Option for WITH DATA
>> >  ---- Some of the bug fixes
>>
>> Much better :-)
>>
>> The only issue that I can find is that if I create a matview, and give
>> it some custom vacuum properties, when I re-open the properties
>> dialogue, I cannot edit any of the auto vacuum properties - they're
>> all greyed out.
>>
>
> Yes it is an issue with index. Auto vacuum properties should be disable for
> the Toast Table tab, not for the Table tab. It depends on the type of query
> we are providing in definition. If it contains the ToastTable then Toast
> Table tab will be enabled otherwise it will be disabled. Here in below case
> query for "foo" and "ruletest" views has toast table so it will not disable
> the tab but "gerp" view's query doen't have toast table so it will be
> disable the toast table tab ( Currently it is disabling the Table tab which
> has beed fixed with attached patch).
>
>
>>
>> *But*, it doesn't happen with all views - and in a (admittedly brief)
>> test, I couldn't see what caused it. I see the problem with "gerp",
>> but not "foo" or "ruletest":
>>
>> CREATE MATERIALIZED VIEW foo
>> WITH (
>>   FILLFACTOR=12,
>>   autovacuum_enabled=true,
>>   autovacuum_analyze_threshold=10,
>>   toast.autovacuum_enabled=true
>> ) AS
>>  SELECT pg_class.relname,
>>     pg_class.relnamespace,
>>     pg_class.reltype,
>>     pg_class.reloftype,
>>     pg_class.relowner,
>>     pg_class.relam,
>>     pg_class.relfilenode,
>>     pg_class.reltablespace,
>>     pg_class.relpages,
>>     pg_class.reltuples,
>>     pg_class.relallvisible,
>>     pg_class.reltoastrelid,
>>     pg_class.reltoastidxid,
>>     pg_class.relhasindex,
>>     pg_class.relisshared,
>>     pg_class.relpersistence,
>>     pg_class.relkind,
>>     pg_class.relnatts,
>>     pg_class.relchecks,
>>     pg_class.relhasoids,
>>     pg_class.relhaspkey,
>>     pg_class.relhasrules,
>>     pg_class.relhastriggers,
>>     pg_class.relhassubclass,
>>     pg_class.relispopulated,
>>     pg_class.relfrozenxid,
>>     pg_class.relminmxid,
>>     pg_class.relacl,
>>     pg_class.reloptions
>>    FROM pg_class
>> WITH DATA;
>>
>> ALTER TABLE foo
>>   OWNER TO postgres;
>>
>> CREATE MATERIALIZED VIEW gerp
>> WITH (
>>   FILLFACTOR=12,
>>   autovacuum_enabled=true,
>>   autovacuum_vacuum_threshold=40
>> ) AS
>>  SELECT pg_class.oid,
>>     pg_class.relname
>>    FROM pg_class
>> WITH DATA;
>>
>> ALTER TABLE gerp
>>   OWNER TO postgres;
>>
>> CREATE MATERIALIZED VIEW ruletest
>> WITH (
>>   autovacuum_enabled=true,
>>   autovacuum_vacuum_threshold=23,
>>   autovacuum_vacuum_cost_delay=15,
>>   toast.autovacuum_enabled=true,
>>   toast.autovacuum_freeze_min_age=500000
>> ) AS
>>  SELECT pg_class.relname,
>>     pg_class.relnamespace,
>>     pg_class.reltype,
>>     pg_class.reloftype,
>>     pg_class.relowner,
>>     pg_class.relam,
>>     pg_class.relfilenode,
>>     pg_class.reltablespace,
>>     pg_class.relpages,
>>     pg_class.reltuples,
>>     pg_class.relallvisible,
>>     pg_class.reltoastrelid,
>>     pg_class.reltoastidxid,
>>     pg_class.relhasindex,
>>     pg_class.relisshared,
>>     pg_class.relpersistence,
>>     pg_class.relkind,
>>     pg_class.relnatts,
>>     pg_class.relchecks,
>>     pg_class.relhasoids,
>>     pg_class.relhaspkey,
>>     pg_class.relhasrules,
>>     pg_class.relhastriggers,
>>     pg_class.relhassubclass,
>>     pg_class.relispopulated,
>>     pg_class.relfrozenxid,
>>     pg_class.relminmxid,
>>     pg_class.relacl,
>>     pg_class.reloptions
>>    FROM pg_class
>> WITH DATA;
>>
>> ALTER TABLE ruletest
>>   OWNER TO postgres;
>>
>> Aside from that issue, I think it's just about done :-)
>>
>> --
>> Dave Page
>> Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Thanks, applied.

On Wed, Jun 19, 2013 at 12:54 PM, Neel Patel
<neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Fixed the below bug.
>
> Bug:-
>
> When user creates the new schema and provides <schema_name>.<table_name> in
> the definition of the materialized view then it fails to display correct
> information in properties and SQL pane. Below are the steps to reproduce it.
>
> Step 1 :- Create new schema under database.
> Step 2:- Create new table under newly created schema
> Step 3:- Create new materialized view and give in definition
> <new_schema>.<new_table>
> Step 4:- Click on the materialized view and check properties and SQL Pane.
>
> Please find attached patch for fix.
>
> Thanks,
> Neel Patel
>
>
> On Tue, Jun 18, 2013 at 8:13 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Committed with a couple of minor changes:
>>
>> - Add the word "MATERIALIZED" to the COMMENT SQL where appropriate.
>>
>> - List the comment after all the other properties, per our standard.
>>
>> Thanks!!
>>
>> On Fri, Jun 14, 2013 at 7:02 AM, Neel Patel <neel.patel@enterprisedb.com>
>> wrote:
>> > Hi Dave,
>> >
>> >
>> > On Thu, Jun 13, 2013 at 5:41 PM, Dave Page <dave.page@enterprisedb.com>
>> > wrote:
>> >>
>> >> Hi
>> >>
>> >> On Thu, Jun 13, 2013 at 9:23 AM, Neel Patel
>> >> <neel.patel@enterprisedb.com>
>> >> wrote:
>> >> > Hi Dave,
>> >> >
>> >> > Please find the updated patch file for Materialized View with below
>> >> > changes.
>> >> >
>> >> >  ---- Materialized View UI changes
>> >> >  ---- Added Option for WITH DATA
>> >> >  ---- Some of the bug fixes
>> >>
>> >> Much better :-)
>> >>
>> >> The only issue that I can find is that if I create a matview, and give
>> >> it some custom vacuum properties, when I re-open the properties
>> >> dialogue, I cannot edit any of the auto vacuum properties - they're
>> >> all greyed out.
>> >>
>> >
>> > Yes it is an issue with index. Auto vacuum properties should be disable
>> > for
>> > the Toast Table tab, not for the Table tab. It depends on the type of
>> > query
>> > we are providing in definition. If it contains the ToastTable then Toast
>> > Table tab will be enabled otherwise it will be disabled. Here in below
>> > case
>> > query for "foo" and "ruletest" views has toast table so it will not
>> > disable
>> > the tab but "gerp" view's query doen't have toast table so it will be
>> > disable the toast table tab ( Currently it is disabling the Table tab
>> > which
>> > has beed fixed with attached patch).
>> >
>> >
>> >>
>> >> *But*, it doesn't happen with all views - and in a (admittedly brief)
>> >> test, I couldn't see what caused it. I see the problem with "gerp",
>> >> but not "foo" or "ruletest":
>> >>
>> >> CREATE MATERIALIZED VIEW foo
>> >> WITH (
>> >>   FILLFACTOR=12,
>> >>   autovacuum_enabled=true,
>> >>   autovacuum_analyze_threshold=10,
>> >>   toast.autovacuum_enabled=true
>> >> ) AS
>> >>  SELECT pg_class.relname,
>> >>     pg_class.relnamespace,
>> >>     pg_class.reltype,
>> >>     pg_class.reloftype,
>> >>     pg_class.relowner,
>> >>     pg_class.relam,
>> >>     pg_class.relfilenode,
>> >>     pg_class.reltablespace,
>> >>     pg_class.relpages,
>> >>     pg_class.reltuples,
>> >>     pg_class.relallvisible,
>> >>     pg_class.reltoastrelid,
>> >>     pg_class.reltoastidxid,
>> >>     pg_class.relhasindex,
>> >>     pg_class.relisshared,
>> >>     pg_class.relpersistence,
>> >>     pg_class.relkind,
>> >>     pg_class.relnatts,
>> >>     pg_class.relchecks,
>> >>     pg_class.relhasoids,
>> >>     pg_class.relhaspkey,
>> >>     pg_class.relhasrules,
>> >>     pg_class.relhastriggers,
>> >>     pg_class.relhassubclass,
>> >>     pg_class.relispopulated,
>> >>     pg_class.relfrozenxid,
>> >>     pg_class.relminmxid,
>> >>     pg_class.relacl,
>> >>     pg_class.reloptions
>> >>    FROM pg_class
>> >> WITH DATA;
>> >>
>> >> ALTER TABLE foo
>> >>   OWNER TO postgres;
>> >>
>> >> CREATE MATERIALIZED VIEW gerp
>> >> WITH (
>> >>   FILLFACTOR=12,
>> >>   autovacuum_enabled=true,
>> >>   autovacuum_vacuum_threshold=40
>> >> ) AS
>> >>  SELECT pg_class.oid,
>> >>     pg_class.relname
>> >>    FROM pg_class
>> >> WITH DATA;
>> >>
>> >> ALTER TABLE gerp
>> >>   OWNER TO postgres;
>> >>
>> >> CREATE MATERIALIZED VIEW ruletest
>> >> WITH (
>> >>   autovacuum_enabled=true,
>> >>   autovacuum_vacuum_threshold=23,
>> >>   autovacuum_vacuum_cost_delay=15,
>> >>   toast.autovacuum_enabled=true,
>> >>   toast.autovacuum_freeze_min_age=500000
>> >> ) AS
>> >>  SELECT pg_class.relname,
>> >>     pg_class.relnamespace,
>> >>     pg_class.reltype,
>> >>     pg_class.reloftype,
>> >>     pg_class.relowner,
>> >>     pg_class.relam,
>> >>     pg_class.relfilenode,
>> >>     pg_class.reltablespace,
>> >>     pg_class.relpages,
>> >>     pg_class.reltuples,
>> >>     pg_class.relallvisible,
>> >>     pg_class.reltoastrelid,
>> >>     pg_class.reltoastidxid,
>> >>     pg_class.relhasindex,
>> >>     pg_class.relisshared,
>> >>     pg_class.relpersistence,
>> >>     pg_class.relkind,
>> >>     pg_class.relnatts,
>> >>     pg_class.relchecks,
>> >>     pg_class.relhasoids,
>> >>     pg_class.relhaspkey,
>> >>     pg_class.relhasrules,
>> >>     pg_class.relhastriggers,
>> >>     pg_class.relhassubclass,
>> >>     pg_class.relispopulated,
>> >>     pg_class.relfrozenxid,
>> >>     pg_class.relminmxid,
>> >>     pg_class.relacl,
>> >>     pg_class.reloptions
>> >>    FROM pg_class
>> >> WITH DATA;
>> >>
>> >> ALTER TABLE ruletest
>> >>   OWNER TO postgres;
>> >>
>> >> Aside from that issue, I think it's just about done :-)
>> >>
>> >> --
>> >> Dave Page
>> >> Chief Architect, Tools & Installers
>> >> EnterpriseDB: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >>
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Hi

On Mon, Jul 1, 2013 at 1:00 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Find the attached patch file ( it is on top of master branch ) for change in
> icon of materialized view in pgAdmin and also put the icon images attached
> in pgadmin/include/images folder.
>
> Please let me know in case of any issue.

A couple of issues:

- "Closed" (as in GetClosedIconId, smallClosedId and ClosedId) isn't
the right phrase in this context. Maybe s/Closed/Materialized/ ?

- Including a SELECT inside the loop over pgSet *views is a potential
performance killer if there are a lot of views. Please add a suitable
column to the outer query.

- I notice there are also 2 "if
(collection->GetConnection()->BackendMinimumVersion(9, 3))" tests
right next to each other in that outer query, one to get the vacuum
params, and one to get the fill factor. Can you please merge them
together?

Thanks.

>
> Thanks,
> Neel Patel
>
>
>
> On Wed, Jun 19, 2013 at 8:46 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Thanks, applied.
>>
>> On Wed, Jun 19, 2013 at 12:54 PM, Neel Patel
>> <neel.patel@enterprisedb.com> wrote:
>> > Hi Dave,
>> >
>> > Fixed the below bug.
>> >
>> > Bug:-
>> >
>> > When user creates the new schema and provides <schema_name>.<table_name>
>> > in
>> > the definition of the materialized view then it fails to display correct
>> > information in properties and SQL pane. Below are the steps to reproduce
>> > it.
>> >
>> > Step 1 :- Create new schema under database.
>> > Step 2:- Create new table under newly created schema
>> > Step 3:- Create new materialized view and give in definition
>> > <new_schema>.<new_table>
>> > Step 4:- Click on the materialized view and check properties and SQL
>> > Pane.
>> >
>> > Please find attached patch for fix.
>> >
>> > Thanks,
>> > Neel Patel
>> >
>> >
>> > On Tue, Jun 18, 2013 at 8:13 PM, Dave Page <dave.page@enterprisedb.com>
>> > wrote:
>> >>
>> >> Committed with a couple of minor changes:
>> >>
>> >> - Add the word "MATERIALIZED" to the COMMENT SQL where appropriate.
>> >>
>> >> - List the comment after all the other properties, per our standard.
>> >>
>> >> Thanks!!
>> >>
>> >> On Fri, Jun 14, 2013 at 7:02 AM, Neel Patel
>> >> <neel.patel@enterprisedb.com>
>> >> wrote:
>> >> > Hi Dave,
>> >> >
>> >> >
>> >> > On Thu, Jun 13, 2013 at 5:41 PM, Dave Page
>> >> > <dave.page@enterprisedb.com>
>> >> > wrote:
>> >> >>
>> >> >> Hi
>> >> >>
>> >> >> On Thu, Jun 13, 2013 at 9:23 AM, Neel Patel
>> >> >> <neel.patel@enterprisedb.com>
>> >> >> wrote:
>> >> >> > Hi Dave,
>> >> >> >
>> >> >> > Please find the updated patch file for Materialized View with
>> >> >> > below
>> >> >> > changes.
>> >> >> >
>> >> >> >  ---- Materialized View UI changes
>> >> >> >  ---- Added Option for WITH DATA
>> >> >> >  ---- Some of the bug fixes
>> >> >>
>> >> >> Much better :-)
>> >> >>
>> >> >> The only issue that I can find is that if I create a matview, and
>> >> >> give
>> >> >> it some custom vacuum properties, when I re-open the properties
>> >> >> dialogue, I cannot edit any of the auto vacuum properties - they're
>> >> >> all greyed out.
>> >> >>
>> >> >
>> >> > Yes it is an issue with index. Auto vacuum properties should be
>> >> > disable
>> >> > for
>> >> > the Toast Table tab, not for the Table tab. It depends on the type of
>> >> > query
>> >> > we are providing in definition. If it contains the ToastTable then
>> >> > Toast
>> >> > Table tab will be enabled otherwise it will be disabled. Here in
>> >> > below
>> >> > case
>> >> > query for "foo" and "ruletest" views has toast table so it will not
>> >> > disable
>> >> > the tab but "gerp" view's query doen't have toast table so it will be
>> >> > disable the toast table tab ( Currently it is disabling the Table tab
>> >> > which
>> >> > has beed fixed with attached patch).
>> >> >
>> >> >
>> >> >>
>> >> >> *But*, it doesn't happen with all views - and in a (admittedly
>> >> >> brief)
>> >> >> test, I couldn't see what caused it. I see the problem with "gerp",
>> >> >> but not "foo" or "ruletest":
>> >> >>
>> >> >> CREATE MATERIALIZED VIEW foo
>> >> >> WITH (
>> >> >>   FILLFACTOR=12,
>> >> >>   autovacuum_enabled=true,
>> >> >>   autovacuum_analyze_threshold=10,
>> >> >>   toast.autovacuum_enabled=true
>> >> >> ) AS
>> >> >>  SELECT pg_class.relname,
>> >> >>     pg_class.relnamespace,
>> >> >>     pg_class.reltype,
>> >> >>     pg_class.reloftype,
>> >> >>     pg_class.relowner,
>> >> >>     pg_class.relam,
>> >> >>     pg_class.relfilenode,
>> >> >>     pg_class.reltablespace,
>> >> >>     pg_class.relpages,
>> >> >>     pg_class.reltuples,
>> >> >>     pg_class.relallvisible,
>> >> >>     pg_class.reltoastrelid,
>> >> >>     pg_class.reltoastidxid,
>> >> >>     pg_class.relhasindex,
>> >> >>     pg_class.relisshared,
>> >> >>     pg_class.relpersistence,
>> >> >>     pg_class.relkind,
>> >> >>     pg_class.relnatts,
>> >> >>     pg_class.relchecks,
>> >> >>     pg_class.relhasoids,
>> >> >>     pg_class.relhaspkey,
>> >> >>     pg_class.relhasrules,
>> >> >>     pg_class.relhastriggers,
>> >> >>     pg_class.relhassubclass,
>> >> >>     pg_class.relispopulated,
>> >> >>     pg_class.relfrozenxid,
>> >> >>     pg_class.relminmxid,
>> >> >>     pg_class.relacl,
>> >> >>     pg_class.reloptions
>> >> >>    FROM pg_class
>> >> >> WITH DATA;
>> >> >>
>> >> >> ALTER TABLE foo
>> >> >>   OWNER TO postgres;
>> >> >>
>> >> >> CREATE MATERIALIZED VIEW gerp
>> >> >> WITH (
>> >> >>   FILLFACTOR=12,
>> >> >>   autovacuum_enabled=true,
>> >> >>   autovacuum_vacuum_threshold=40
>> >> >> ) AS
>> >> >>  SELECT pg_class.oid,
>> >> >>     pg_class.relname
>> >> >>    FROM pg_class
>> >> >> WITH DATA;
>> >> >>
>> >> >> ALTER TABLE gerp
>> >> >>   OWNER TO postgres;
>> >> >>
>> >> >> CREATE MATERIALIZED VIEW ruletest
>> >> >> WITH (
>> >> >>   autovacuum_enabled=true,
>> >> >>   autovacuum_vacuum_threshold=23,
>> >> >>   autovacuum_vacuum_cost_delay=15,
>> >> >>   toast.autovacuum_enabled=true,
>> >> >>   toast.autovacuum_freeze_min_age=500000
>> >> >> ) AS
>> >> >>  SELECT pg_class.relname,
>> >> >>     pg_class.relnamespace,
>> >> >>     pg_class.reltype,
>> >> >>     pg_class.reloftype,
>> >> >>     pg_class.relowner,
>> >> >>     pg_class.relam,
>> >> >>     pg_class.relfilenode,
>> >> >>     pg_class.reltablespace,
>> >> >>     pg_class.relpages,
>> >> >>     pg_class.reltuples,
>> >> >>     pg_class.relallvisible,
>> >> >>     pg_class.reltoastrelid,
>> >> >>     pg_class.reltoastidxid,
>> >> >>     pg_class.relhasindex,
>> >> >>     pg_class.relisshared,
>> >> >>     pg_class.relpersistence,
>> >> >>     pg_class.relkind,
>> >> >>     pg_class.relnatts,
>> >> >>     pg_class.relchecks,
>> >> >>     pg_class.relhasoids,
>> >> >>     pg_class.relhaspkey,
>> >> >>     pg_class.relhasrules,
>> >> >>     pg_class.relhastriggers,
>> >> >>     pg_class.relhassubclass,
>> >> >>     pg_class.relispopulated,
>> >> >>     pg_class.relfrozenxid,
>> >> >>     pg_class.relminmxid,
>> >> >>     pg_class.relacl,
>> >> >>     pg_class.reloptions
>> >> >>    FROM pg_class
>> >> >> WITH DATA;
>> >> >>
>> >> >> ALTER TABLE ruletest
>> >> >>   OWNER TO postgres;
>> >> >>
>> >> >> Aside from that issue, I think it's just about done :-)
>> >> >>
>> >> >> --
>> >> >> Dave Page
>> >> >> Chief Architect, Tools & Installers
>> >> >> EnterpriseDB: http://www.enterprisedb.com
>> >> >> The Enterprise PostgreSQL Company
>> >> >>
>> >> >> Blog: http://pgsnake.blogspot.com
>> >> >> Twitter: @pgsnake
>> >> >
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Dave Page
>> >> Chief Architect, Tools & Installers
>> >> EnterpriseDB: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >>
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

On 29 May 2013 10:40, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the updated patch after fixing some of the issues and comments
> given by Ashesh.

Just realised this reply has been in my draft folder for over a month...


This comment appears to be incomplete:

//While editing the view, if it is materialized view then only change


The section of if... else if... else if... that begins with:

if (name == wxT("autovacuum_vacuum_cost_delay"))

should probably use:

switch(name)
{
    case wxT("autovacuum_vacuum_cost_delay")

    case ...
}


Given the amount of duplicate functionality this shares with tables,
is there not a way there could be a common class they could both
inherit from to reduce maintenance overhead?

--
Thom

Hi

On Tue, Jul 2, 2013 at 3:25 PM, Thom Brown <thom@linux.com> wrote:
> On 29 May 2013 10:40, Neel Patel <neel.patel@enterprisedb.com> wrote:
>> Hi Dave,
>>
>> Please find the updated patch after fixing some of the issues and comments
>> given by Ashesh.
>
> Just realised this reply has been in my draft folder for over a month...
>
>
> This comment appears to be incomplete:
>
> //While editing the view, if it is materialized view then only change

Yeah, please fix that Neel. Not sure what you were trying to say.

>
> The section of if... else if... else if... that begins with:
>
> if (name == wxT("autovacuum_vacuum_cost_delay"))
>
> should probably use:
>
> switch(name)
> {
>     case wxT("autovacuum_vacuum_cost_delay")
>
>     case ...
> }

wxString's don't play nicely with switch(), if memory serves.

> Given the amount of duplicate functionality this shares with tables,
> is there not a way there could be a common class they could both
> inherit from to reduce maintenance overhead?

Probably not a great deal of benefit. In any case, it would certainly
require a lot of refactoring which is far more work than we have time
for.

--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Thanks, patch applied.

On Wed, Jul 3, 2013 at 8:44 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Thanks for the comments.
>
> Find the attached patch with below fix.
>
> --- Incomplete comments removed and added new comments
> --- s/Closed/Materialized/
> --- Added column "relkind" in outer query and removed the SELECT from the
> loop.
> --- Combined the below statement.
>      "(collection->GetConnection()->BackendMinimumVersion(9, 3))"
>
> Please let me know for further comments.
>
> Thanks,
> Neel Patel
>
>
> On Tue, Jul 2, 2013 at 8:06 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Hi
>>
>> On Tue, Jul 2, 2013 at 3:25 PM, Thom Brown <thom@linux.com> wrote:
>> > On 29 May 2013 10:40, Neel Patel <neel.patel@enterprisedb.com> wrote:
>> >> Hi Dave,
>> >>
>> >> Please find the updated patch after fixing some of the issues and
>> >> comments
>> >> given by Ashesh.
>> >
>> > Just realised this reply has been in my draft folder for over a month...
>> >
>> >
>> > This comment appears to be incomplete:
>> >
>> > //While editing the view, if it is materialized view then only change
>>
>> Yeah, please fix that Neel. Not sure what you were trying to say.
>>
>> >
>> > The section of if... else if... else if... that begins with:
>> >
>> > if (name == wxT("autovacuum_vacuum_cost_delay"))
>> >
>> > should probably use:
>> >
>> > switch(name)
>> > {
>> >     case wxT("autovacuum_vacuum_cost_delay")
>> >
>> >     case ...
>> > }
>>
>> wxString's don't play nicely with switch(), if memory serves.
>>
>> > Given the amount of duplicate functionality this shares with tables,
>> > is there not a way there could be a common class they could both
>> > inherit from to reduce maintenance overhead?
>>
>> Probably not a great deal of benefit. In any case, it would certainly
>> require a lot of refactoring which is far more work than we have time
>> for.
>>
>> --
>> Dave Page
>> Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Hi

On Tue, Jul 16, 2013 at 2:58 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Below is the bug description and find attached patch for fix.
>
>
> "If create any MATERIALIZED VIEW with double quote in SQL pane of
> pgAdminIII, the syntax is not displayed properly and it's showing create
> view instead of MATERIALIZED View, and user can not drop that and displayed
> use MATERIALIZED VIEW"
>
> Steps:
>
> 1. Install PostgreSQL 9.3 beta 2
> 2. Launch pgAdmin III
> 3. and create this view from SQL editor
> CREATE MATERIALIZED VIEW public."z a" AS
> select * from emp
> WITH DATA;
> 4. created successfully.
> 5. Click on SQL pane

Isn't the correct fix more like:

wxString sql = wxT("SELECT count(*) FROM pg_matviews WHERE matviewname
= ") + this->GetQuotedIdentifier() + wxT(" AND schemaname = ") +
this->GetSchema()->GetQuotedIdentifier();

The point being that GetQuotedIdentifier() should never need to be
passed through qtDbString(), and always knows the proper quoting rules
for that particular object class (not an issue in this case, but it
can be for things like functions, where the parentheses and arguments
should be excluded from the quoting).

--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

On Wed, Jul 17, 2013 at 12:23 PM, Neel Patel
<neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Just want to know.
>
> Now we are passing GetName() to qtDbString() and to form the complete query
> we need to use qtDbString() to compare the string.
>
> We have added below string.
>
> wxString sql = wxT("SELECT count(*) FROM pg_matviews WHERE matviewname = ")
> + qtDbString(this->GetName()) + wxT(" AND schemaname = ") +
> qtDbString(this->GetSchema()->GetName());
>
> Is it correct ?

I'd expect you to be using GetQuotedIdentifier() there. For a view and
schema, the identifier is the name anyway.


--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Hi

I realised this code is all wonky anyway - why is IsMaterializedView()
not just an accessor over a pgView private variable that is set in
pgViewFactory::CreateObjects when we create the object? We already
know at that point whether it's a matview or not (from
pg_class.relkind). Querying pg_class every time we want to check if
the view is materialized or not is horribly inefficient. There should
be just one function, that is a member of pgView and takes no
arguments. Whereever it's called, we just need to get the pgView
object and call it.

Please fix.

Thanks.

On Thu, Jul 18, 2013 at 5:58 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the updated patch with modification in query.
>
> Thanks,
> Neel Patel
>
>
> On Wed, Jul 17, 2013 at 5:15 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> On Wed, Jul 17, 2013 at 12:23 PM, Neel Patel
>> <neel.patel@enterprisedb.com> wrote:
>> > Hi Dave,
>> >
>> > Just want to know.
>> >
>> > Now we are passing GetName() to qtDbString() and to form the complete
>> > query
>> > we need to use qtDbString() to compare the string.
>> >
>> > We have added below string.
>> >
>> > wxString sql = wxT("SELECT count(*) FROM pg_matviews WHERE matviewname =
>> > ")
>> > + qtDbString(this->GetName()) + wxT(" AND schemaname = ") +
>> > qtDbString(this->GetSchema()->GetName());
>> >
>> > Is it correct ?
>>
>> I'd expect you to be using GetQuotedIdentifier() there. For a view and
>> schema, the identifier is the name anyway.
>>
>>
>> --
>> Dave Page
>> Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Thanks, patch applied.

On Fri, Jul 19, 2013 at 7:21 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>
> Thanks for the comment.
>
> We made the changes as you suggested.
>
> Please find the updated patch in attachment and let me know if you have any
> query.
>
> Thanks,
> Neel Patel
>
>
> On Thu, Jul 18, 2013 at 5:54 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Hi
>>
>> I realised this code is all wonky anyway - why is IsMaterializedView()
>> not just an accessor over a pgView private variable that is set in
>> pgViewFactory::CreateObjects when we create the object? We already
>> know at that point whether it's a matview or not (from
>> pg_class.relkind). Querying pg_class every time we want to check if
>> the view is materialized or not is horribly inefficient. There should
>> be just one function, that is a member of pgView and takes no
>> arguments. Whereever it's called, we just need to get the pgView
>> object and call it.
>>
>> Please fix.
>>
>> Thanks.
>>
>> On Thu, Jul 18, 2013 at 5:58 AM, Neel Patel <neel.patel@enterprisedb.com>
>> wrote:
>> > Hi Dave,
>> >
>> > Please find the updated patch with modification in query.
>> >
>> > Thanks,
>> > Neel Patel
>> >
>> >
>> > On Wed, Jul 17, 2013 at 5:15 PM, Dave Page <dave.page@enterprisedb.com>
>> > wrote:
>> >>
>> >> On Wed, Jul 17, 2013 at 12:23 PM, Neel Patel
>> >> <neel.patel@enterprisedb.com> wrote:
>> >> > Hi Dave,
>> >> >
>> >> > Just want to know.
>> >> >
>> >> > Now we are passing GetName() to qtDbString() and to form the complete
>> >> > query
>> >> > we need to use qtDbString() to compare the string.
>> >> >
>> >> > We have added below string.
>> >> >
>> >> > wxString sql = wxT("SELECT count(*) FROM pg_matviews WHERE
>> >> > matviewname =
>> >> > ")
>> >> > + qtDbString(this->GetName()) + wxT(" AND schemaname = ") +
>> >> > qtDbString(this->GetSchema()->GetName());
>> >> >
>> >> > Is it correct ?
>> >>
>> >> I'd expect you to be using GetQuotedIdentifier() there. For a view and
>> >> schema, the identifier is the name anyway.
>> >>
>> >>
>> >> --
>> >> Dave Page
>> >> Chief Architect, Tools & Installers
>> >> EnterpriseDB: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >>
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>



--
Dave Page
Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake