Opened 17 months ago

Closed 15 months ago

Last modified 15 months ago

#22424 closed Bug (fixed)

Default value for TextField

Reported by: Nevsky Owned by: Loic Bistuer <loic.bistuer@…>
Component: Migrations Version: 1.7-beta-1
Severity: Release blocker Keywords:
Cc: loic84, denis.cornehl@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by loic84)

Hi! I start testing Django 1.7beta1. I use MySQL database.
I add new TextField into my model like that:

class Article(models.Model):
    title = models.CharField(max_length=200)
    pub_date = models.DateField()
    text = models.TextField()
    text2 = models.TextField() #new TextField

When I type "python manage.py makemigrations" I am asked to enter default value.
But BLOB/TEXT columns can't have a default value and if i will type something like 'blabla' then after command "python manage.py migrate" i will see error. I think it is bug :)

Change History (19)

comment:1 follow-up: Changed 17 months ago by loic84

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Hi @Nevsky, default is actually handled at the Python level, not added to the SQL, so that shouldn't be an issue.

I can reproduce that adding a TextField() triggers the questioner which is probably confusing, I wonder if we can't just use '' as default automatically when default=NOT_PROVIDED and empty_strings_allowed=True. Accepting the ticket on this basis.

Regarding the error with 'blabla', did you actually type the quotes? That prompt interprets Python, so if you just type blabla it'll throw an error because it can't find a variable named "blabla".

comment:2 in reply to: ↑ 1 Changed 17 months ago by Nevsky

Replying to loic84:

I can reproduce that adding a TextField() triggers the questioner which is probably confusing, I wonder if we can't just use as default automatically when default=NOT_PROVIDED and empty_strings_allowed=True. Accepting the ticket on this basis.

Yes I think correct is to use "None" as default in this instance.

Regarding the error with 'blabla', did you actually type the quotes? That prompt interprets Python, so if you just type blabla it'll throw an error because it can't find a variable named "blabla".

Yes, I type the quotes and migration successfully making. I confused commands "migrate" and "magemigrations" in ticked description. I get error when I trying to apply created migration because it contains default value for field that can't have default value in MySQL database :)

Last edited 17 months ago by Nevsky (previous) (diff)

comment:3 Changed 16 months ago by timo

Loic, I'm not sure using '' as an automatic default is better than the current situation. Why not give the user the option of entering something else besides that (especially if blank=False)?

Last edited 16 months ago by timo (previous) (diff)

comment:4 Changed 16 months ago by loic84

  • Description modified (diff)

Fixed the name of the commands in the ticket description.

I originally assumed this was an issue before the migrate step, but it's indeed an SQL issue due to one-off defaults, Django uses db defaults for these.

The fix for MySQL is to not provide a default for BLOB/TEXT in the SchemaEditor. Although we'll now need to propagate the default value manually.

comment:5 Changed 16 months ago by loic84

@Nevsky could you check if this solves your problem? https://github.com/django/django/pull/2634

comment:6 Changed 16 months ago by loic84

  • Cc loic84 added

comment:7 Changed 16 months ago by andrewgodwin

loic84: That PR looks like the right approach to me; will MySQL reject any default value on blob/text columns or just empty ones? Oracle has a weird behaviour where it treats empty strings as NULL on text columns that causes similar issues, but we have a fix for that.

comment:8 Changed 16 months ago by loic84

MySQL rejects anything even remotely related to a default for blob/text columns (i.e. ALTER TABLE 'table' ALTER COLUMN 'blob' DROP DEFAULT).

comment:9 Changed 16 months ago by syphar

  • Owner changed from nobody to syphar
  • Status changed from new to assigned

I'll have a look at it and verify the fix.

(atm I get Nevsky's error when adding the column without the fix, with the fix I get another error. But checking it.

comment:10 Changed 16 months ago by syphar

  • Owner syphar deleted
  • Status changed from assigned to new

comment:11 Changed 16 months ago by syphar

Update the PR with some comments:
https://github.com/django/django/pull/2634

short version:
PR failing when there are empty or string defaults.
But with fixed everything (in this ticket) seems fine.

comment:12 Changed 16 months ago by syphar

  • Cc denis.cornehl@… added

comment:13 Changed 16 months ago by loic84

Updated the PR to fix the issue reported by @syphar and to incorporate the fix for #22626.

Sadly the handling of bytes/strings is still less than ideal in the many quote_value:

  • " in a default will break SQLite and MySQL.
  • Oracle doesn't support bytes so breaks on BinaryField.
  • Oracle uses repr to quote six.string_types, which will probably result in an extra u prefix.

I propose we open a new ticket to deal with this issue separately.

comment:14 Changed 16 months ago by CollinAnderson

#22649 SchemaEditor.quote_value is very fragile.

comment:15 Changed 15 months ago by Loic Bistuer <loic.bistuer@…>

In 6aacb4c9915c83a897dbde0b11cc46131fc7d16e:

Fixed #22626 -- Allow BinaryField defaults with SQlite.

Also fixes a slight issue in sqlite3.schema._remake_table where
default values where quoted with "column name" quoting rules.

Reference for quoting: http://www.sqlite.org/lang_expr.html

Thanks Shai Berger for the review. Refs #22424.

comment:16 Changed 15 months ago by Loic Bistuer <loic.bistuer@…>

  • Owner set to Loic Bistuer <loic.bistuer@…>
  • Resolution set to fixed
  • Status changed from new to closed

In 1d3d01b4f74f80655ac354f7a39e3908a9b41b14:

Fixed #22424 -- Fixed handling of default values for TextField/BinaryField on MySQL.

Thanks syphar for the review and suggestions.

comment:17 Changed 15 months ago by Andrew Godwin <andrew@…>

In 4e32e47348473757280ca7a8d78edf6011fe65a3:

Merge pull request #2634 from loic/ticket22424

Fixed #22424 -- MySQL doesn't accept migrations' one-off default values ...

comment:18 Changed 15 months ago by Tim Graham <timograham@…>

In 1a29675d76de00d6fad9b366fc66e8b6d541c3b6:

[1.7.x] Fixed #22626 -- Allow BinaryField defaults with SQlite.

Also fixes a slight issue in sqlite3.schema._remake_table where
default values where quoted with "column name" quoting rules.

Reference for quoting: http://www.sqlite.org/lang_expr.html

Thanks Shai Berger for the review. Refs #22424.

Backport of 6aacb4c991 from master

comment:19 Changed 15 months ago by Tim Graham <timograham@…>

In d61b6224b09cb2aa6fb75ccecab246f91553ef7b:

[1.7.x] Fixed #22424 -- Fixed handling of default values for TextField/BinaryField on MySQL.

Thanks syphar for the review and suggestions.

Backport of 1d3d01b4f7 from master

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