MIME-Version: 1.0
In-Reply-To: 
 <CAA-aLv7fzZnxRhwV6umxhA-k52YekWyi7t1F34aPpUXR+41tjg@mail.gmail.com>
References: 
 <CAKKotZRCf07F_SGHi4KawBtLHPSeKz5Uvzwu6ddQ=bKc1FDRRw@mail.gmail.com>
 <CAFiP3vw61T0n--F4vYXALdtrkBKznBegBPq5Ds=6uZrg02vWXQ@mail.gmail.com>
 <CAA-aLv7fzZnxRhwV6umxhA-k52YekWyi7t1F34aPpUXR+41tjg@mail.gmail.com>
From: Thom Brown <thom@linux.com>
Date: Wed, 27 Apr 2016 14:34:57 +0100
Message-ID: 
 <CAA-aLv4z0RcdDvYLtQFH1JgWH2pdPizW9Fa+XaOSTQYQgZeLzg@mail.gmail.com>
Subject: Re: [PATCH] Tables node (pgAdmin4)
To: Harshal Dhumal <harshal.dhumal@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>,
	Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com>,
	Surinder Kumar <surinder.kumar@enterprisedb.com>
Content-Type: text/plain; charset=UTF-8
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

On 27 April 2016 at 13:43, Thom Brown <thom@linux.com> wrote:
> On 27 April 2016 at 10:22, Harshal Dhumal
> <harshal.dhumal@enterprisedb.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.
>
> 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.

A couple more:

If I go to create a new table, give it a name, select a table to
inherit from, and then do nothing else, I get an error:

ERROR: syntax error at or near "INHERITS"
LINE 2: INHERITS (test)
^

This is because no column list was provided, and in this case, an
empty one would be needed.  So instead of:

CREATE TABLE public.test2
(
)
    INHERITS (test)
WITH (
    OIDS = FALSE
)
TABLESPACE pg_default;

It should be:

CREATE TABLE public.test2
    INHERITS (test)
WITH (
    OIDS = FALSE
)
TABLESPACE pg_default;


If I go to create another new table, give it a name, and then in the
Like section on the Advance tab, I select a table, and enable each of
the With * options, the following SQL is produced:

CREATE TABLE public.test2
    LIKE public.test
        INCLUDING DEFAULTS
        INCLUDING CONSTRAINTS
        INCLUDING INDEXES
        INCLUDING STORAGE
        INCLUDING COMMENTSWITH (
    OIDS = FALSE
)
TABLESPACE pg_default;

The LIKE statement should appear in the column list section, so should
be surrounded in parentheses.  Also, the WITH statement here is not on
a new line.

So this should be:

CREATE TABLE public.test2
(
    LIKE public.test
        INCLUDING DEFAULTS
        INCLUDING CONSTRAINTS
        INCLUDING INDEXES
        INCLUDING STORAGE
        INCLUDING COMMENTS
)
WITH (
    OIDS = FALSE
)
TABLESPACE pg_default;

Regards

Thom


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