Re: [PATCH] Tables node (pgAdmin4)

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Thom Brown <thom(at)linux(dot)com>
Cc: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>, Arun Kollan <arun(dot)kollan(at)enterprisedb(dot)com>
Subject: Re: [PATCH] Tables node (pgAdmin4)
Date: 2016-04-28 08:52:59
Message-ID: 921AFBF6-CB51-4898-B5F5-73CE38311399@pgadmin.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

> On 27 Apr 2016, at 13:43, Thom Brown <thom(at)linux(dot)com> wrote:
>
> On 27 April 2016 at 10:22, Harshal Dhumal
> <harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>
>> Hi,
>>
>> PFA attached patches for table node and all table child nodes.
>>
>> This patch includes below nodes,
>>
>> 1) Table node -- Initial patch by Murtuza, constraints compatibility by Harshal.
>> 2) Column node -- by Murtuza.
>> 3) Index node -- by Murtuza.
>> 4) Trigger node -- by Murtuzz.
>> 6) Rules node -- by Surinder.
>> 7) Constraints nodes:
>> i] Index Constraint -- Initial patch by Harshal, Integration with table node by Murtuza.
>> ii] Foreign key -- Initial patch and Integration with table node by Harshal.
>> iii] Check constraint -- Initial patch and Integration with table node by Harshal.
>> iv] Exclusion constraint -- Initial patch and Integration with table node by Harshal.
>>
>> Please apply patches in following order as all of them depends on each other.
>>
>> Order: Table Node ----> Index constraint ---> remaining patches in any order.
>
> Nice work. Here's some initial feedback from a very quick play around.
>
> On the Create table editor, in the Advance tab (which should probably
> be labelled "Advanced"), the Like section should grey out the "With *"
> values if no relation is selected in the drop-down box.
>
> The way primary keys are defined are kinda awkward. It might be
> useful to provide some kind of checkbox on the initial column list
> that tells it which columns are involved in the primary key, then the
> user could just select which ones they want. If they want to refine
> it, they could edit it in the Constraints > Primary Key section.

If the design we did has been properly followed (I can't check right now), then that's exactly how it should be working. Harshal?

Arun Kollan and I spent a lot of time redesigning the table dialogue to make it as quick and efficient to use as possible. There will likely be minor deviations from that design to ensure consistency with the way other parts of the app have turned out, but the basic principles should be there.

>
> I'm getting weird spacing in the SQL output. Here's an example:
>
> CREATE UNLOGGED TABLE public.test
> (
> id integer COLLATE pg_catalog."de_DE.utf8" NOT NULL DEFAULT -1,
> stuff text COLLATE pg_catalog."C.UTF-8" DEFAULT "hello",
> CONSTRAINT pk PRIMARY KEY (id, stuff) WITH (FILLFACTOR=33) DEFERRABLE
> )
> WITH (
> OIDS = TRUE,
> FILLFACTOR = 88,
> autovacuum_enabled = TRUE,
> autovacuum_analyze_scale_factor = 0.33,
> autovacuum_analyze_threshold = 30,
> autovacuum_freeze_max_age = 3333333,
> autovacuum_vacuum_cost_delay = 30,
> autovacuum_vacuum_cost_limit = 3,
> autovacuum_vacuum_scale_factor = 0.33,
> autovacuum_vacuum_threshold = 33,
> autovacuum_freeze_min_age = 3300000,
> autovacuum_freeze_table_age = 333000000
> )
> TABLESPACE pg_default;
>
> ALTER TABLE public.test
> OWNER to thom;
> GRANT ALL ON TABLE public.test TO thom;
>
>
> COMMENT ON TABLE public.test
> IS 'This is just a test table';
>
> COMMENT ON COLUMN public.test.id
> IS 'the main ID';
>
> ALTER TABLE public.test
> ALTER COLUMN id
> SET (n_distinct='0.2');
> COMMENT ON CONSTRAINT pk ON public.test
> IS 'primary key test'
>
> Note there are 2 blank lines after the GRANT ALL ON TABLE line, and
> none before the COMMENT ON CONSTRAINT line.
>
> This SQL fails because collations aren't allowed on integer columns,
> and the DEFAULT value for the column named stuff doesn't quote it as a
> string literal, so it's looking for a column called "hello".
>
> There's also no way to view the autovacuum options I defined other
> than the SQL pane.
>
> Thom
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2016-04-28 10:06:11 Re: [PATCH] Tables node (pgAdmin4)
Previous Message Thom Brown 2016-04-28 08:44:00 Re: [PATCH] Tables node (pgAdmin4)