Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#24675 closed Cleanup/optimization (fixed)

Skip "SET SQL_AUTO_IS_NULL = 0" on versions of MySQL that don't need it

Reported by: ssjunior Owned by: ssjunior
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords: mysql
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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

Change History (18)

comment:1 Changed 7 years ago by ssjunior

Owner: changed from nobody to ssjunior
Status: newassigned

comment:2 Changed 7 years ago by ssjunior

Resolution: fixed
Status: assignedclosed

comment:3 Changed 7 years ago by ssjunior

Resolution: fixed
Status: closednew

comment:4 Changed 7 years ago by Tim Graham

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?

comment:5 Changed 7 years ago by Tim Graham

Has patch: set
Patch needs improvement: set
Summary: Unnecessary queries since MySQL 5.6Skip "SET SQL_AUTO_IS_NULL = 0" on versions of MySQL that don't need it
Triage Stage: UnreviewedAccepted

comment:6 Changed 7 years ago 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')

comment:7 Changed 7 years ago by Claude Paroz

Looks good, please make it a pull request. It will need some tests, though.

comment:8 Changed 7 years ago by Tim Graham

Needs tests: set
Patch needs improvement: unset

Claude, how did you imagine a test would look like for this?

comment:9 Changed 7 years ago by Claude Paroz

Something like: take a connection object, close it, reopen it and check that no SQL_AUTO_IS_NULL query is done.

comment:10 Changed 7 years ago by Stewart Park

Resolution: fixed
Status: newclosed

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?

comment:11 Changed 7 years ago by Stewart Park

Needs tests: unset

comment:12 Changed 7 years ago by Shai Berger

Resolution: fixed
Status: closednew

We only mark tickets fixed when the PR has been merged.

comment:13 Changed 7 years ago by Stewart Park

I see! Sorry about that. Any plan on reviewing this pull request?

comment:14 Changed 7 years ago by Tim Graham

Patch needs improvement: set

Left some minor comments. Please uncheck "Patch needs improvement" when you update it, thanks!

comment:15 Changed 7 years ago by Stewart Park

Patch needs improvement: unset

I fixed the code the way you suggested. :) Also passed all the test cases. Thanks.

comment:16 Changed 7 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In b7fdd60:

Fixed #24675 -- Skipped SQL_AUTO_IS_NULL query on MySQL if not needed.

comment:17 Changed 6 years ago by Brian Wawok

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?

comment:18 Changed 6 years ago by Simon Charette

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.

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