Re: [pgAdmin4][Patch] - RM 4030 - IDENTITY column not recognised

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch] - RM 4030 - IDENTITY column not recognised
Date: 2019-03-28 17:17:09
Message-ID: CA+OCxoyg8FcMTt96XKQabCvkK4YBzZmZzm3KNYChH1nLWtGMng@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, patch applied!

One thing I did notice - Sequences are missing the system object property.
I've re-opened #1257

On Thu, Mar 28, 2019 at 5:17 AM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
>
> On Thu, Mar 21, 2019 at 3:57 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Thu, Mar 21, 2019 at 9:52 AM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Wed, Mar 20, 2019 at 8:37 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please find the attached patch to fix the RM #4030 - IDENTITY column
>>>>> not recognised.
>>>>> - Added support for IDENTITY column for PostgreSQL >= 10.0
>>>>>
>>>>
>>>> A few issues unfortunately:
>>>>
>>>> - There's an extra space in the generated SQL for an identity column on
>>>> a table, right before the end comma (in both the CREATE and reverse
>>>> engineered SQL.
>>>>
>>>> - I cannot make an IDENTITY column a primary key through the UI, nor
>>>> does it reverse-engineer that property in the SQL if I create it via SQL
>>>> (it does properly set the switch value though).
>>>>
>>> This issue has already been logged earlier, but I will fix this with
>>> this patch.
>>>
>> Fixed
>
>>
>>>> - After creating an IDENTITY column, there should be a dependency on
>>>> the sequence, but I don't see this listed.
>>>>
>>> If the *Show System Object* is enabled, then only you can see.
>>>
>>
>> Hmm, OK. I turned on "Show System Objects" and I do now see the
>> dependencies. Something seems funky though:
>>
>> - The sequence has a dependency of the column
>> - The sequence is dependent on the table
>>
>> Shouldn't they both be dependencies? It's the table and column that needs
>> the sequence.
>>
>> - The sequence is only listed as a dependent of the column (which is the
>> opposite of what is seen on the sequence, as you'd expect), but it doesn't
>> show that it's dependent on the table, in the same way that a table lists a
>> schema as a dependency.
>>
>> Is our SQL messed up here, or is PostgreSQL listing things in a funky
>> way? I'd expect to see:
>>
>> When clicking on the sequence:
>>
>> - Dependencies should list the schema.
>> - Dependents should list the table and column.
>>
>> If we check the pg_depend for a sequence,
> select * from pg_depend where objid=36946, here objid = {seqid}
> then, the output is
>
> *classid*
>
> *objid*
>
> *objsubid*
>
> *refclassid*
>
> *refobjid*
>
> *refobjsubid*
>
> *deptype*
>
> *1259*
>
> 36946
>
> 0
>
> 2615
>
> 2200
>
> 0
>
> n
>
> *1259*
>
> 36946
>
> 0
>
> 1259
>
> 36897
>
> 3
>
> i
>
> So, it shows the schema, table, and column as its dependency.
> We display only schema and column but we can also consider the table in
> this case.
>
> As per you, Dependents should display table and column, do you still
> think, we are doing something wrong?
>
> When clicking on the column:
>>
>> - Dependencies should list the sequence.
>>
>> - Dependents should list the table.
>>
>> I grant you it's confusing and open to interpretation though. I think as
>> long as we're definitely listing things as PG tracks them, it's all good.
>>
>>
>>>
>>>> - We should consider the auto-created sequence a system object, and
>>>> hide it in the treeview by default as it's an implementation detail.
>>>>
>>> How, can I identify those as a system object? I tried but couldn't find
>>> a way.
>>>
>>
>> Check if there's a dependency on a column.
>>
>>
>>>
>>>> - If I click on an IDENTITY column in the treeview, the
>>>> reverse-engineered SQL just shows the plain datatype.
>>>>
>>> Fixed.
>>>
>> Fixed
>
>>
>>>> - Can we reasonably support the sequence_options clause?
>>>>
>>> I will look into it.
>>>
>>
>> Done
>
>> Thanks.
>>
>>
>>>
>>>> Thanks.
>>>>
>>>> Thanks,
>>> Khushboo
>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2019-03-28 18:21:00 pgAdmin 4 commit: PEP-8 fixes.
Previous Message Dave Page 2019-03-28 17:13:50 pgAdmin 4 commit: Add support for IDENTITY columns. Fixes #4030