On Tue, Mar 8, 2016 at 4:34 PM, Ashesh Vashi <ashesh.va...@enterprisedb.com> wrote:
> 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. > OK, I added tasks to do both to our internal tracker. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
