Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9411 closed (fixed)

Test added in [9081] fails on MySQL

Reported by: kmtracey Owned by: mtredinnick
Component: Uncategorized Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

[9081] added:

Make sure bump_prefix() (an internal Query method) doesn't (re-)break.
>>> query = Tag.objects.values_list('id').order_by().query
>>> query.bump_prefix()
>>> print query.as_sql()[0]
SELECT U0."id" FROM "queries_tag" U0

to regressionstest/queries/models.py but this fails on MySQL due to quoting differences:

======================================================================
FAIL: Doctest: regressiontests.queries.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kmt/django/trunk/django/test/_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for regressiontests.queries.models.__test__.API_TESTS
  File "/home/kmt/django/trunk/tests/regressiontests/queries/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/home/kmt/django/trunk/tests/regressiontests/queries/models.py", line ?, in regressiontests.queries.models.__test__.API_TESTS
Failed example:
    print query.as_sql()[0]
Expected:
    SELECT U0."id" FROM "queries_tag" U0
Got:
    SELECT U0.`id` FROM `queries_tag` U0


----------------------------------------------------------------------
Ran 1 test in 1.251s

FAILED (failures=1)

Not sure how to fix it since I'm not sure how much of that output is significant. Is it sufficient that no exception was raised (seemed to be what happened before the fix)? Or do we have a general way of dealing with quoting differences in backends (like file path differences)?

Change History (6)

comment:1 Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to mtredinnick
  • Patch needs improvement unset
  • Status changed from new to assigned

Does MySQL require this somewhat nutty backtick quoting? It's pretty unreadable in general and if it can use double-quotes, we should just change it to be like all the other database backends.

But, yeah, I can rewrite the test. Probably if it runs correctly (i.e. if the query executes) it's sufficient, so I could just run it against the database.

comment:2 Changed 6 years ago by kmtracey

Looks like SET sql_mode='ANSI_QUOTES'; can be used to switch to double-quotes-signifies-identifier, single-quotes-signifies-string mode:

http://dev.mysql.com/doc/refman/5.0/en/identifiers.html

It's in the 3.23/4.0/4.1 version of the manual as well so it seems to be available far back. So it appears we could change the mysql backend to be more consistent with the others in this area.

But...it's potentially a backwards-incompatible change for anyone with raw SQL queries in their Django app code who has used double-quotes to signify strings (I'm thinking hardcoded ones, so not going through any kind of backend auto-quoting), since in the default mode you can quote strings with either single or double quotes. After switching the mode to ansi_quotes these queries would start failing.

comment:3 follow-up: Changed 6 years ago by mtredinnick

No, if it doesn't just work out of the box (i.e. if it requires a "SET..." call), there's no point in worrying about it. That's extra overhead we can avoid.

I'll work around the representation issue, but I was being hopeful on the other front (who in their right mind uses backticks as a distinguished marker?! We learnt the error of that approach with sub-processes in Unix shells about 15 years ago, long before MySQL was on the scene).

comment:4 in reply to: ↑ 3 Changed 6 years ago by kmtracey

Replying to mtredinnick:

...(who in their right mind uses backticks as a distinguished marker?! We learnt the error of that approach with sub-processes in Unix shells about 15 years ago, long before MySQL was on the scene).

Isn't just about everything in computers relearned/rediscovered/redeveloped every few years? (I am perhaps a tad cynical.)

comment:5 Changed 6 years ago by mtredinnick

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

(In [9254]) Fixed #9411 -- Changed the test from r9081 to be more robust across different database backends.

comment:6 Changed 6 years ago by kmtracey

(In [9275]) [1.0.X] Fixed #9411 -- Changed the test from r9081 to be more robust across different database backends.

Backport of [9254] from trunk (I neglected to mention in the ticket this affected 1.0.X branch).

Also updated svnmerge metadata; all eligible fixes have been backported and all others blocked.

Note: See TracTickets for help on using tickets.
Back to Top