Code

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9862 closed (fixed)

Non-standard SQL generated in column definition for nullable columns in create table DDL

Reported by: Ambrish Owned by: mtredinnick
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords:
Cc: gabor@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Hi,

While doing backend testing for new DB2 adapter (I am developing it), I found that in CREATE TABLE DDL generation, it contains some non-standard SQL syntax.

Here is one example of model.

class User(models.Model):
    first_name = models.CharField(max_length=20, null=False)
    last_name = models.CharField(max_length=20, null=True)

    class Meta:
        db_table = "temp_user"

For the column last_name, the SQL that gets generated is LAST_NAME VARCHAR(20) NULL. The trailing NULL is not a standard SQL. This can be validated by the SQL validator tool - http://developer.mimer.com/validator/parser200x/index.tml Needless to say, this will fail in DB2 and so too in Oracle.

I find only one method in the docs which deals with the table DDL generation BaseDatabaseCreation.sql_create_model.

Regards,
Ambrish Bhargava

Attachments (1)

9862.diff (1.3 KB) - added by ikelly 5 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 5 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Accepted

No database on earth implements the same subset of SQL, so references to the standard, with all its optional bits and non-standard extensions in implementation, isn't too helpful either way. However, if a database doesn't support something, we should make sure it's not required. Practice always trumps theory.

In this case, the solution is probably just to not include the NULL. NOT NULL is permitted, so we include that for null=False fields and use the fact that columns should be nullable by default.

Ambrish: it's only NULL that is causing a problem on db2, right? NOT NULL is fine?

We still need to check that this works sufficiently on all the other backends (including getting some confirmation from Ramiro Morales about MS SQL). If there's an inconsistency in requirements, it will need an addition of another BaseDatabaseFeature function.

comment:2 Changed 5 years ago by Ambrish

Hi,


it's only NULL that is causing a problem on db2, right? NOT NULL is fine?


With DB2, only NULL is causing a problem and with NOT NULL there are no issues.

Regards,
Ambrish Bhargava

comment:3 Changed 5 years ago by gabor

  • Cc gabor@… added

Changed 5 years ago by ikelly

comment:4 Changed 5 years ago by ikelly

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Has patch set

comment:5 Changed 5 years ago by Ambrish

Thanks Ian...

Regards,
Ambrish Bhargava

comment:6 Changed 5 years ago by mtredinnick

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

comment:7 Changed 5 years ago by mtredinnick

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

(In [9703]) Fixed #9862 -- For better SQL portability, don't specify "NULL" on nullable
columns when creating tables. Patch from Ian Kelly.

Columns are NULL by default, so we only need to use "NOT NULL" when we want
non-default behaviour.

comment:8 Changed 5 years ago by mtredinnick

(In [9704]) [1.0.X] Fixed #9862 -- For better SQL portability, don't specify "NULL" on nullable columns when creating tables. Patch from Ian Kelly.

Columns are NULL by default, so we only need to use "NOT NULL" when we want
non-default behaviour.

Backport of r9703 from trunk.

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.