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

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
>