Copilot commented on code in PR #10860: URL: https://github.com/apache/gravitino/pull/10860#discussion_r3135921331
########## scripts/h2/upgrade-1.2.0-to-1.3.0-h2.sql: ########## @@ -0,0 +1,42 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file-- +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"). You may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. +-- + +ALTER TABLE `view_meta` + ADD COLUMN `audit_info` CLOB NOT NULL COMMENT 'view audit info' AFTER `schema_id`; Review Comment: The `AFTER `schema_id`` clause in the H2 upgrade script is a MySQL-specific column-ordering feature and is not used in other H2 upgrade scripts in this repo (e.g., `scripts/h2/upgrade-0.8.0-to-0.9.0-h2.sql:21`, `scripts/h2/upgrade-0.9.0-to-1.0.0-h2.sql:63`). To keep the H2 script maximally compatible, consider dropping the `AFTER ...` portion (column order is not semantically relevant). ```suggestion ADD COLUMN `audit_info` CLOB NOT NULL COMMENT 'view audit info'; ``` ########## scripts/h2/upgrade-1.2.0-to-1.3.0-h2.sql: ########## @@ -0,0 +1,42 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file-- +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"). You may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. +-- + +ALTER TABLE `view_meta` + ADD COLUMN `audit_info` CLOB NOT NULL COMMENT 'view audit info' AFTER `schema_id`; + Review Comment: Adding `audit_info` as `NOT NULL` on an existing `view_meta` table can fail during upgrade if there are already rows (the new column would be NULL for existing records). Consider a two-step migration: add the column as nullable, backfill existing rows (e.g., to an empty JSON object string), then alter the column to `NOT NULL` (consistent with the compatibility approach used in `scripts/h2/upgrade-0.8.0-to-0.9.0-h2.sql:20-25`). ```suggestion ADD COLUMN `audit_info` CLOB DEFAULT NULL COMMENT 'view audit info' AFTER `schema_id`; UPDATE `view_meta` SET `audit_info` = '{}' WHERE `audit_info` IS NULL; ALTER TABLE `view_meta` ALTER COLUMN `audit_info` SET NOT NULL; ``` ########## scripts/mysql/upgrade-1.2.0-to-1.3.0-mysql.sql: ########## @@ -0,0 +1,42 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file-- +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"). You may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. +-- + +ALTER TABLE `view_meta` + ADD COLUMN `audit_info` MEDIUMTEXT NOT NULL COMMENT 'view audit info' AFTER `schema_id`; + Review Comment: Adding `audit_info` as `NOT NULL` on an existing `view_meta` table can fail during upgrade if there are already rows (the new column would be NULL for existing records). Consider a two-step migration: add the column as nullable, backfill existing rows (e.g., to an empty JSON object string), then alter the column to `NOT NULL` (mirrors the compatibility approach used in `scripts/mysql/upgrade-0.8.0-to-0.9.0-mysql.sql:20-26`). ```suggestion ADD COLUMN `audit_info` MEDIUMTEXT DEFAULT NULL COMMENT 'view audit info' AFTER `schema_id`; UPDATE `view_meta` SET `audit_info` = '{}' WHERE `audit_info` IS NULL; ALTER TABLE `view_meta` MODIFY COLUMN `audit_info` MEDIUMTEXT NOT NULL COMMENT 'view audit info' AFTER `schema_id`; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
