Обсуждение: [pgAdmin4][Patch] - RM 4030 - IDENTITY column not recognised
Hi,
Please find the attached patch to fix the RM #4030 - IDENTITY column not recognised.
- Added support for IDENTITY column for PostgreSQL >= 10.0
Thanks,
Khushboo
Вложения
Hi
On Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <khushboo.vashi@enterprisedb.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).
- After creating an IDENTITY column, there should be a dependency on the sequence, but I don't see this listed.
- We should consider the auto-created sequence a system object, and hide it in the treeview by default as it's an implementation detail.
- If I click on an IDENTITY column in the treeview, the reverse-engineered SQL just shows the plain datatype.
- Can we reasonably support the sequence_options clause?
Thanks.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi Dave,
On Wed, Mar 20, 2019 at 8:37 PM Dave Page <dpage@pgadmin.org> wrote:
HiOn Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <khushboo.vashi@enterprisedb.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.0A 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.
- 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.
- 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.
- If I click on an IDENTITY column in the treeview, the reverse-engineered SQL just shows the plain datatype.
Fixed.
- Can we reasonably support the sequence_options clause?
I will look into it.
Thanks.
Thanks,
Khushboo
--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi
On Thu, Mar 21, 2019 at 9:52 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi Dave,On Wed, Mar 20, 2019 at 8:37 PM Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <khushboo.vashi@enterprisedb.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.0A 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.- 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.
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.- Can we reasonably support the sequence_options clause?I will look into it.
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
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Mar 21, 2019 at 3:22 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi Dave,On Wed, Mar 20, 2019 at 8:37 PM Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <khushboo.vashi@enterprisedb.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.0A 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.- 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.- 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.- If I click on an IDENTITY column in the treeview, the reverse-engineered SQL just shows the plain datatype.Fixed.- Can we reasonably support the sequence_options clause?I will look into it.
To show the sequence option, we have 2 options.
We can provide a simple textbox or all the options listed with textboxes same as Sequence dialogue.
So, which one is preferable?
Thanks.Thanks,Khushboo--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Mon, Mar 25, 2019 at 2:18 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Thu, Mar 21, 2019 at 3:22 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi Dave,On Wed, Mar 20, 2019 at 8:37 PM Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <khushboo.vashi@enterprisedb.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.0A 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.- 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.- 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.- If I click on an IDENTITY column in the treeview, the reverse-engineered SQL just shows the plain datatype.Fixed.- Can we reasonably support the sequence_options clause?I will look into it.To show the sequence option, we have 2 options.We can provide a simple textbox or all the options listed with textboxes same as Sequence dialogue.So, which one is preferable?
Much as I hate to add to the workload, this is a GUI designed to prevent the need to know the SQL syntax, so it's got to be individual listing of the options I think.
Thanks.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi Dave,
Please find the attached updated patch.
Thanks,
Khushboo
On Thu, Mar 21, 2019 at 3:57 PM Dave Page <dpage@pgadmin.org> wrote:
HiOn Thu, Mar 21, 2019 at 9:52 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi Dave,On Wed, Mar 20, 2019 at 8:37 PM Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <khushboo.vashi@enterprisedb.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.0A 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 tableShouldn'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
Вложения
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.vashi@enterprisedb.com> wrote:
Hi Dave,Please find the attached updated patch.Thanks,KhushbooOn Thu, Mar 21, 2019 at 3:57 PM Dave Page <dpage@pgadmin.org> wrote:HiOn Thu, Mar 21, 2019 at 9:52 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:Hi Dave,On Wed, Mar 20, 2019 at 8:37 PM Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Mar 20, 2019 at 7:30 AM Khushboo Vashi <khushboo.vashi@enterprisedb.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.0A 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 tableShouldn'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.DoneThanks.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
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company