When specifying TRUE or FALSE as default value in the schema API, you’ll get a database error because (string)FALSE === ''. This means that you’ll end up with invalid SQL. The patch checks whether the value is a boolean and converts it to an int so that we end up with '0' or '1' instead of '' and '1'.

CommentFileSizeAuthor
#4 6dad83e7.patch1.64 KBkkaefer
#2 6dad83e7.patch1.06 KBkkaefer
6dad83e7.patch510 byteskkaefer
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kkaefer’s picture

kkaefer CreditAttribution: kkaefer commented
Status: Active » Needs review
kkaefer’s picture

kkaefer CreditAttribution: kkaefer commented
FileSize
6dad83e7.patch1.06 KB

Same for pgsql (untested!)

kkaefer’s picture

kkaefer CreditAttribution: kkaefer commented

BTW, I noticed that the pgsql version lacks the 

elseif (is_null($spec['default'])) {
  $spec['default'] = 'NULL';
}

part. Is this on purpose or should that be added to pgsql as well?

kkaefer’s picture

kkaefer CreditAttribution: kkaefer commented
Title: Handle boolean default values correctly » Schema API: Handle default values correctly
FileSize
6dad83e7.patch1.64 KB

Also ignore the default value when the field is a serial.

Crell’s picture

Crell CreditAttribution: Crell commented

Since we don't use "real" booleans in the database anyway, shouldn't we just tell people to use 1/0 instead of TRUE/FALSE for an int field? They'll have to write 1/0 to it in insert/update statements anyway rather than TRUE/FALSE, so cleaning up after them here and not elsewhere doesn't make sense.

kkaefer’s picture

kkaefer CreditAttribution: kkaefer commented

Do they? when you update an int column, you specify int in the dbal and it converts TRUE/FALSE automatically to int. That was my understanding thus far.

Status: Needs review » Needs work

The last submitted patch failed testing.

jbrown’s picture

jbrown CreditAttribution: jbrown commented

Note that default values are also not handled correct for fields with 'serialize' => TRUE.

Say you want a default value of array() in a serialized field. The default value should be serialized by the database layer before being put into the create table query, otherwise it will cause an exception.

Simply setting the default value to 'a:0:{}' will not work as then drupal_write_record() will serialize this to s:6:"a:0:{}";

Crell’s picture

Crell CreditAttribution: Crell commented

kkaefer: Drupal 6 did so implicitly because we preprocessed all values. The D7 API doesn't do that anymore, as it passes (almost) all type juggling off to the database itself via prepared statements where they belong.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

This is related to #403840: Cast numeric values at the level of the DB driver. If we don't fix that (suggested by Josh Waihi in #403840-14: Cast numeric values at the level of the DB driver, then there is no point in fixing this either.