Column default values are not replaced by placeholders when creating new tables. This means curly brackets will have the table prefix added to it, and an exception will be thrown if a semicolon appears in the default.

Potential solution is to use placeholders for default values in CREATE TABLE queries, but this would require Schema::createTableSQL() to return an array of arrays, where each element is a query string and an array of placeholders/values.

Alternatively, the table name can be prefixed prior to creating the query string, and the query can be prepared without calling prefixTables()

This bug is likely also an issue with Schema::changeField().

CommentFileSizeAuthor
#12 reroll_diff_3130333_3-12.txt3.4 KBankithashetty
#12 3130333-12.patch2.39 KBankithashetty
#3 3130333-3.patch1.95 KBBeakerboy
#2 3130333-2.patch1.94 KBBeakerboy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Beakerboy created an issue. See original summary.

Beakerboy’s picture

Beakerboy
Primary language English
Location Kentucky
CreditAttribution: Beakerboy as a volunteer commented
FileSize
3130333-2.patch1.94 KB

Added tests to see if these conditions lead to failures.

Beakerboy’s picture

Beakerboy
Primary language English
Location Kentucky
CreditAttribution: Beakerboy as a volunteer commented
FileSize
3130333-3.patch1.95 KB

revised patch with correct function and white-space.

Beakerboy’s picture

Beakerboy
Primary language English
Location Kentucky
CreditAttribution: Beakerboy as a volunteer commented
Title: Column Default Values » Column Default Values can not contain curly brackets or semicolons
Assigned: Beakerboy » Unassigned
Issue summary: View changes
Beakerboy’s picture

Beakerboy
Primary language English
Location Kentucky
CreditAttribution: Beakerboy as a volunteer commented
Parent issue: » #1292966: Curly braces cannot be used in default values for columns
Related issues: +#1292966: Curly braces cannot be used in default values for columns

Looks like the curly brace problem has already been reported.

Beakerboy’s picture

Beakerboy
Primary language English
Location Kentucky
CreditAttribution: Beakerboy as a volunteer commented
Issue summary: View changes
daffie’s picture

daffie CreditAttribution: daffie commented
Status: Active » Needs work

Hi @Beakerboy: In both cases you are right. Only fixing either one of them is a lot of work. My question to you is: is it necessary for your SQL-server database driver or do you have some use-case where one or both cases are necessary? If the answer is no, shall we then put our time and effort in something more usefull?
If you however want to fix this problem, then add the fixes for the by core supported databases and I will review the patch.

Beakerboy’s picture

Beakerboy
Primary language English
Location Kentucky
CreditAttribution: Beakerboy as a volunteer commented

My motivation is to simplify the sqlsrv driver. The createTable() function has some code which fixes the curly brace bug, and includes the comment:


  // $this->createTableSql already prefixes the table name, and we must
    // inhibit prefixing at the query level because field
    // default_context_menu_block_active_values definitions can contain string
    // literals with braces.

my thought on this is, if that situation is a bug that affects users, It should be fixed in core not at the sqlsrv level. Now that I have verified that it is an issue in core, I plan on dropping the special handling in sqlsrv to make it simpler.

The semicolon bug doesn’t affect me either. For some reason Microsoft insists that any merge statement must terminate with a semicolon. To accommodate this I had to change the way the query function trims the end of the query string, and I imagined that semicolons in default values would probably lead to an exception.

I’m not itching to fix these myself, but since there has been a lot of very good discussion about the database abstraction layer lately, I figured we could have some smart people brainstorm on the best solution to at least have a plan in place.

My thought is to move all of the table creation code out of the Schema class and into a Create class, similar to Select, Insert, Delete, etc. this would shrink the Schema class, and allow each driver to manage its own toString() and execute() process.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

daffie’s picture

daffie CreditAttribution: daffie commented
Version: 8.9.x-dev » 9.3.x-dev
Issue tags: +Needs reroll
Related issues: +#3130579: Make Drupal\Core\Database\Schema work with reserved keywords for naming

Yes, these are bugs. Not sure if these can be fixed. Maybe relate to #3130579: Make Drupal\Core\Database\Schema work with reserved keywords for naming.

+++ b/core/tests/Drupal/KernelTests/Core/Database/InsertDefaultsTest.php
@@ -58,4 +58,37 @@ public function testDefaultInsertWithFields() {
+    $job = $this->connection->query('SELECT job FROM {test_semicolon_default} WHERE id = :id', [':id' => $id])->fetchField();
...
+    $job = $this->connection->query('SELECT job FROM {test_bracket_default} WHERE id = :id', [':id' => $id])->fetchField();

Can we change this to $this->connection->select()

daffie’s picture

daffie CreditAttribution: daffie commented
Issue tags: +Bug Smash Initiative
ankithashetty’s picture

ankithashetty
Location Karnataka, India
CreditAttribution: ankithashetty as a volunteer and at Specbee commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3130333-12.patch2.39 KB
reroll_diff_3130333_3-12.txt3.4 KB

Re-rolled the patch in #3 and made the following changes to it:

Thank you!

Status: Needs review » Needs work

The last submitted patch, 12: 3130333-12.patch, failed testing. View results

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.