Simple decimals fail to pass validation

Created on 1 April 2014, about 10 years ago
Updated 3 April 2024, 10 days ago

Problem/Motivation

- Create decimal field, set precision to 10 (minimum in the UI and scale to 4
- Saving new node with value 19999.0000 succeeds (precision is 5+4 = 9).
- Saving new node with value 99999.0000 fails (same precision as above).

or

- Create decimal field, set scale to anything over 6 (need 8 to store bitcoin values for example). I used 16 as precission.
- Saving new node with value 20.12345678 fails validation while 0.1234567 succeeds.

This is because Drupal\Component\Utility\Number::validStep() returns false. Detailed investigation on this function (which is only used once in Drupal), reveals that the first argument ($value) is received as a string, but $step and $offset are floats. PHP seems to slightly mangle the $step value on the first case above from 0.0001 to 0.00010000000000000001438. Passing the step parameter as a string in the case of decimal numbers maintains the correct precision, and allows a correct approximation calculation.

Proposed resolution

Bypass weak PHP floating-point handling by passing the step as a string.

Remaining tasks

Merge. Commit. Decimals FTW!

User interface changes

None.

API changes

None.

Data model changes

None.

Possible workaround if you need big decimals

Disable this validation by setting the render element #step to 'any'

i.e.

$element['#step'] = 'any';

In the case of fields, you can do

function MYMODULE_form_FORM_WITH_BIG_DECIMAL_FIELD_alter (array &$form, FormStateInterface $form_state) {
  $form['field_some']['widget'][0]['value']['#step']='any';
}

And if you want to target all decimal fields:

/**
 * Prevents validation of decimal numbers
 * @see https://www.drupal.org/node/2230909
 */
function MYMODULE_field_widget_form_alter(&$element, \Drupal\Core\Form\FormStateInterface $form_state, $context) {
  $field_definition = $context['items']->getFieldDefinition();
  if ($field_definition->getType() == 'decimal') {
    $element['value']['#step'] = 'any';
  }
}
๐Ÿ› Bug report
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Numberย  โ†’

Last updated 10 days ago

No maintainer
Created by

๐Ÿ‡ซ๐Ÿ‡ฎFinland exlin

Live updates comments and jobs are added and updated live.
  • Triaged core major

    There is consensus among core maintainers that this is a major issue. Only core committers should add this tag.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates : some issue and comment data are missing.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Seems points in #291 have been addressed.

  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ
    +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/DecimalItem.php
    @@ -69,7 +69,7 @@ public function storageSettingsForm(array &$form, FormStateInterface $form_state
    -      '#max' => 32,
    +      '#max' => ini_get('precision') <= 10 ? 10 : ini_get('precision'),
    
    @@ -79,7 +79,7 @@ public function storageSettingsForm(array &$form, FormStateInterface $form_state
    -      '#max' => 10,
    +      '#max' => ini_get('precision'),
    

    I think we need a update function here for existing fields as we're changing the maximum so current configuration can be invalid. Not sure what we should be doing with existing decimal fields...

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom kiwimind

    Having come across this issue on 9.5.8, applying the patch from https://www.drupal.org/project/drupal/issues/2230909#comment-14626332 ๐Ÿ› Simple decimals fail to pass validation Needs work has solved this issue.

    Prior to the patch we were unable to set a default value of a trillion (1 with 12 zeroes) on a 15,2 decimal field. After the patch it has saved as expected.

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost
    +++ b/core/lib/Drupal/Component/Utility/Number.php
    @@ -9,6 +9,71 @@
    +      // If the float has less significant decimals than the number we can
    +      // guarantee, convert it to a string directly.
    +      if (preg_match(sprintf('/^\d+\.\d{1,%d}$/', ini_get('precision')), (string) $number)) {
    +        return (string) $number;
    +      }
    +      // For floats with more significant decimals than the number we can
    +      // guarantee, discard those that are not guaranteed.
    +      return rtrim(number_format($number, ini_get('precision'), '.', ''), '0');
    
    +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/DecimalItem.php
    @@ -69,7 +69,7 @@ public function storageSettingsForm(array &$form, FormStateInterface $form_state
           '#title' => $this->t('Precision'),
           '#min' => 10,
    -      '#max' => 32,
    +      '#max' => ini_get('precision') <= 10 ? 10 : ini_get('precision'),
    
    @@ -79,7 +79,7 @@ public function storageSettingsForm(array &$form, FormStateInterface $form_state
           '#title' => $this->t('Scale', [], ['context' => 'decimal places']),
           '#min' => 0,
    -      '#max' => 10,
    +      '#max' => ini_get('precision'),
    
    +++ b/core/tests/Drupal/Tests/Component/Utility/NumberTest.php
    @@ -34,17 +34,107 @@ public function testValidStep($value, $step, $expected) {
    +   * @covers \Drupal\Component\Utility\Number::normalize
    ...
    +  public function testNormalize($number, $expected) {
    +    $this->assertEquals($expected, Number::normalize($number));
    ...
    +   * Provides data to self::testNormalize().
    

    Checking requirements should go to report/status instead of runtime

    If you need properly configured server no reason to scatter this checks all over codebase

    Also a test should use permutations for precision configured in field and via ini, I mean missing case when ini_get returns wrong data or it <10

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    Also this code should be overridable from contrib to allow override math via PHP bc, gmp, decial extensions

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pwolanin

    @anypost - this important patch has been waiting for 7+ years to get committed. we should not expand the scope. If you want to e.g. be able to use BC math, please open a follow-up issue.

  • ๐Ÿ‡ธ๐Ÿ‡ฆSaudi Arabia samaphp Riyadh, SA ๐Ÿ‡ธ๐Ÿ‡ฆ
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia vinaymahale

    Any more work need to be done here?

  • Status changed to Needs work 9 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    #294

    and #296 or follow up least opened.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly kopeboy Mainland

    I'm not an expert developer and couldn't read all the comments and patches, but I hope this can help.

    I was appalled to see I couldn't save web3 wallet balances to my Drupal site (there a lot of decimals places) even if I could easily define my "balance" field (in my custom entity's BaseFieldDefinition) with precision = 65 and scale = 30 (which correctly creates a column in the database with Type: DECIMAL, Lenght: 65,30)...
    These values will come from APIs, but while testing with a manual form input a value of 0.123456789012345678901234567890 would be magically converted to 0.123456789012350000000000000000 in the UI AND database (without validation warnings).
    Strangely, the same input given programmatically (I tested with ECA model, ie. plugin, both in plain text and in YAML format) would become 0.123456789012350000000000000000 (last non-zero digit = 6 instead of 5 ๐Ÿง)

    So I checked core/lib/Drupal/Core/Field/Plugin/Field/FieldType/DecimalItem.php myself and I found that I could fix the problem just by commenting lines 122 to 127:

      /**
       * {@inheritdoc}
       */
      public function preSave() {
        $this->value = round($this->value, $this->getSetting('scale'));
      }

    Note that scale should be already defined by this (right?):

      public static function baseFieldDefinitions(EntityTypeInterface $entity_type): array {
    
        $fields = parent::baseFieldDefinitions($entity_type);
    
        $fields['balance'] = BaseFieldDefinition::create('decimal')
          ->setSettings(array(
            'type' => 'numeric',
            'unsigned' => TRUE,
            'precision' => 65,
            'scale' => 30,
            'not null' => FALSE,
            'description' => t('the decimal integer amount'),
          ))
          ->setLabel(t('Balance'))
          ->setDisplayConfigurable('form', TRUE)
          ->setDisplayConfigurable('view', TRUE);

    So why do we want to round at all when we are already forcing the precision and scale in the field (storage) definition?
    From https://www.php.net/manual/en/function.round I immediately read

    round โ€” Rounds a float

    ,
    but this is a decimal!!

    Then shouldn't that be just an optional Manage Display's setting for decimal fields?
    Also, why isn't the round function called for float number fields (FloatItems.php file in the same folder) instead?

  • ๐Ÿ‡ฆ๐Ÿ‡นAustria tgoeg

    Also not a dev, but I think I can at least clear up things a little.

    First, this is not a decimal (as strange as it may sound). Your number is a float (mathematically) and SQL stores it as an exact data type DECIMAL with a given precision (which in fact is a float). There is an explicit FLOAT data type in SQL, but that is not guaranteed to be exact.

    The basic underlying problem is that computers "think" binary and not decimally. Representing an arbitrary number with lots of places after the decimal point will inherently produce errors (depending on which number it is, as some ("machine numbers") map cleanly to binary base and some do not. That's why people here see this only with select numbers). See https://en.wikipedia.org/wiki/Round-off_error for details.

    The internal representation of a number 0.123456 might be 0.123456789 because of rounding errors. That's why rounding to the exact given scale does make sense. It's no actual rounding of the given number in the human sense. It just makes sure the number is exactly the one entered and has exactly that many decimal places as configured.

    Does the patch in #2230909-292: Simple decimals fail to pass validation โ†’ not fix it for you? That would be an important information.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia asad_ahmed

    I have made the changes as per comment #294. And write a update function in Field.install file but i am not sure where to place it, currently i have placed Field.install here core/lib/Drupal/Core/Field/Field.install. Please review it and give feedback if any. Thanks

  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 4 months ago
    Custom Commands Failed
  • ๐Ÿ‡ฆ๐Ÿ‡นAustria maxilein

    patch #305 applies to D10.2 but it does not work.

    I tested a default decimal field (10,2) and a large decimal field (14,2)

    Problem 1: Both act like a standard field with 10 digits.
    Problem 2: the decimal separator is counted as one digit!
    Problem 3: There used to be larger numbers possible than 14.

    Please see attached screenshots.

  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 4 months ago
    Patch Failed to Apply
  • ๐Ÿ‡ฆ๐Ÿ‡นAustria maxilein

    I just realized that #305 is just an addition.

    #292 does not apply to 10.2

  • ๐Ÿ‡ท๐Ÿ‡บRussia APolitsin

    ะผะพะน ะปัŽะฑะธะผั‹ะน ั‚ะธะบะตั‚ ะฝะฐ ะดั€ัƒะฟะฐะปะพั€ะณะต

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany szeidler Berlin

    Rerolling #292 for Drupal 10.2

  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 4 months ago
    Build Successful
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MySQL 5.7 updated deps
    last update 4 months ago
    Build Successful
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 4 months ago
    25,970 pass, 1,784 fail
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 4 months ago
    25,849 pass, 1,787 fail
  • last update 4 months ago
    Build Successful
  • last update 4 months ago
    25,826 pass, 1,797 fail
  • ๐Ÿ‡ฆ๐Ÿ‡นAustria maxilein

    #309 helps to use large numbers. Thank you very much!!!! We can upgrade to 10.2!

    If we get out of bounds there is just a nasty crash error. Not a warning in the gui.

    1,123,123,123.00 - just ok.
    123,123,123,123 -> crash

    
    The website encountered an unexpected error. Try again later.
    
    Drupal\Core\Entity\EntityStorageException: SQLSTATE[22003]: Numeric value out of range: 1264 Out of range value for column 'field_kostenschaetzung_value' at row 1: INSERT INTO "node__field_kostenschaetzung" ("entity_id", "revision_id", "bundle", "delta", "langcode", "field_kostenschaetzung_value") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5); Array ( [:db_insert_placeholder_0] => 6479 [:db_insert_placeholder_1] => 19995 [:db_insert_placeholder_2] => fue_projekt_kosten [:db_insert_placeholder_3] => 0 [:db_insert_placeholder_4] => en [:db_insert_placeholder_5] => 123123123123 ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 817 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
    
    Drupal\Core\Database\StatementWrapperIterator->execute() (Line: 44)
    Drupal\mysql\Driver\Database\mysql\Insert->execute() (Line: 1400)
    Drupal\Core\Entity\Sql\SqlContentEntityStorage->saveToDedicatedTables() (Line: 971)
    Drupal\Core\Entity\Sql\SqlContentEntityStorage->doSaveFieldItems() (Line: 718)
    Drupal\Core\Entity\ContentEntityStorageBase->doSave() (Line: 486)
    Drupal\Core\Entity\EntityStorageBase->save() (Line: 806)
    Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (Line: 352)
    Drupal\Core\Entity\EntityBase->save() (Line: 270)
    Drupal\node\NodeForm->save()
    call_user_func_array() (Line: 129)
    Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (Line: 67)
    Drupal\Core\Form\FormSubmitter->doSubmitForm() (Line: 597)
    Drupal\Core\Form\FormBuilder->processForm() (Line: 325)
    Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
    Drupal\Core\Controller\FormController->getContentResult() (Line: 39)
    Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController->getContentResult()
    call_user_func_array() (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 627)
    Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
    Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 58)
    Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 106)
    Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 85)
    Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
    Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
    Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 704)
    Drupal\Core\DrupalKernel->handle() (Line: 19)
    
    
  • last update 3 months ago
    30,919 pass, 15 fail
  • last update 3 months ago
    26,069 pass, 1,831 fail
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MySQL 5.7 updated deps
    last update 3 months ago
    Build Successful
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 3 months ago
    Build Successful
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium weseze

    Same experience as maxilein.

    The patch fixes our problem for storing correct numbers.
    But if we try and enter bigger numbers we get a nasty WSOD and some SQL out-of-bound errors. Not a warning in the interface.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium keszthelyi Brussels

    @maxilein, @weseze There is another issue and patch for the exception thrown when the entered value is too big: https://www.drupal.org/project/drupal/issues/1003692 ๐Ÿ› PDOException: SQLSTATE[22003] after entering large value in integer and decimal field Needs work

  • ๐Ÿ‡ท๐Ÿ‡บRussia APolitsin

    ๐Ÿ’ฅ Friends!
    ๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰ 10 years to this wonderful issue! ๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰
    ๐Ÿ”ฅ Created 2 Apr 2014 at 00:19 MSK

    Congratulations to everyone on this important event, especially parent #1 @exlin

    โ˜€๏ธ Good day, everyone.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia mstrelan

    I guess that's supposed to be sarcasm. Seems that we need to take 309 and add the test from 305 and review against that. A merge request would probably be helpful. The hook_requirements in 296 hasn't been addressed either I don't think.

Production build https://api.contrib.social 0.62.1