Code

Opened 7 years ago

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

Download all attachments as: .zip

Change History (24)

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

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

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

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

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

  • Cc jshaffer2112@… added

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

Patch against [5678].

comment:3 Changed 7 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 7 years ago by John Shaffer <jshaffer2112@…>

Patch against [5743], with tests.

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

#4557 sounds like a similar problem.

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

  • Summary changed from SELECT fails for mix-cased column name using postgresql to quote_name missing in postgresql backends

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

Patch against [5988].

comment:6 Changed 7 years ago by mir

  • Cc mir@… added
  • Triage Stage changed from Unreviewed to Ready 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 7 years ago by mir

  • Triage Stage changed from Ready for checkin to Unreviewed

reverting my accidental state change.

comment:8 Changed 7 years ago by SmileyChris

  • Patch needs improvement set
  • Summary changed from quote_name missing in postgresql backends to Provide tests for mixed-case column names
  • Triage Stage changed from Unreviewed to Accepted

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 7 years ago by jshaffer

  • Cc jshaffer2112@… removed

Changed 6 years ago by jshaffer

Patch against [7583].

comment:10 Changed 6 years ago by jshaffer

  • Patch needs improvement unset

comment:11 Changed 4 years ago by adamnelson

  • Patch needs improvement set

Patch needs to be updated again.

comment:12 Changed 3 years ago by gabrielhurley

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:13 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:14 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:15 Changed 10 months ago by timo

  • Easy pickings set

comment:16 Changed 10 months 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 months ago by velis

Version 0, edited 10 months ago by velis (next)

comment:18 Changed 10 months ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

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)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.