Opened 16 years ago

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

Download all attachments as: .zip

Change History (24)

Changed 16 years ago by John Shaffer <jshaffer2112@…>

Attachment: add-quote_name.diff added

comment:1 Changed 16 years ago by John Shaffer <jshaffer2112@…>

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

Changed 16 years ago by John Shaffer <jshaffer2112@…>

comment:2 Changed 16 years ago by John Shaffer <jshaffer2112@…>

Cc: jshaffer2112@… added

Changed 16 years ago by John Shaffer <jshaffer2112@…>

Attachment: add-quote-name_r5678.diff added

Patch against [5678].

comment:3 Changed 16 years ago by John Shaffer <jshaffer2112@…>

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
    

Changed 16 years ago by John Shaffer <jshaffer2112@…>

Attachment: ticket-4492_r5743.diff added

Patch against [5743], with tests.

comment:4 Changed 16 years ago by Simon G. <dev@…>

#4557 sounds like a similar problem.

comment:5 Changed 16 years ago by John Shaffer <jshaffer2112@…>

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

Changed 16 years ago by John Shaffer <jshaffer2112@…>

Attachment: add-quote_name_r5988.diff added

Patch against [5988].

comment:6 Changed 16 years ago by Michael Radziej

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 Changed 16 years ago by Michael Radziej

Triage Stage: Ready for checkinUnreviewed

reverting my accidental state change.

comment:8 Changed 16 years ago by Chris Beaven

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 Changed 16 years ago by John Shaffer

Cc: jshaffer2112@… removed

Changed 15 years ago by John Shaffer

Attachment: ticket-4492_r7583.diff added

Patch against [7583].

comment:10 Changed 15 years ago by John Shaffer

Patch needs improvement: unset

comment:11 Changed 14 years ago by Adam Nelson

Patch needs improvement: set

Patch needs to be updated again.

comment:12 Changed 13 years ago by Gabriel Hurley

Severity: Normal
Type: Cleanup/optimization

comment:13 Changed 12 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:14 Changed 12 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:15 Changed 10 years ago by Tim Graham

Easy pickings: set

comment:16 Changed 10 years ago by velis

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 Changed 10 years ago by velis

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 10 years ago by velis (previous) (diff)

comment:18 Changed 10 years ago by Tim Graham

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