Skip to content

Commit 7f74c08

Browse files
JanJakesJanJakes
committed
Improve and document DEFAULT and CHECK expression formatting
1 parent 08ab6da commit 7f74c08

4 files changed

Lines changed: 83 additions & 48 deletions

File tree

‎tests/WP_SQLite_Driver_Metadata_Tests.php‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,43 +2147,43 @@ public function testInformationSchemaCheckConstraints(): void {
21472147
'CONSTRAINT_CATALOG' => 'def',
21482148
'CONSTRAINT_SCHEMA' => 'wp',
21492149
'CONSTRAINT_NAME' => 'c1',
2150-
'CHECK_CLAUSE' => '( id < 10 )',
2150+
'CHECK_CLAUSE' => 'id < 10',
21512151
),
21522152
(object) array(
21532153
'CONSTRAINT_CATALOG' => 'def',
21542154
'CONSTRAINT_SCHEMA' => 'wp',
21552155
'CONSTRAINT_NAME' => 'c2',
2156-
'CHECK_CLAUSE' => '( start_timestamp < end_timestamp )',
2156+
'CHECK_CLAUSE' => 'start_timestamp < end_timestamp',
21572157
),
21582158
(object) array(
21592159
'CONSTRAINT_CATALOG' => 'def',
21602160
'CONSTRAINT_SCHEMA' => 'wp',
21612161
'CONSTRAINT_NAME' => 'c3',
2162-
'CHECK_CLAUSE' => '( length ( data ) < 20 )',
2162+
'CHECK_CLAUSE' => 'length(data)< 20',
21632163
),
21642164
(object) array(
21652165
'CONSTRAINT_CATALOG' => 'def',
21662166
'CONSTRAINT_SCHEMA' => 'wp',
21672167
'CONSTRAINT_NAME' => 't_chk_1',
2168-
'CHECK_CLAUSE' => '( id > 0 )',
2168+
'CHECK_CLAUSE' => 'id > 0',
21692169
),
21702170
(object) array(
21712171
'CONSTRAINT_CATALOG' => 'def',
21722172
'CONSTRAINT_SCHEMA' => 'wp',
21732173
'CONSTRAINT_NAME' => 't_chk_2',
2174-
'CHECK_CLAUSE' => "( name != '' )",
2174+
'CHECK_CLAUSE' => "name != ''",
21752175
),
21762176
(object) array(
21772177
'CONSTRAINT_CATALOG' => 'def',
21782178
'CONSTRAINT_SCHEMA' => 'wp',
21792179
'CONSTRAINT_NAME' => 't_chk_3',
2180-
'CHECK_CLAUSE' => '( score > 0 AND score < 100 )',
2180+
'CHECK_CLAUSE' => 'score > 0 AND score < 100',
21812181
),
21822182
(object) array(
21832183
'CONSTRAINT_CATALOG' => 'def',
21842184
'CONSTRAINT_SCHEMA' => 'wp',
21852185
'CONSTRAINT_NAME' => 't_chk_4',
2186-
'CHECK_CLAUSE' => '( json_valid ( data ) )',
2186+
'CHECK_CLAUSE' => 'json_valid(data)',
21872187
),
21882188
),
21892189
$result
@@ -2233,13 +2233,13 @@ public function testInformationSchemaAlterTableAddCheckConstraint(): void {
22332233
'CONSTRAINT_CATALOG' => 'def',
22342234
'CONSTRAINT_SCHEMA' => 'wp',
22352235
'CONSTRAINT_NAME' => 'c',
2236-
'CHECK_CLAUSE' => '( id > 0 )',
2236+
'CHECK_CLAUSE' => 'id > 0',
22372237
),
22382238
(object) array(
22392239
'CONSTRAINT_CATALOG' => 'def',
22402240
'CONSTRAINT_SCHEMA' => 'wp',
22412241
'CONSTRAINT_NAME' => 't_chk_1',
2242-
'CHECK_CLAUSE' => '( id < 10 )',
2242+
'CHECK_CLAUSE' => 'id < 10',
22432243
),
22442244
),
22452245
$result

