Skip to content

Commit d62e028

Browse files
JanJakesJanJakes
committed
Improve and document DEFAULT and CHECK expression formatting
1 parent 2535825 commit d62e028

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
@@ -2176,43 +2176,43 @@ public function testInformationSchemaCheckConstraints(): void {
21762176
'CONSTRAINT_CATALOG' => 'def',
21772177
'CONSTRAINT_SCHEMA' => 'wp',
21782178
'CONSTRAINT_NAME' => 'c1',
2179-
'CHECK_CLAUSE' => '( id < 10 )',
2179+
'CHECK_CLAUSE' => 'id < 10',
21802180
),
21812181
(object) array(
21822182
'CONSTRAINT_CATALOG' => 'def',
21832183
'CONSTRAINT_SCHEMA' => 'wp',
21842184
'CONSTRAINT_NAME' => 'c2',
2185-
'CHECK_CLAUSE' => '( start_timestamp < end_timestamp )',
2185+
'CHECK_CLAUSE' => 'start_timestamp < end_timestamp',
21862186
),
21872187
(object) array(
21882188
'CONSTRAINT_CATALOG' => 'def',
21892189
'CONSTRAINT_SCHEMA' => 'wp',
21902190
'CONSTRAINT_NAME' => 'c3',
2191-
'CHECK_CLAUSE' => '( length ( data ) < 20 )',
2191+
'CHECK_CLAUSE' => 'length(data)< 20',
21922192
),
21932193
(object) array(
21942194
'CONSTRAINT_CATALOG' => 'def',
21952195
'CONSTRAINT_SCHEMA' => 'wp',
21962196
'CONSTRAINT_NAME' => 't_chk_1',
2197-
'CHECK_CLAUSE' => '( id > 0 )',
2197+
'CHECK_CLAUSE' => 'id > 0',
21982198
),
21992199
(object) array(
22002200
'CONSTRAINT_CATALOG' => 'def',
22012201
'CONSTRAINT_SCHEMA' => 'wp',
22022202
'CONSTRAINT_NAME' => 't_chk_2',
2203-
'CHECK_CLAUSE' => "( name != '' )",
2203+
'CHECK_CLAUSE' => "name != ''",
22042204
),
22052205
(object) array(
22062206
'CONSTRAINT_CATALOG' => 'def',
22072207
'CONSTRAINT_SCHEMA' => 'wp',
22082208
'CONSTRAINT_NAME' => 't_chk_3',
2209-
'CHECK_CLAUSE' => '( score > 0 AND score < 100 )',
2209+
'CHECK_CLAUSE' => 'score > 0 AND score < 100',
22102210
),
22112211
(object) array(
22122212
'CONSTRAINT_CATALOG' => 'def',
22132213
'CONSTRAINT_SCHEMA' => 'wp',
22142214
'CONSTRAINT_NAME' => 't_chk_4',
2215-
'CHECK_CLAUSE' => '( json_valid ( data ) )',
2215+
'CHECK_CLAUSE' => 'json_valid(data)',
22162216
),
22172217
),
22182218
$result
@@ -2262,13 +2262,13 @@ public function testInformationSchemaAlterTableAddCheckConstraint(): void {
22622262
'CONSTRAINT_CATALOG' => 'def',
22632263
'CONSTRAINT_SCHEMA' => 'wp',
22642264
'CONSTRAINT_NAME' => 'c',
2265-
'CHECK_CLAUSE' => '( id > 0 )',
2265+
'CHECK_CLAUSE' => 'id > 0',
22662266
),
22672267
(object) array(
22682268
'CONSTRAINT_CATALOG' => 'def',
22692269
'CONSTRAINT_SCHEMA' => 'wp',
22702270
'CONSTRAINT_NAME' => 't_chk_1',
2271-
'CHECK_CLAUSE' => '( id < 10 )',
2271+
'CHECK_CLAUSE' => 'id < 10',
22722272
),
22732273
),
22742274
$result

‎tests/WP_SQLite_Driver_Tests.php‎

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

