This also affects the PostgreSQL database.

In both includes/database.mysql-common.inc and includes/database.pgsql.inc, the function _db_create_field_sql has the precision and scale arguments in the wrong order for numeric fields. They should be precision then scale.

That is, the lines:

  elseif (isset($spec['precision']) && isset($spec['scale'])) {
    $sql .= '('. $spec['scale'] .', '. $spec['precision'] .')';
  }

should be

  elseif (isset($spec['precision']) && isset($spec['scale'])) {
    $sql .= '('. $spec['precision'] .', '. $spec['scale'] .')';
  }

I'm sorry I can't roll an official patch — I'm not set up for that right now.

CommentFileSizeAuthor
#1 database-scale-precision-swap-218054-1.patch1.4 KBcburschka
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
Status: Active » Needs review
FileSize
database-scale-precision-swap-218054-1.patch1.4 KB

Here you go. The MySQL docs say you're right, but I haven't tested one bit of this myself.

mfb’s picture

mfb
they or he
CreditAttribution: mfb commented

see also http://www.postgresql.org/docs/8.3/interactive/datatype-numeric.html#DAT...

pukku’s picture

pukku CreditAttribution: pukku commented
Status: Needs review » Reviewed & tested by the community

I don't know if I'm really allowed to make this ready to be committed, but the code looks right...

mfb’s picture

mfb
they or he
CreditAttribution: mfb commented

I tested the patch with precision 5, scale 2. The table is now created (where before there was an error) and it all looks good in mysql:

+--------+--------------+------+-----+---------+-------+
| foobar | decimal(5,2) | YES  |     | NULL    |       | 
+--------+--------------+------+-----+---------+-------+
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Is this used anywhere in Drupal core? Where we would need an update function to correct previously miscreated fields?

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented

It's impossible to miscreate a field this way - as mfb tested, MySQL returns an error if the precision is smaller than the scale.

I grepped for "precision" to make sure that we didn't "work around" the bug by switching precision and scale elsewhere, but didn't find anything.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Version: 6.x-dev » 7.x-dev

Great, thanks, committed to 6.x. Still needs to be committed to 7.x.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD as well -- thanks folks and good catch.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.