[Django] #24675: Unnecessary queries since MySQL 5.6
Django
----------------------------------------------+--------------------
Reporter: ssjunior | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Database layer (models, ORM) | Version: 1.8
Severity: Normal | Keywords: mysql
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Every connection mada to MySQL is issuing an unnecessary query setting
SQL_AUTO_IS_NULL to 0. Since MySQL this is not needed anymore and this can
be set in the server variables too. Avoiding this query will result in
better performance. This shoud be a settings options that allow user to
enable or disable this unnecessary query.
http://dev.mysql.com/doc/refman/5.6/en/server-system-
variables.html#sysvar_sql_auto_is_null
System Variable Name sql_auto_is_null
Variable Scope Global, Session
Dynamic Variable Yes
Permitted Values Type boolean
Default 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24675>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: ssjunior | Owner: ssjunior
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ssjunior):
* owner: nobody => ssjunior
* status: new => assigned
* needs_docs: => 0
* needs_tests: => 0
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:1>
Django
Reporter: ssjunior | Owner: ssjunior
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: mysql | Triage Stage:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by ssjunior):
* status: assigned => closed
* resolution: => fixed
Comment:
Pull request submitted.
https://github.com/django/django/pull/4538
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:2>
Django
Reporter: ssjunior | Owner: ssjunior
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by ssjunior):
* status: closed => new
* resolution: fixed =>
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:3>
Django
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Can we avoid the setting and make it a function of the MySQL version? I'm
not clear if it's always safe to skip this query on MySQL 5.6+, or only
when also setting a MySQL server variable?
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by ssjunior):
I think that I have a better solution. Check in the features for the
status on the server and issue the command only if it is True on the
server. Tell me what do you think and if it is ok I will submitt a new PR.
https://github.com/ssjunior/django/commit/52bf6c7630ab6b17a3a3ff64213e1ad36d6fe270
{{{
@cached_property
def sql_auto_is_null(self):
"Return if server global variable sql_auto_is_null is set to True"
with self.connection.cursor() as cursor:
cursor.execute("SHOW GLOBAL VARIABLES LIKE
'sql_auto_is_null'")
return cursor.fetchone()[1] == 'ON'
def init_connection_state(self):
# SQL_AUTO_IS_NULL in MySQL controls whether an AUTO_INCREMENT
column
# on a recently-inserted row will return when the field is tested
for
# NULL. Disabling this value brings this aspect of MySQL in line
with
# SQL standards.
# Check the sql_auto_is_null status on the MySQL server and if
necessary
# set it to False.
if self.features.sql_auto_is_null:
with self.cursor() as cursor:
cursor.execute('SET SQL_AUTO_IS_NULL = 0')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by claudep):
Looks good, please make it a pull request. It will need some tests,
though.
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
* needs_tests: 0 => 1
Comment:
Claude, how did you imagine a test would look like for this?
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Something like: take a connection object, close it, reopen it and check
that no `SQL_AUTO_IS_NULL` query is done.
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by stewartpark):
* status: new => closed
* resolution: => fixed
Comment:
https://github.com/django/django/pull/5794
I made a new pull request for this issue. Added testing as well.
Can you check if it looks good?
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by stewartpark):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by shaib):
* status: closed => new
* resolution: fixed =>
Comment:
We only mark tickets fixed when the PR has been merged.
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by stewartpark):
I see! Sorry about that. Any plan on reviewing this pull request?
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Left some minor comments. Please uncheck "Patch needs improvement" when
you update it, thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by stewartpark):
* needs_better_patch: 1 => 0
Comment:
I fixed the code the way you suggested. :) Also passed all the test cases.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"b7fdd60d85ee94df444369be1b2c778cda44a0c2" b7fdd60]:
{{{
#!CommitTicketReference repository=""
revision="b7fdd60d85ee94df444369be1b2c778cda44a0c2"
Fixed #24675 -- Skipped SQL_AUTO_IS_NULL query on MySQL if not needed.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by brianwawok):
In development I still see 2 queries per page load for this. I assume
because connections do not persist in dev.
Seems this needs dealt with in a different way in dev?
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: ssjunior | Owner: ssjunior
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: mysql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
This patch has only be merged in the ''master'' branch so far and is not
part of any official release. It should be part of the upcoming 1.10
release.
--
Ticket URL: <https://code.djangoproject.com/ticket/24675#comment:18>