Improve and document DEFAULT and CHECK expression formatting

Author: JanJakes

tests/WP_SQLite_Driver_Tests.php

@@ -9219,13 +9219,13 @@ public function testCheckConstraints(): void {
 
 					// The of the check expressions below is not 100% matching MySQL,
 					// because in MySQL the expressions are parsed and normalized.
-					'  CONSTRAINT `c1` CHECK ( id < 10 ),',
-					'  CONSTRAINT `c2` CHECK ( start_timestamp < end_timestamp ),',
-					'  CONSTRAINT `c3` CHECK ( length ( data ) < 20 ),',
-					'  CONSTRAINT `t_chk_1` CHECK ( id > 0 ),',
-					"  CONSTRAINT `t_chk_2` CHECK ( name != '' ),",
-					'  CONSTRAINT `t_chk_3` CHECK ( score > 0 AND score < 100 ),',
-					'  CONSTRAINT `t_chk_4` CHECK ( json_valid ( data ) )',
+					'  CONSTRAINT `c1` CHECK (id < 10),',
+					'  CONSTRAINT `c2` CHECK (start_timestamp < end_timestamp),',
+					'  CONSTRAINT `c3` CHECK (length(data)< 20),',
+					'  CONSTRAINT `t_chk_1` CHECK (id > 0),',
+					"  CONSTRAINT `t_chk_2` CHECK (name != ''),",
+					'  CONSTRAINT `t_chk_3` CHECK (score > 0 AND score < 100),',
+					'  CONSTRAINT `t_chk_4` CHECK (json_valid(data))',
 					') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
 				)
 			),
@@ -9251,8 +9251,8 @@ public function testAlterTableAddCheckConstraint(): void {
 				array(
 					'CREATE TABLE `t` (',
 					'  `id` int DEFAULT NULL,',
-					'  CONSTRAINT `c` CHECK ( id > 0 ),',
-					'  CONSTRAINT `t_chk_1` CHECK ( id < 10 )',
+					'  CONSTRAINT `c` CHECK (id > 0),',
+					'  CONSTRAINT `t_chk_1` CHECK (id < 10)',
 					') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
 				)
 			),
@@ -9321,7 +9321,7 @@ public function testCheckConstraintNotEnforced(): void {
 				array(
 					'CREATE TABLE `t` (',
 					'  `id` int DEFAULT NULL,',
-					'  CONSTRAINT `c` CHECK ( id > 0 ) /*!80016 NOT ENFORCED */',
+					'  CONSTRAINT `c` CHECK (id > 0) /*!80016 NOT ENFORCED */',
 					') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
 				)
 			),
@@ -11326,7 +11326,7 @@ public function testCreateTableWithDefaultNowFunction(): void {
 				array(
 					'CREATE TABLE `test_now_default` (',
 					'  `id` int NOT NULL,',
-					'  `updated` timestamp NOT NULL DEFAULT ( now( ) ) ON UPDATE CURRENT_TIMESTAMP',
+					'  `updated` timestamp NOT NULL DEFAULT (now()) ON UPDATE CURRENT_TIMESTAMP',
 					') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_520_ci',
 				)
 			),
@@ -11351,7 +11351,7 @@ public function testCreateTableWithDefaultNowFunction(): void {
 					'Type'    => 'timestamp',
 					'Null'    => 'NO',
 					'Key'     => '',
-					'Default' => '( now( ) )',
+					'Default' => '(now())',
 					'Extra'   => 'DEFAULT_GENERATED on update CURRENT_TIMESTAMP',
 				),
 			),
@@ -11391,9 +11391,9 @@ public function testCreateTableWithDefaultExpressions(): void {
 				array(
 					'CREATE TABLE `t` (',
 					'  `id` int NOT NULL,',
-					'  `col1` int NOT NULL DEFAULT ( 1 + 2 ),',
-					'  `col2` datetime NOT NULL DEFAULT ( DATE_ADD( NOW( ) , INTERVAL 1 YEAR ) ),',
-					"  `col3` varchar(255) NOT NULL DEFAULT ( CONCAT( 'a' , 'b' ) )",
+					'  `col1` int NOT NULL DEFAULT (1 + 2),',
+					'  `col2` datetime NOT NULL DEFAULT (DATE_ADD(NOW(), INTERVAL 1 YEAR)),',
+					"  `col3` varchar(255) NOT NULL DEFAULT (CONCAT('a' , 'b'))",
 					') ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci',
 				)
 			),
