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

Comments

cburschka’s picture

cburschka
they
commented
Status: Active » Needs review
StatusFileSize
new 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
commented

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

pukku’s picture

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
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
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
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
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 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) commented
Status: Fixed » Closed (fixed)

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