‎tests/WP_SQLite_Driver_Tests.php‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9219,13 +9219,13 @@ public function testCheckConstraints(): void {
92199219

92209220
// The of the check expressions below is not 100% matching MySQL,
92219221
// because in MySQL the expressions are parsed and normalized.
9222-
' CONSTRAINT `c1` CHECK ( id < 10 ),',
9223-
' CONSTRAINT `c2` CHECK ( start_timestamp < end_timestamp ),',
9224-
' CONSTRAINT `c3` CHECK ( length ( data ) < 20 ),',
9225-
' CONSTRAINT `t_chk_1` CHECK ( id > 0 ),',
9226-
" CONSTRAINT `t_chk_2` CHECK ( name != '' ),",
9227-
' CONSTRAINT `t_chk_3` CHECK ( score > 0 AND score < 100 ),',
9228-
' CONSTRAINT `t_chk_4` CHECK ( json_valid ( data ) )',
9222+
' CONSTRAINT `c1` CHECK (id < 10),',
9223+
' CONSTRAINT `c2` CHECK (start_timestamp < end_timestamp),',
9224+
' CONSTRAINT `c3` CHECK (length(data)< 20),',
9225+
' CONSTRAINT `t_chk_1` CHECK (id > 0),',
9226+
" CONSTRAINT `t_chk_2` CHECK (name != ''),",
9227+
' CONSTRAINT `t_chk_3` CHECK (score > 0 AND score < 100),',
9228+
' CONSTRAINT `t_chk_4` CHECK (json_valid(data))',
92299229
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
92309230
)
92319231
),
@@ -9251,8 +9251,8 @@ public function testAlterTableAddCheckConstraint(): void {
92519251
array(
92529252
'CREATE TABLE `t` (',
92539253
' `id` int DEFAULT NULL,',
9254-
' CONSTRAINT `c` CHECK ( id > 0 ),',
9255-
' CONSTRAINT `t_chk_1` CHECK ( id < 10 )',
9254+
' CONSTRAINT `c` CHECK (id > 0),',
9255+
' CONSTRAINT `t_chk_1` CHECK (id < 10)',
92569256
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
92579257
)
92589258
),
@@ -9321,7 +9321,7 @@ public function testCheckConstraintNotEnforced(): void {
93219321
array(
93229322
'CREATE TABLE `t` (',
93239323
' `id` int DEFAULT NULL,',
9324-
' CONSTRAINT `c` CHECK ( id > 0 ) /*!80016 NOT ENFORCED */',
9324+
' CONSTRAINT `c` CHECK (id > 0) /*!80016 NOT ENFORCED */',
93259325
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
93269326
)
93279327
),
@@ -11326,7 +11326,7 @@ public function testCreateTableWithDefaultNowFunction(): void {
1132611326
array(
1132711327
'CREATE TABLE `test_now_default` (',
1132811328
' `id` int NOT NULL,',
11329-
' `updated` timestamp NOT NULL DEFAULT ( now( ) ) ON UPDATE CURRENT_TIMESTAMP',
11329+
' `updated` timestamp NOT NULL DEFAULT (now()) ON UPDATE CURRENT_TIMESTAMP',
1133011330
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_520_ci',
1133111331
)
1133211332
),
@@ -11351,7 +11351,7 @@ public function testCreateTableWithDefaultNowFunction(): void {
1135111351
'Type' => 'timestamp',
1135211352
'Null' => 'NO',
1135311353
'Key' => '',
11354-
'Default' => '( now( ) )',
11354+
'Default' => 'now()',
1135511355
'Extra' => 'DEFAULT_GENERATED on update CURRENT_TIMESTAMP',
1135611356
),
1135711357
),
@@ -11391,9 +11391,9 @@ public function testCreateTableWithDefaultExpressions(): void {
1139111391
array(
1139211392
'CREATE TABLE `t` (',
1139311393
' `id` int NOT NULL,',
11394-
' `col1` int NOT NULL DEFAULT ( 1 + 2 ),',
11395-
' `col2` datetime NOT NULL DEFAULT ( DATE_ADD( NOW( ) , INTERVAL 1 YEAR ) ),',
11396-
" `col3` varchar(255) NOT NULL DEFAULT ( CONCAT( 'a' , 'b' ) )",
11394+
' `col1` int NOT NULL DEFAULT (1 + 2),',
11395+
' `col2` datetime NOT NULL DEFAULT (DATE_ADD(NOW(), INTERVAL 1 YEAR)),',
11396+
" `col3` varchar(255) NOT NULL DEFAULT (CONCAT('a' , 'b'))",
1139711397
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
1139811398
)
1139911399
),
@@ -11418,23 +11418,23 @@ public function testCreateTableWithDefaultExpressions(): void {
1141811418
'Type' => 'int',
1141911419
'Null' => 'NO',
1142011420
'Key' => '',
11421-
'Default' => '( 1 + 2 )',
11421+
'Default' => '1 + 2',
1142211422
'Extra' => 'DEFAULT_GENERATED',
1142311423
),
1142411424
(object) array(
1142511425
'Field' => 'col2',
1142611426
'Type' => 'datetime',
1142711427
'Null' => 'NO',
1142811428
'Key' => '',
11429-
'Default' => '( DATE_ADD( NOW( ) , INTERVAL 1 YEAR ) )',
11429+
'Default' => 'DATE_ADD(NOW(), INTERVAL 1 YEAR)',
1143011430
'Extra' => 'DEFAULT_GENERATED',
1143111431
),
1143211432
(object) array(
1143311433
'Field' => 'col3',
1143411434
'Type' => 'varchar(255)',
1143511435
'Null' => 'NO',
1143611436
'Key' => '',
11437-
'Default' => "( CONCAT( 'a' , 'b' ) )",
11437+
'Default' => "CONCAT('a' , 'b')",
1143811438
'Extra' => 'DEFAULT_GENERATED',
1143911439
),
1144011440
),

