On Tue, Mar 8, 2016 at 9:23 PM, Dave Page <dp...@pgadmin.org> wrote: > On Tue, Mar 8, 2016 at 2:46 PM, Ashesh Vashi > <ashesh.va...@enterprisedb.com> wrote: > > On Tue, Mar 8, 2016 at 8:08 PM, Dave Page <dp...@pgadmin.org> wrote: > >> > >> > >> > >> On Tue, Mar 8, 2016 at 2:18 PM, Ashesh Vashi > >> <ashesh.va...@enterprisedb.com> wrote: > >>> > >>> On Tue, Mar 8, 2016 at 7:36 PM, Ashesh Vashi > >>> <ashesh.va...@enterprisedb.com> wrote: > >>>> > >>>> Hi Dave, > >>>> > >>>> On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi > >>>> <ashesh.va...@enterprisedb.com> wrote: > >>>>> > >>>>> Hi Dave, > >>>>> > >>>>> > >>>>> On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dp...@pgadmin.org> wrote: > >>>>>> > >>>>>> Hi > >>>>>> > >>>>>> On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi > >>>>>> <ashesh.va...@enterprisedb.com> wrote: > >>>>>>> > >>>>>>> Hi Dave, > >>>>>>> > >>>>>>> As discussed, I have worked on this patch to improve some code > level > >>>>>>> changes. > >>>>>>> (because - Murtuza was not available due to some other engagement.) > >>>>>>> > >>>>>>> Can you please review it, and share your feedback? > >>>>>> > >>>>>> > >>>>>> I think it's mostly there. I've attached an updated patch where > I've > >>>>>> fixed a few minor issues as I read through the code. The following > (likely > >>>>>> simple) issues need to be fixed: > >>>>>> > >>>>>> - Dropping a schema fails with an error message of > >>>>>> "schema/pg/9.2_plus/sql/get_name.sql". > >>>>> > >>>>> Done. > >>>>>> > >>>>>> > >>>>>> - Creating a schema appears to fail with "'data' is undefined", > >>>>>> however the schema is created, it's just that the dialogue doesn't > close and > >>>>>> the new schema isn't added to the tree. > >>>>> > >>>>> Done. > >>>>>> > >>>>>> > >>>>>> - There is some discrepancy between default privileges as displayed > on > >>>>>> the properties summary, the edit dialogue, and the RE-SQL. As you > can see in > >>>>>> the screenshot, the SQL just GRANTS ALL, and the properties panel > doesn't > >>>>>> show anything. > >>>>> > >>>>> Yes - there were some typos in the schema/catalog node > implementation, > >>>>> which I have resolved now. > >>>>> > >>>>> Please find the updated patch. > >>>> > >>>> One more updated patch: > >>>> Some of the catalogs will not have all the schema child objects. > >>>> Hence - they will need to check certain thing likes they're not being > >>>> loading in the catalog with such property (i.e. pg_catalog). > >>> > >>> As per my conversation with Murtuza, who has already implemented > >>> catalog_obejcts for this kind of catalogs, these objects are only > supported > >>> for catalogs like information_schema (and, PPAS specific dbo, sys). > >>>> > >>>> To ease the work, I have introduced a class name SchemaChildModule, > >>>> which does that job for us. > >>> > >>> Please find the patch as per his input. > >>> > >> > >> Can you split out the new changes please? I just spent 30 minutes > tweaking > >> the last patch. > > > > Sure. > > Here is the patch based on the v10 patch. > > Thanks - patch committed. I made the following changes: > > - Removed explicit support for 9.0 and below. > - Hid the default ACLs from the properties list for catalogs. > - Tidied up some of the SQL formatting > > Open questions: > > - We don't allow default ACLs to be specified when creating a schema > (neither does pgAdmin). Why not? Shouldn't we? > Hmm.. I don't see any reason, why we should not do it. We have adopted that from pgAdmin III.
> > - We create new objects in 2 SQL statements, one that runs create.sql > and one that runs alter.sql to apply ACL, label options and more. I > strongly believe we need to push this into a single statement for all > object types, to ensure creation is completely atomic. Right now, you > can easily get an error by trying to set an unregistered security > label, which keeps the create dialogue open, however the object has > been successfully created. > Agree. Even if they needed to be created from two, or more separate templates, they should ran together unless there are some statements, which require to run in separate transaction. -- Thanks & Regards, Ashesh Vashi EnterpriseDB INDIA: Enterprise PostgreSQL Company <http://www.enterprisedb.com/> *http://www.linkedin.com/in/asheshvashi* <http://www.linkedin.com/in/asheshvashi> > > Thoughts? > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