92959295
// The of the check expressions below is not 100% matching MySQL,
92969296
// because in MySQL the expressions are parsed and normalized.
9297-
' CONSTRAINT `c1` CHECK ( id < 10 ),',
9298-
' CONSTRAINT `c2` CHECK ( start_timestamp < end_timestamp ),',
9299-
' CONSTRAINT `c3` CHECK ( length ( data ) < 20 ),',
9300-
' CONSTRAINT `t_chk_1` CHECK ( id > 0 ),',
9301-
" CONSTRAINT `t_chk_2` CHECK ( name != '' ),",
9302-
' CONSTRAINT `t_chk_3` CHECK ( score > 0 AND score < 100 ),',
9303-
' CONSTRAINT `t_chk_4` CHECK ( json_valid ( data ) )',
9297+
' CONSTRAINT `c1` CHECK (id < 10),',
9298+
' CONSTRAINT `c2` CHECK (start_timestamp < end_timestamp),',
9299+
' CONSTRAINT `c3` CHECK (length(data)< 20),',
9300+
' CONSTRAINT `t_chk_1` CHECK (id > 0),',
9301+
" CONSTRAINT `t_chk_2` CHECK (name != ''),",
9302+
' CONSTRAINT `t_chk_3` CHECK (score > 0 AND score < 100),',
9303+
' CONSTRAINT `t_chk_4` CHECK (json_valid(data))',
93049304
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
93059305
)
93069306
),
@@ -9326,8 +9326,8 @@ public function testAlterTableAddCheckConstraint(): void {
93269326
array(
93279327
'CREATE TABLE `t` (',
93289328
' `id` int DEFAULT NULL,',
9329-
' CONSTRAINT `c` CHECK ( id > 0 ),',
9330-
' CONSTRAINT `t_chk_1` CHECK ( id < 10 )',
9329+
' CONSTRAINT `c` CHECK (id > 0),',
9330+
' CONSTRAINT `t_chk_1` CHECK (id < 10)',
93319331
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
93329332
)
93339333
),
@@ -9396,7 +9396,7 @@ public function testCheckConstraintNotEnforced(): void {
93969396
array(
93979397
'CREATE TABLE `t` (',
93989398
' `id` int DEFAULT NULL,',
9399-
' CONSTRAINT `c` CHECK ( id > 0 ) /*!80016 NOT ENFORCED */',
9399+
' CONSTRAINT `c` CHECK (id > 0) /*!80016 NOT ENFORCED */',
94009400
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
94019401
)
94029402
),
@@ -11401,7 +11401,7 @@ public function testCreateTableWithDefaultNowFunction(): void {
1140111401
array(
1140211402
'CREATE TABLE `test_now_default` (',
1140311403
' `id` int NOT NULL,',
11404-
' `updated` timestamp NOT NULL DEFAULT ( now( ) ) ON UPDATE CURRENT_TIMESTAMP',
11404+
' `updated` timestamp NOT NULL DEFAULT (now()) ON UPDATE CURRENT_TIMESTAMP',
1140511405
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_520_ci',
1140611406
)
1140711407
),
@@ -11426,7 +11426,7 @@ public function testCreateTableWithDefaultNowFunction(): void {
1142611426
'Type' => 'timestamp',
1142711427
'Null' => 'NO',
1142811428
'Key' => '',
11429-
'Default' => '( now( ) )',
11429+
'Default' => 'now()',
1143011430
'Extra' => 'DEFAULT_GENERATED on update CURRENT_TIMESTAMP',
1143111431
),
1143211432
),
@@ -11466,9 +11466,9 @@ public function testCreateTableWithDefaultExpressions(): void {
1146611466
array(
1146711467
'CREATE TABLE `t` (',
1146811468
' `id` int NOT NULL,',
11469-
' `col1` int NOT NULL DEFAULT ( 1 + 2 ),',
11470-
' `col2` datetime NOT NULL DEFAULT ( DATE_ADD( NOW( ) , INTERVAL 1 YEAR ) ),',
11471-
" `col3` varchar(255) NOT NULL DEFAULT ( CONCAT( 'a' , 'b' ) )",
11469+
' `col1` int NOT NULL DEFAULT (1 + 2),',
11470+
' `col2` datetime NOT NULL DEFAULT (DATE_ADD(NOW(), INTERVAL 1 YEAR)),',
11471+
" `col3` varchar(255) NOT NULL DEFAULT (CONCAT('a' , 'b'))",
1147211472
') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
1147311473
)
1147411474
),
@@ -11493,23 +11493,23 @@ public function testCreateTableWithDefaultExpressions(): void {
1149311493
'Type' => 'int',
1149411494
'Null' => 'NO',
1149511495
'Key' => '',
11496-
'Default' => '( 1 + 2 )',
11496+
'Default' => '1 + 2',
1149711497
'Extra' => 'DEFAULT_GENERATED',
1149811498
),
1149911499
(object) array(
1150011500
'Field' => 'col2',
1150111501
'Type' => 'datetime',
1150211502
'Null' => 'NO',
1150311503
'Key' => '',
11504-
'Default' => '( DATE_ADD( NOW( ) , INTERVAL 1 YEAR ) )',
11504+
'Default' => 'DATE_ADD(NOW(), INTERVAL 1 YEAR)',
1150511505
'Extra' => 'DEFAULT_GENERATED',
1150611506
),
1150711507
(object) array(
1150811508
'Field' => 'col3',
1150911509
'Type' => 'varchar(255)',
1151011510
'Null' => 'NO',
1151111511
'Key' => '',
11512-
'Default' => "( CONCAT( 'a' , 'b' ) )",
11512+
'Default' => "CONCAT('a' , 'b')",
1151311513
'Extra' => 'DEFAULT_GENERATED',
1151411514
),
1151511515
),

