non-standard precision limits at testSchemaAddFieldDefaultInitial

@bohart: Could add some more documentation to the patch. Say that Oracle has a limit of 38 and add a link to the Oracle page where they state that the limit is 38. After that is it RTBC for me.

Let me do that.

Please find the Attached patch and inter diff to review.

Let me know if any further changes to be made.

Added link text further to above patch.Please refer this patch.

@dhirendra.mishra: Could you put all above or all below foreach ([1, 5, 10, 38] as $precision) {. Thank you for adding the correct link.

Hope its corrected now.

@dhirendra.mishra: Thanks!

For me it is RTBC.

And what happens when someone wants to support a DB that has even less precision support?

As I've said in other issues we need to come up with a better way to support testing of contrib database testing using core tests. If we were to add another driver to core knowing that it differed in the amount of precision it supports would be useful - especially when that limit is lower than what core's supplied drivers support. What happens now if someone adds a precision of 60 to a contrib or core db field?

That said, the field system currently has:

    $element['precision'] = [
      '#type' => 'number',
      '#title' => t('Precision'),
      '#min' => 10,
      '#max' => 32,
      '#default_value' => $settings['precision'],
      '#description' => t('The total number of digits to store in the database, including those to the right of the decimal.'),
      '#disabled' => $has_data,
    ];

+++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
@@ -638,7 +638,11 @@ public function testSchemaAddFieldDefaultInitial() {
+    // As Oracle has limit of 38.
+    // https://docs.oracle.com/cd/B19306_01/server.102/b14237/limits001.htm#i287903
+    // Due to the limits of the different databases, the maximum precision
+    // to test is 38.

This comment is not flowing properly - i.e always using the full 80 characters and the first fullstop is in an interesting place. Plus the latest oracle db server is 20 and this points to 10. So https://docs.oracle.com/en/database/oracle/oracle-database/20/refrn/data... is a better link. But I think that this comment needs to be re-written and point to the UI limit as a sensible maximum to test to. If a driver doesn't support 32 then they'd need to handle fields created via the UI.

As test changes for contrib database drivers are not allowed into core. I would like to suggest a different approach. I have moved the array of values for which the precision of numeric values is tested to a class variable. The Oracle database driver can then add an test by extending the test class SchemaTest in the following way:

class SchemaTest extends CoreSchemaTest {

  protected $precision = [1, 5, 10, 38];

}

In the file phpunit.xml a exclude group must then be added (See: https://phpunit.readthedocs.io/en/7.5/configuration.html#the-groups-element). Something like:

  <exclude>
    <group>does-not-work-for-oracle</group>
    <file>tests/Drupal/KernelTests/Core/Database/SchemaTest.php</file>
  </exclude>

The testbot must then called with the parameter --exclude-group does-not-work-for-oracle. The current DrupalCL testbot does not have that ability, but it could be added. Should not be that difficult.

Good evening everyone.
I had a look at this during the #drupalmountaincamp

And I have the feeling that we should not be trying to figure out the limit each core or contrib driver defines, but instead try to ensure that conservatively low values work across core and contrib; but most importantly, try to cause an error by setting a very high precision value and catch that case too.

Here is a patch that does just that:

diff --git a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
index 87fa76586c..2f5f5ade0a 100644
--- a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
@@ -10,7 +10,9 @@
 use Drupal\Core\Database\SchemaObjectExistsException;
 use Drupal\KernelTests\KernelTestBase;
 use Drupal\Component\Utility\Unicode;
+use Drupal\Core\Database\DatabaseExceptionWrapper;
 use Drupal\Tests\Core\Database\SchemaIntrospectionTestTrait;
+use Drupal\sqlite\Driver\Database\sqlite\Schema as SqliteSchema;
 
 /**
  * Tests table creation and modification via the schema API.
@@ -639,7 +641,7 @@ public function testSchemaAddFieldDefaultInitial() {
     }
 
     // Test numeric types.
-    foreach ([1, 5, 10, 40, 65] as $precision) {
+    foreach ([1, 5, 10] as $precision) {
       foreach ([0, 2, 10, 30] as $scale) {
         // Skip combinations where precision is smaller than scale.
         if ($precision <= $scale) {
@@ -665,6 +667,18 @@ public function testSchemaAddFieldDefaultInitial() {
         }
       }
     }
+    // Test numeric types with unsupported precision.
+    $failing_precision_field_spec = [
+      'type' => 'numeric',
+      'scale' => 2,
+      'precision' => 9999,
+    ];
+    // SQLite does not enforce neither precision nor precision limits.
+    if (!is_a($this->schema, SqliteSchema::class)) {
+      $this->expectException(DatabaseExceptionWrapper::class);
+      $this->expectExceptionMessageMatches('/precision/i');
+    }
+    $this->assertFieldAdditionRemoval($failing_precision_field_spec);
   }
 
   /**

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.