@@ -11418,23 +11418,23 @@ public function testCreateTableWithDefaultExpressions(): void {
 					'Type'    => 'int',
 					'Null'    => 'NO',
 					'Key'     => '',
-					'Default' => '( 1 + 2 )',
+					'Default' => '(1 + 2)',
 					'Extra'   => 'DEFAULT_GENERATED',
 				),
 				(object) array(
 					'Field'   => 'col2',
 					'Type'    => 'datetime',
 					'Null'    => 'NO',
 					'Key'     => '',
-					'Default' => '( DATE_ADD( NOW( ) , INTERVAL 1 YEAR ) )',
+					'Default' => '(DATE_ADD(NOW(), INTERVAL 1 YEAR))',
 					'Extra'   => 'DEFAULT_GENERATED',
 				),
 				(object) array(
 					'Field'   => 'col3',
 					'Type'    => 'varchar(255)',
 					'Null'    => 'NO',
 					'Key'     => '',
-					'Default' => "( CONCAT( 'a' , 'b' ) )",
+					'Default' => "(CONCAT('a' , 'b'))",
 					'Extra'   => 'DEFAULT_GENERATED',
 				),
 			),

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

@@ -1912,10 +1912,7 @@ private function extract_check_constraint_data( WP_Parser_Node $node, string $ta
 		}
 
 		$expr         = $check_constraint->get_first_child_node( 'exprWithParentheses' );
-		$check_clause = '';
-		foreach ( $expr->get_descendant_tokens() as $i => $token ) {
-			$check_clause .= ( $i > 0 ? ' ' : '' ) . $token->get_bytes();
-		}
+		$check_clause = $this->serialize_mysql_expression( $expr );
 
 		return array(
 			'constraint_schema' => self::SAVED_DATABASE_NAME,
@@ -2096,19 +2093,7 @@ private function get_column_default( WP_Parser_Node $node ): ?string {
 		// DEFAULT (expression) - MySQL 8.0.13+ supports exprWithParentheses
 		$expr_with_parens = $default_attr->get_first_child_node( 'exprWithParentheses' );
 		if ( $expr_with_parens ) {
-			$default_clause = '';
-			foreach ( $expr_with_parens->get_descendant_tokens() as $i => $token ) {
-				if ( WP_MySQL_Lexer::OPEN_PAR_SYMBOL === $token->id ) {
-					// TODO: This is just a quick fix to avoid inserting whitespace
-					//       before '(', which would break function call expressions.
-					//       The proper fix is to implement a "$node->get_bytes()" API.
-					//       This same applies to the CHECK (expression) case as well.
-					$default_clause .= $token->get_bytes();
-				} else {
-					$default_clause .= ( $i > 0 ? ' ' : '' ) . $token->get_bytes();
-				}
-			}
-			return $default_clause;
+			return $this->serialize_mysql_expression( $expr_with_parens );
 		}
 
 		throw new Exception( 'DEFAULT value of this type is not supported.' );
@@ -3034,6 +3019,50 @@ private function get_value( WP_Parser_Node $node ): string {
 		return $full_value;
 	}
 
+	/**
+	 * Serialize a MySQL expression for storing in the information schema.
+	 *
+	 * This is used for storing DEFAULT and CHECK expressions in the database.
+	 *
+	 * The current implementation is using a naive approach based on directly
+	 * joining the original expression token bytes. This is safe, beacuase the
+	 * original tokens must comprise a valid expression. While functionally
+	 * equivalent, it is not strictly identical to what MySQL stores, because
+	 * MySQL normalizes and prints the expression in a specific format.
+	 *
+	 * TODO: Consider implementing a full MySQL expression node -> string printer
+	 *       that would produce results that are identical to MySQL formatting.
+	 *       This gets tricky from MySQL 8, where a double-escaping regression
+	 *       was introduced, storing strings like "_utf8mb4\'abc\'" instead of
+	 *       "_utf8mb4'abc'", but displaying them correctly in SHOW statements.
+	 *       @see https://bugs.mysql.com/bug.php?id=100607
+	 *
+	 * @param  WP_Parser_Node $node The AST node that needs to be serialized.
+	 * @return string               The serialized value of the node.
+	 */
+	private function serialize_mysql_expression( WP_Parser_Node $node ): string {
+		$value         = '';
+		$last_token_id = null;
+		foreach ( $node->get_descendant_tokens() as $i => $token ) {
+			// Do not insert whitespace around parentheses. This is primarily to
+			// avoid inserting whitespace before '(', which may break function
+			// calls, depending on the value of the "IGNORE_SPACE" SQL mode.
+			if (
+				0 === $i
+				|| WP_MySQL_Lexer::OPEN_PAR_SYMBOL === $token->id
+				|| WP_MySQL_Lexer::CLOSE_PAR_SYMBOL === $token->id
+				|| WP_MySQL_Lexer::OPEN_PAR_SYMBOL === $last_token_id
+				|| WP_MySQL_Lexer::CLOSE_PAR_SYMBOL === $last_token_id
+			) {
+				$value .= $token->get_bytes();
+			} else {
+				$value .= ' ' . $token->get_bytes();
+			}
+			$last_token_id = $token->id;
+		}
+		return $value;
+	}
+
 	/**
 	 * Insert values into an SQLite table.
 	 *