‎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
@@ -5921,7 +5921,7 @@ private function get_sqlite_create_table_statement(
59215921
$ast = $this->create_parser( 'SELECT ' . $column['COLUMN_DEFAULT'] )->parse();
59225922
$expr = $ast->get_first_descendant_node( 'selectItem' )->get_first_child_node();
59235923
$default_clause = $this->translate( $expr );
5924-
$query .= ' DEFAULT ' . $default_clause;
5924+
$query .= sprintf( ' DEFAULT (%s)', $default_clause );
59255925
} else {
59265926
$query .= ' DEFAULT ' . $this->quote_sqlite_value( $column['COLUMN_DEFAULT'] );
59275927
}
@@ -6049,7 +6049,7 @@ function ( $column ) {
60496049
$check_clause = $this->translate( $expr );
60506050

60516051
$sql = sprintf(
6052-
' CONSTRAINT %s CHECK %s',
6052+
' CONSTRAINT %s CHECK (%s)',
60536053
$this->quote_sqlite_identifier( $check_constraint['CONSTRAINT_NAME'] ),
60546054
$check_clause
60556055
);
@@ -6226,7 +6226,7 @@ private function get_mysql_create_table_statement( bool $table_is_temporary, str
62266226
$sql .= ' DEFAULT CURRENT_TIMESTAMP';
62276227
} elseif ( null !== $column['COLUMN_DEFAULT'] ) {
62286228
if ( str_contains( $column['EXTRA'], 'DEFAULT_GENERATED' ) ) {
6229-
$sql .= ' DEFAULT ' . $column['COLUMN_DEFAULT'];
6229+
$sql .= sprintf( ' DEFAULT (%s)', $column['COLUMN_DEFAULT'] );
62306230
} else {
62316231
$sql .= ' DEFAULT ' . $this->quote_mysql_utf8_string_literal( $column['COLUMN_DEFAULT'] );
62326232
}
@@ -6333,7 +6333,7 @@ function ( $column ) {
63336333
// 9. Add CHECK constraints.
63346334
foreach ( $check_constraints_info as $check_constraint ) {
63356335
$sql = sprintf(
6336-
' CONSTRAINT %s CHECK %s%s',
6336+
' CONSTRAINT %s CHECK (%s)%s',
63376337
$this->quote_mysql_identifier( $check_constraint['CONSTRAINT_NAME'] ),
63386338
$check_constraint['CHECK_CLAUSE'],
63396339
'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)