[Django] #27090: pg_get_serial_sequence is broken on postgres, use a lookup in information_schema.columns instead
Django
information_schema.columns instead
-------------------------------+-------------------------------
Reporter: hmpf | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Severity: Normal | Keywords: postgres sequence
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-------------------------------
pg_get_serial_sequence() is broken because it does '''not''' look up the
actual sequence in use for a table-column pair, it only concats the
tablename and columnname together to yield "tablename_columnname_seq",
which is what django used to do itself prior to postgres 8.1 or so.
To verify: manually alter which sequence to use for a table-column pair by
changing the default value for the column, then manually run "select
pg_get_serial_sequence(tablename, columname);". You will not get back the
sequence you just set, at least on postgres 9.3 and 9.5. If you try
pg_get_serial_sequence() on a child table utilizing postgres' table
inheritance you'll get a single row back with an empty string.
This is not a problem for databases created by django but turns up again
and again on legacy systems. It also means existing django-built columns
cannot switch to a different sequence.
The following methods in
django.db.backends.postgresql_psycopg2.base.DatabaseOperations use the
postgres function pg_get_serial_sequence() to fetch a sequence name:
* last_insert_id
* sequence_reset_by_name_sql
* sequence_reset_sql
I use the function in the attached patch in a fork of the postgres-backend
instead. It looks up the sequence name that is actually in use in the
"information_schema.columns"-table. The SQL's been tested on postgres 9.3
and 9.5, which is what I currently have available.
It would be nice if postgres would fix pg_get_serial_sequence() but we
don't have to wait for that.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
information_schema.columns instead
-------------------------------+----------------------------
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Keywords: postgres sequence | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------+----------------------------
* Attachment "sequence_name_getter.py" added.
Python-function looking up sequence_name in postgres
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Keywords: postgres sequence | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hmpf):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> pg_get_serial_sequence() is broken because it does '''not''' look up the
> actual sequence in use for a table-column pair, it only concats the
> tablename and columnname together to yield "tablename_columnname_seq",
> which is what django used to do itself prior to postgres 8.1 or so.
>
> To verify: manually alter which sequence to use for a table-column pair
> by changing the default value for the column, then manually run "select
> pg_get_serial_sequence(tablename, columname);". You will not get back the
> sequence you just set, at least on postgres 9.3 and 9.5. If you try
> pg_get_serial_sequence() on a child table utilizing postgres' table
> inheritance you'll get a single row back with an empty string.
>
> This is not a problem for databases created by django but turns up again
> and again on legacy systems. It also means existing django-built columns
> cannot switch to a different sequence.
>
> The following methods in
> django.db.backends.postgresql_psycopg2.base.DatabaseOperations use the
> postgres function pg_get_serial_sequence() to fetch a sequence name:
>
> * last_insert_id
> * sequence_reset_by_name_sql
> * sequence_reset_sql
>
> I use the function in the attached patch in a fork of the postgres-
> backend instead. It looks up the sequence name that is actually in use in
> the "information_schema.columns"-table. The SQL's been tested on postgres
> 9.3 and 9.5, which is what I currently have available.
>
> It would be nice if postgres would fix pg_get_serial_sequence() but we
> don't have to wait for that.
New description:
pg_get_serial_sequence() is broken because it does '''not''' look up the
actual sequence in use for a table-column pair, it only concats the
tablename and columnname together to yield "tablename_columnname_seq",
which is what django used to do itself prior to postgres 8.1 or so.
To verify: manually alter which sequence to use for a table-column pair by
changing the default value for the column, then manually run "select
pg_get_serial_sequence(tablename, columname);". You will not get back the
sequence you just set, at least on postgres 9.3 and 9.5. If you try
pg_get_serial_sequence() on a child table utilizing postgres' table
inheritance you'll get a single row back with an empty string.
This is not a problem for databases created by django but turns up again
and again on legacy systems. It also means existing django-built columns
cannot switch to a different sequence.
The following methods in
django.db.backends.postgresql_psycopg2.base.DatabaseOperations use the
postgres function pg_get_serial_sequence() to fetch a sequence name:
* last_insert_id
* sequence_reset_by_name_sql
* sequence_reset_sql
I use the function in the attached file in a fork of the postgres-backend
instead. It looks up the sequence name that is actually in use in the
"information_schema.columns"-table. The SQL's been tested on postgres 9.3
and 9.5, which is what I currently have available.
It would be nice if postgres would fix pg_get_serial_sequence() but we
don't have to wait for that.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:1>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Keywords: postgres sequence | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I don't have expertise here. Can you submit a pull request to verify this
doesn't break the Django test suite? Is it possible to write a regression
test?
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:2>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Keywords: postgres sequence | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by hmpf):
I need this for older django versions so I'll be making a standalone
package regardless, with tests.
I'm not too experienced in making tests that depend on a specific database
though, so a pull request will not be forthoming before 1) I'm content
with the tests in the package 2) I have a working test-setup for Django
itself.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:3>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: postgres sequence | Triage Stage:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* component: Uncategorized => Database layer (models, ORM)
* type: Uncategorized => Cleanup/optimization
Comment:
Is there an open issue to correct the behavior in PostgreSQL? I found
[https://www.postgresql.org/message-
id/flat/4800.1445898830%40sss.pgh.pa.us#4800.14...@sss.pgh.pa.us
#13735] but the reply looks like the behavior is expected (again, I'm not
an expert on PG behavior, so I may have misunderstood).
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:4>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I also found some possibly related Django tickets (e.g.
ticket:8901#comment:25) that were closed as a duplicate of #13295. Maybe
that would address this?
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:5>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by hmpf):
Replying to [comment:4 timgraham]:
> Is there an open issue to correct the behavior in PostgreSQL? I found
[https://www.postgresql.org/message-
id/flat/4800.1445898830%40sss.pgh.pa.us#4800.14...@sss.pgh.pa.us
#13735] but the reply looks like the behavior is expected (again, I'm not
an expert on PG behavior, so I may have misunderstood).
That something works as documented doesn't necessarily mean that it is
useful :)
Interesting that it checks owner, with an inherited table only the
ancestor can be the owner of an inherited sequence so
pg_get_serial_sequence() cannot ever work on them as it is defined today.
I'm guessing what would be useful is a new postgres function that always
picks the currently in use sequence.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:6>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by hmpf):
Discovered during testing: the standard Django TestCase does not zero out
sequences on teardown/end of test-method/between classes, which
complicates matters.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:7>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by hmpf):
Replying to [comment:5 timgraham]:
> I also found some possibly related Django tickets (e.g.
ticket:8901#comment:25) that were closed as a duplicate of #13295. Maybe
that would address this?
That would also solve the problem but I think the solution I'm working on
involves less code. On the other hand, explicit is better than implicit...
This ticket would make more postgres databases just work out of the box.
Ticket #13295 could use the lookup in information_schema as part of its
own solution. Seen that way, #13295 depends on this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:8>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by hmpf):
Link to code of current standalone backend: https://github.com/hmpf
/django-postgres-backend
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:9>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Could you open a PostgreSQL feature request and get confirmation that your
proposed solution is the best way to do introspection in the meantime?
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:10>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by cjerdonek):
Another thought: more generally, it might be good to think about whether
the backends can be made more friendly to subclassing for cases like this.
That way the whole backend doesn't need to be forked just to change one
aspect. It's also good design in general.
I don't whether this is already easy or doable for the case at hand. It
would also reduce the importance / need for Django to "take a stance" in
cases where the best implementation is unknown or unclear, etc.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:11>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by cjerdonek):
* cc: chris.jerdonek@… (added)
Comment:
I'm adding this comment only to let me to add myself as a CC.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:12>
Django
information_schema.columns instead
Reporter: hmpf | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Someday/Maybe
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:13>
Django
information_schema.columns instead
Reporter: Hanne Moa | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hanne Moa):
The postgres-general list has some very interesting spam filters, took a
while to get through. (Don't have a line in the body of the email starting
with the word "which"...)
https://www.postgresql.org/message-id/nu7ebq%24a2h%241%40blaine.gmane.org
A different query is suggested, the result need not be parsed:
"Personally, I'd try looking in pg_depend to see if the column's default
expression has a dependency on a relation of type sequence. That avoids
all the fun of parsing the expression and turns it into a simple SQL
join problem." -- Tom Lane
It's a very simple join =):
{{{
select sn.nspname as sequence_schema, s.relname as sequence_name,
col.attname
from pg_class s
join pg_namespace sn on sn.oid = s.relnamespace
join pg_depend d on d.refobjid = s.oid and
d.refclassid='pg_class'::regclass
join pg_attrdef ad on ad.oid = d.objid and d.classid =
'pg_attrdef'::regclass
join pg_attribute col on col.attrelid = ad.adrelid and col.attnum =
ad.adnum
join pg_class tbl on tbl.oid = ad.adrelid
join pg_namespace n on n.oid = tbl.relnamespace
where s.relkind = 'S'
and d.deptype in ('a', 'n')
and n.nspname = 'public'
and tbl.relname = 'THE_TABLE_NAME'
and col.attname = 'THE_COLUMN_NAME'
}}}
Anyway, even using the information_schema, it might return a schema in
addition to the name so adjustments need to be made to my existing code.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:14>
Django
information_schema.columns instead
Reporter: Hanne Moa | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hanne Moa):
Tom Lane's suggestion is what we will be going with.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:15>
Django
information_schema.columns instead
Reporter: Hanne Moa | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
I think the first step is to add real sequence introspection support.
[https://github.com/django/django/pull/8395 This PR] (missing Oracle) does
that. Then we should be able to build on it to fix this ticket's issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:16>
Django
information_schema.columns instead
Reporter: Hanne Moa | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"c6a1faecc3d6e25597a7105df74f11678f2f2aac" c6a1fae]:
{{{
#!CommitTicketReference repository=""
revision="c6a1faecc3d6e25597a7105df74f11678f2f2aac"
Refs #27090 -- Added real database sequence introspection.
Thanks Mariusz Felisiak for the Oracle part and Tim Graham for the
review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:17>
Django
information_schema.columns instead
Reporter: Hanne Moa | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
I'm trying to create a failing test, but on the PostgreSQL 9.4 and 9.6
instances I'm using to test, the `pg_get_serial_sequence` function does
return the real sequence name even after renaming the sequence with `ALTER
SEQUENCE ... RENAME TO`.
I would be interested to get more feedback from other testers...
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:18>
Django
information_schema.columns instead
Reporter: Hanne Moa | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 1.10
Severity: Normal | worksforme
Keywords: postgres sequence | Triage Stage:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* status: new => closed
* resolution: => worksforme
Comment:
Closing per absence of reaction after my latest comment. Reopen if you can
reproduce with recent versions.
--
Ticket URL: <https://code.djangoproject.com/ticket/27090#comment:19>