Code

Opened 3 years ago

Closed 23 months ago

#16926 closed Bug (fixed)

Custom SQL with Windows or Mac end-of-lines fail with Python 3

Reported by: adsworth Owned by: adsworth
Component: Python 3 Version: 1.3
Severity: Normal Keywords:
Cc: adsworth Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In core.managment.sql.custom_sql_for_model. The execution of custom sql fails when the sql file has windows or mac line endings. The sql file is opened with universal newline so that windows or mac end-of-lines are automatically translated to unix end-of-lines. In python 2.7 if the universal newline mode passed to open() the file is opened in text mode, even if binary mode is set, as is the case here. In Python 3 universal newline only works if the file is opened in text mode. Specifying b and U in the mode apparently opens it in binary mode and thus ignores the universal newline mode.

The python re module only uses \n as end-of-line. So the \r in the window or mac sql file causes the regex to fail. Subsequently multiple SQL statements are passed to the backend. This fails in e.g. SQLite.

The attached patch changes the regex which splits the files.

Another option would be to use different open() calls depending on python the version. If the file is opened in text mode in Python 3 the decode method is missing on the returned string. In python 3 the encoding parameter should probably be used in that case.

Attachments (2)

fix-also-split-on-carrige-return.diff (585 bytes) - added by adsworth 3 years ago.
16926-fix-custom_sql_for_model.patch (1.2 KB) - added by adsworth 3 years ago.

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by adsworth

comment:1 Changed 3 years ago by adsworth

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to adsworth
  • Patch needs improvement unset

comment:2 Changed 3 years ago by aaugustin

  • Needs documentation set
  • Triage Stage changed from Unreviewed to Accepted

I think this is an artifact of the 2to3 conversion.

Trunk has:

            fp = open(sql_file, 'U')

while the py3k branch has:

            fp = open(sql_file, 'rbU')

As far as I can tell, it should be enough to change the mode flag to 'U' in the py3k branch.

comment:3 Changed 3 years ago by adsworth

In theory, yes.
A py3k string, does not have a decode method though. So we need to pass the encoding parameter in case of py3k and call decode in case of py2. I'll create a new patch.

comment:4 Changed 3 years ago by aaugustin

In fact, I was surprised by the 'rbU' pattern that appears nowhere else in Django, because the b and U flags are contradictory.

It is intentional but apparently hackish: see the commit message of https://bitbucket.org/loewis/django-3k-old/changeset/0d7900201511

It would be a good idea to check with MvL before changing this code again.

Changed 3 years ago by adsworth

comment:5 Changed 3 years ago by adsworth

The new patch 16926-fix-custom_sql_for_model.patch now first tries to open the custom SQL file with the Python3 encoding parameter failing that it opens the file without the encoding parameter and then decodes the resulting string.

I'll mail MvL and ask him to comment on this patch.

comment:6 Changed 3 years ago by anonymous

Thanks for the patch. I have adopted it slightly as https://bitbucket.org/loewis/django-3k/changeset/b61b9900f687

I'll merge those back into the branch shortly.

comment:7 Changed 23 months ago by aaugustin

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

This was fixed in [4e7f04cd].

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.