‎wp-includes/sqlite-ast/class-wp-pdo-mysql-on-sqlite.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5707,7 +5707,7 @@ private function get_sqlite_create_table_statement(
57075707
$ast = $this->create_parser( 'SELECT ' . $column['COLUMN_DEFAULT'] )->parse();
57085708
$expr = $ast->get_first_descendant_node( 'selectItem' )->get_first_child_node();
57095709
$default_clause = $this->translate( $expr );
5710-
$query .= ' DEFAULT ' . $default_clause;
5710+
$query .= sprintf( ' DEFAULT (%s)', $default_clause );
57115711
} else {
57125712
$query .= ' DEFAULT ' . $this->connection->quote( $column['COLUMN_DEFAULT'] );
57135713
}
@@ -5835,7 +5835,7 @@ function ( $column ) {
58355835
$check_clause = $this->translate( $expr );
58365836

58375837
$sql = sprintf(
5838-
' CONSTRAINT %s CHECK %s',
5838+
' CONSTRAINT %s CHECK (%s)',
58395839
$this->quote_sqlite_identifier( $check_constraint['CONSTRAINT_NAME'] ),
58405840
$check_clause
58415841
);
@@ -6012,7 +6012,7 @@ private function get_mysql_create_table_statement( bool $table_is_temporary, str
60126012
$sql .= ' DEFAULT CURRENT_TIMESTAMP';
60136013
} elseif ( null !== $column['COLUMN_DEFAULT'] ) {
60146014
if ( str_contains( $column['EXTRA'], 'DEFAULT_GENERATED' ) ) {
6015-
$sql .= ' DEFAULT ' . $column['COLUMN_DEFAULT'];
6015+
$sql .= sprintf( ' DEFAULT (%s)', $column['COLUMN_DEFAULT'] );
60166016
} else {
60176017
$sql .= ' DEFAULT ' . $this->quote_mysql_utf8_string_literal( $column['COLUMN_DEFAULT'] );
60186018
}
@@ -6119,7 +6119,7 @@ function ( $column ) {
61196119
// 9. Add CHECK constraints.
61206120
foreach ( $check_constraints_info as $check_constraint ) {
61216121
$sql = sprintf(
6122-
' CONSTRAINT %s CHECK %s%s',
6122+
' CONSTRAINT %s CHECK (%s)%s',
61236123
$this->quote_mysql_identifier( $check_constraint['CONSTRAINT_NAME'] ),
61246124
$check_constraint['CHECK_CLAUSE'],
61256125
'NO' === $check_constraint['ENFORCED'] ? ' /*!80016 NOT ENFORCED */' : ''

‎wp-includes/sqlite-ast/class-wp-sqlite-information-schema-builder.php‎

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,10 +1912,7 @@ private function extract_check_constraint_data( WP_Parser_Node $node, string $ta
19121912
}
19131913

19141914
$expr = $check_constraint->get_first_child_node( 'exprWithParentheses' );
1915-
$check_clause = '';
1916-
foreach ( $expr->get_descendant_tokens() as $i => $token ) {
1917-
$check_clause .= ( $i > 0 ? ' ' : '' ) . $token->get_bytes();
1918-
}
1915+
$check_clause = $this->serialize_mysql_expression( $expr );
19191916

19201917
return array(
19211918
'constraint_schema' => self::SAVED_DATABASE_NAME,
@@ -2096,19 +2093,7 @@ private function get_column_default( WP_Parser_Node $node ): ?string {
20962093
// DEFAULT (expression) - MySQL 8.0.13+ supports exprWithParentheses
20972094
$expr_with_parens = $default_attr->get_first_child_node( 'exprWithParentheses' );
20982095
if ( $expr_with_parens ) {
2099-
$default_clause = '';
2100-
foreach ( $expr_with_parens->get_descendant_tokens() as $i => $token ) {
2101-
if ( WP_MySQL_Lexer::OPEN_PAR_SYMBOL === $token->id ) {
2102-
// TODO: This is just a quick fix to avoid inserting whitespace
2103-
// before '(', which would break function call expressions.
2104-
// The proper fix is to implement a "$node->get_bytes()" API.
2105-
// This same applies to the CHECK (expression) case as well.
2106-
$default_clause .= $token->get_bytes();
2107-
} else {
2108-
$default_clause .= ( $i > 0 ? ' ' : '' ) . $token->get_bytes();
2109-
}
2110-
}
2111-
return $default_clause;
2096+
return $this->serialize_mysql_expression( $expr_with_parens );
21122097
}
21132098

21142099
throw new Exception( 'DEFAULT value of this type is not supported.' );
@@ -3034,6 +3019,56 @@ private function get_value( WP_Parser_Node $node ): string {
30343019
return $full_value;
30353020
}
30363021

3022+
/**
3023+
* Serialize a MySQL expression for storing in the information schema.
3024+
*
3025+
* This is used for storing DEFAULT and CHECK expressions in the database.
3026+
*
3027+
* The current implementation is using a naive approach based on directly
3028+
* joining the original expression token bytes. This is safe, beacuase the
3029+
* original tokens must comprise a valid expression. While functionally
3030+
* equivalent, it is not strictly identical to what MySQL stores, because
3031+
* MySQL normalizes and prints the expression in a specific format.
3032+
*
3033+
* TODO: Consider implementing a MySQL expression node -> string formatter
3034+
* that would produce results that are identical to MySQL formatting.
3035+
* This gets tricky from MySQL 8, where a double-escaping regression
3036+
* was introduced, storing strings like "_utf8mb4\'abc\'" instead of
3037+
* "_utf8mb4'abc'", but displaying them correctly in SHOW statements.
3038+
* @see https://bugs.mysql.com/bug.php?id=100607
3039+
*
3040+
* @param WP_Parser_Node $node The AST node that needs to be serialized.
3041+
* @return string The serialized value of the node.
3042+
*/
3043+
private function serialize_mysql_expression( WP_Parser_Node $node ): string {
3044+
// The wrapping parentheses are generally not stored, although in MySQL,
3045+
// this varies by expression type as per the expression formatter logic.
3046+
if ( 'exprWithParentheses' === $node->rule_name ) {
3047+
return $this->serialize_mysql_expression( $node->get_first_child_node( 'expr' ) );
3048+
}
3049+
3050+
$value = '';
3051+
$last_token_id = null;
3052+
foreach ( $node->get_descendant_tokens() as $i => $token ) {
3053+
// Do not insert whitespace around parentheses. This is primarily to
3054+
// avoid inserting whitespace before '(', which may break function
3055+
// calls, depending on the value of the "IGNORE_SPACE" SQL mode.
3056+
if (
3057+
0 === $i
3058+
|| WP_MySQL_Lexer::OPEN_PAR_SYMBOL === $token->id
3059+
|| WP_MySQL_Lexer::CLOSE_PAR_SYMBOL === $token->id
3060+
|| WP_MySQL_Lexer::OPEN_PAR_SYMBOL === $last_token_id
3061+
|| WP_MySQL_Lexer::CLOSE_PAR_SYMBOL === $last_token_id
3062+
) {
3063+
$value .= $token->get_bytes();
3064+
} else {
3065+
$value .= ' ' . $token->get_bytes();
3066+
}
3067+
$last_token_id = $token->id;
3068+
}
3069+
return $value;
3070+
}
3071+
30373072
/**
30383073
* Insert values into an SQLite table.
30393074
*

0 commit comments

Comments
 (0)