Opened 17 years ago

Closed 11 years ago

#4492 closed Cleanup/optimization (fixed)

Provide tests for mixed-case column names

Reported by: John Shaffer <jshaffer2112@…> Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: postgresql psycopg
Cc: mir@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

Django generates this query, which gives this error:

SELECT setval('"product_item_relatedItems_id_seq"', (SELECT max("id") FROM product_item_relatedItems));

psycopg.ProgrammingError: ERROR:  relation "product_item_relateditems" does not exist

I'm attaching a patch to generate this query instead:

SELECT setval('"product_item_relatedItems_id_seq"', (SELECT max("id") FROM "product_item_relatedItems"));

Attachments (6)

add-quote_name.diff (593 bytes ) - added by John Shaffer <jshaffer2112@…> 17 years ago.
add-quote_name-psycopg2.diff (565 bytes ) - added by John Shaffer <jshaffer2112@…> 17 years ago.
add-quote-name_r5678.diff (1.1 KB ) - added by John Shaffer <jshaffer2112@…> 17 years ago.
Patch against [5678].
ticket-4492_r5743.diff (3.4 KB ) - added by John Shaffer <jshaffer2112@…> 17 years ago.
Patch against [5743], with tests.
add-quote_name_r5988.diff (2.9 KB ) - added by John Shaffer <jshaffer2112@…> 17 years ago.
Patch against [5988].
ticket-4492_r7583.diff (2.3 KB ) - added by John Shaffer 16 years ago.
Patch against [7583].

Download all attachments as: .zip

Change History (24)

by John Shaffer <jshaffer2112@…>, 17 years ago

Attachment: add-quote_name.diff added

comment:1 by John Shaffer <jshaffer2112@…>, 17 years ago

I noticed that the psycopg2 backend has the same issue, so here is an additional patch.

by John Shaffer <jshaffer2112@…>, 17 years ago

comment:2 by John Shaffer <jshaffer2112@…>, 17 years ago

Cc: jshaffer2112@… added

by John Shaffer <jshaffer2112@…>, 17 years ago

Attachment: add-quote-name_r5678.diff added

Patch against [5678].

comment:3 by John Shaffer <jshaffer2112@…>, 17 years ago

To reproduce:

  • Create a model with a mixed-case ManyToManyField:
    class Item(models.Model):
        relatedItems = models.ManyToManyField('self', blank=True, null=True)
    
  • Load a fixture:
    - fields:
        relatedItems: []
      model: product.item
      pk: '1'
    
  • You'll get a backtrace here:
    psycopg2.ProgrammingError: relation "product_item_relateditems" does not exist
    

by John Shaffer <jshaffer2112@…>, 17 years ago

Attachment: ticket-4492_r5743.diff added

Patch against [5743], with tests.

comment:4 by Simon G. <dev@…>, 17 years ago

#4557 sounds like a similar problem.

comment:5 by John Shaffer <jshaffer2112@…>, 17 years ago

Summary: SELECT fails for mix-cased column name using postgresqlquote_name missing in postgresql backends

by John Shaffer <jshaffer2112@…>, 17 years ago

Attachment: add-quote_name_r5988.diff added

Patch against [5988].

comment:6 by Michael Radziej, 17 years ago

Cc: mir@… added
Triage Stage: UnreviewedReady for checkin

John, can you bring this up on the developer mailing list? Everything looks good to me, but I don't have the expertise needed here, so I hesitate to put this into "ready for checkin".

I wouldn't mind anybody to advance the stage of this ticket if they think so.

comment:7 by Michael Radziej, 17 years ago

Triage Stage: Ready for checkinUnreviewed

reverting my accidental state change.

comment:8 by Chris Beaven, 17 years ago

Patch needs improvement: set
Summary: quote_name missing in postgresql backendsProvide tests for mixed-case column names
Triage Stage: UnreviewedAccepted

This has actually been fixed in [6507] in response to #5710, but it didn't have tests in that one. It would be good to get your test in John, so could you please provide an updated patch? Thanks

comment:9 by John Shaffer, 17 years ago

Cc: jshaffer2112@… removed

by John Shaffer, 16 years ago

Attachment: ticket-4492_r7583.diff added

Patch against [7583].

comment:10 by John Shaffer, 16 years ago

Patch needs improvement: unset

comment:11 by Adam Nelson, 15 years ago

Patch needs improvement: set

Patch needs to be updated again.

comment:12 by Gabriel Hurley, 14 years ago

Severity: Normal
Type: Cleanup/optimization

comment:13 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:14 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:15 by Tim Graham, 11 years ago

Easy pickings: set

comment:16 by velis, 11 years ago

I just want to clarify: it says tests are needed, but the appropriate checkbox is not ticked. So, is it the tests that are needed or is there something more to this ticket?
p.s.: I'm sprinting django on PyCon UK and I'm looking at this ticket because I have dealt with this problem before. So I know what the issue was, I can write the appropriate tests, but I also want to make it right so that my first contribution wouldn't end up a failure :)

comment:17 by velis, 11 years ago

https://github.com/django/django/pull/1667

Please review

I created a model for testing mixed case entity names + a suite of 4 tests that test that. Since PostgreSQL lowercases everything not in quotes, usually only the last test would fail if this isn't implemented properly in the drivers.
Tested the unit-tests on PostgreSQL 9.2 and SQLite

Last edited 11 years ago by velis (previous) (diff)

comment:18 by Tim Graham, 11 years ago

Resolution: fixed
Status: newclosed

On further investigation, I think we have tests to cover this. In particular, if I comment out the fix from [6507], I get a failure in test_generic_relation (backends.tests.SequenceResetTest), and here are some models with mixed case column names:

aggregation_regress/models.py:    Entry = models.CharField(unique=True, max_length=50)
aggregation_regress/models.py:    Clue = models.CharField(max_length=150)
Note: See TracTickets for help on using tickets.
Back to Top