Conversation
Walkthrough新增 SQL 存储过程 变更内容
Sequence Diagram(s)sequenceDiagram
participant Proc as UpgradeApplicationDevelopmentServiceVersion
participant MSG as ModelServiceVO
participant MSIG as ModelServiceInstanceGroupVO
participant ADS as ApplicationDevelopmentServiceVO
Proc->>MSG: 查询需修正的记录并从 installPath 提取版本
Proc->>MSG: 更新 ModelServiceVO.version（事务内）
Proc->>MSIG: 通过实例组定位对应 ModelService
Proc->>ADS: 同步 ModelServiceVO.version 到 ApplicationDevelopmentServiceVO.packageVersion
Proc->>Proc: 提交事务；若异常则回滚并 SIGNAL 错误
代码审查工作量评估🎯 4 (复杂) | ⏱️ ~45 分钟 庆祝诗句
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@conf/db/upgrade/V5.5.6__schema.sql`:
- Around line 163-168: The UPDATE uses the wrong JOIN key between
ApplicationDevelopmentServiceVO and ModelServiceInstanceGroupVO: change the join
ON condition from a.`uuid` = g.`uuid` to a.`modelServiceGroupUuid` = g.`uuid`,
and ensure the subsequent join uses g.`modelServiceUuid` = m.`uuid` (preserving
the SET a.`packageVersion` = m.`version` and the existing WHERE filters) so the
update follows the defined foreign key relationship.
🧹 Nitpick comments (1)
conf/db/upgrade/V5.5.6__schema.sql (1)
152-153: 幂等性通过 WHERE 条件实现，可接受但不如显式检查健壮。当前通过
WHERE version LIKE '%:%' OR version IS NULL和WHERE packageVersion IS NULL OR packageVersion LIKE '%:%'实现幂等性。这种方式在大多数情况下有效，但如果存在边界数据（例如 version 恰好为空字符串但不满足更新条件），可能会遗漏。基于 learnings：ZStack 数据库升级脚本通常通过
INFORMATION_SCHEMA检查来确保脚本可以安全重复执行。当前实现可以接受，但如果需要更高的健壮性，可考虑添加标记位或显式检查。Also applies to: 167-168
MatheMatrix
force-pushed
the
sync/ye.zou/ZSTAC-81309-p2@@2
branch
from
e276b82 to
375c0c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@conf/db/upgrade/V5.5.6__schema.sql`:
- Around line 132-135: Add idempotency by dropping the stored procedure before
creating it: insert a DROP PROCEDURE IF EXISTS
UpgradeApplicationDevelopmentServiceVersion statement immediately before the
CREATE PROCEDURE block (i.e., before the DELIMITER $$ and the definition of
UpgradeApplicationDevelopmentServiceVersion) so the upgrade script can be safely
re-run even if a previous partial run left the procedure defined.
- Around line 145-160: The current UPDATE on zstack.ModelServiceVO sets
`version` from the tail of `installPath` but the REGEXP check only verifies the
start, so values like "1.11.1.qcow2" or "2.5.3-rc1" get written whole; change
the assignment and predicate to extract the pure semantic version using
REGEXP_SUBSTR on the same SUBSTRING_INDEX(...) expression (e.g., pattern
'[0-9]+(\\.[0-9]+){1,2}') and only UPDATE rows where REGEXP_SUBSTR(...) IS NOT
NULL; ensure you reference the same expression for both SET `version` and the
WHERE filter and verify target DB supports REGEXP_SUBSTR (MySQL 8.0+).
| DELIMITER $$ | ||
|
|
||
| CREATE PROCEDURE UpgradeApplicationDevelopmentServiceVersion() | ||
| BEGIN |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "V5.5.6__schema.sql" --type fRepository: MatheMatrix/zstack
Length of output: 96
🏁 Script executed:
cat -n conf/db/upgrade/V5.5.6__schema.sql | sed -n '125,180p'Repository: MatheMatrix/zstack
Length of output: 2689
在创建存储过程前添加 DROP PROCEDURE IF EXISTS 保证升级脚本幂等性
当前存储过程缺少幂等性保护。如果第 174 行的 CALL 执行失败，第 175 行的 DROP 不会被执行，下一次运行升级脚本时会在第 134 行因过程已存在而失败。建议在第 132 行 DELIMITER 前添加 DROP PROCEDURE IF EXISTS 以确保升级脚本可以安全重复执行。
建议改动
+DROP PROCEDURE IF EXISTS UpgradeApplicationDevelopmentServiceVersion;
DELIMITER $$
CREATE PROCEDURE UpgradeApplicationDevelopmentServiceVersion()🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.6__schema.sql` around lines 132 - 135, Add idempotency
by dropping the stored procedure before creating it: insert a DROP PROCEDURE IF
EXISTS UpgradeApplicationDevelopmentServiceVersion statement immediately before
the CREATE PROCEDURE block (i.e., before the DELIMITER $$ and the definition of
UpgradeApplicationDevelopmentServiceVersion) so the upgrade script can be safely
re-run even if a previous partial run left the procedure defined.
| UPDATE `zstack`.`ModelServiceVO` | ||
| SET `version` = SUBSTRING_INDEX( | ||
| SUBSTRING_INDEX( | ||
| TRIM(TRAILING '/' FROM REPLACE(REPLACE(`installPath`, 'file://', ''), 'file:', '')), | ||
| '/', -1 | ||
| ), '_', -1 | ||
| ) | ||
| WHERE `type` = 'App' | ||
| AND (`version` LIKE '%:%' OR `version` IS NULL OR `version` = '') | ||
| AND `installPath` IS NOT NULL AND TRIM(`installPath`) != '' | ||
| AND SUBSTRING_INDEX( | ||
| SUBSTRING_INDEX( | ||
| TRIM(TRAILING '/' FROM REPLACE(REPLACE(`installPath`, 'file://', ''), 'file:', '')), | ||
| '/', -1 | ||
| ), '_', -1 | ||
| ) REGEXP '^[0-9]+\\.[0-9]+'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "V5.5.6__schema.sql" -type fRepository: MatheMatrix/zstack
Length of output: 98
🏁 Script executed:
# Check project structure and identify target database versions
find . -name "*.properties" -o -name "*.yml" -o -name "*.yaml" | head -20Repository: MatheMatrix/zstack
Length of output: 1168
🏁 Script executed:
# Search for database version references in the project
rg "MySQL|GreatSQL|version" -t properties -t yml -t yaml | grep -i "mysql\|version" | head -20Repository: MatheMatrix/zstack
Length of output: 96
🏁 Script executed:
# Read the actual SQL file to verify the code at lines 145-160
sed -n '140,165p' ./conf/db/upgrade/V5.5.6__schema.sqlRepository: MatheMatrix/zstack
Length of output: 1247
🏁 Script executed:
# Search for MySQL version requirements in the project
rg "MySQL|mysql|version" ./conf -A 2 -B 2 | head -50Repository: MatheMatrix/zstack
Length of output: 3776
🏁 Script executed:
# Look for Flyway or migration version information
find . -name "*.sql" -type f | head -5
git ls-files | grep -E "(README|CHANGELOG|VERSION)" | head -10Repository: MatheMatrix/zstack
Length of output: 343
🏁 Script executed:
# Check VERSION file and MySQL requirements
cat ./VERSIONRepository: MatheMatrix/zstack
Length of output: 86
🏁 Script executed:
# Look for MySQL version specifications in documentation or build files
find . -name "pom.xml" -o -name "build.gradle" | head -5
rg "MySQL|mysql|5\.[0-9]" . --type xml | head -20Repository: MatheMatrix/zstack
Length of output: 1347
🏁 Script executed:
# Search for MySQL Connector version which indicates MySQL support
rg "mysql.*connector|com\.mysql" . -t xml | head -10Repository: MatheMatrix/zstack
Length of output: 643
🏁 Script executed:
# Get exact MySQL Connector version from main pom.xml
sed -n '/<groupId>com\.mysql<\/groupId>/,/<\/dependency>/p' ./pom.xml | head -20Repository: MatheMatrix/zstack
Length of output: 232
🏁 Script executed:
# Look for MySQL version documentation or requirements
find . -name "README*" -o -name "*.md" | xargs grep -l "MySQL\|mysql" 2>/dev/null | head -5Repository: MatheMatrix/zstack
Length of output: 117
🏁 Script executed:
# Check MySQL version requirements in documentation
cat ./README.md | grep -i "mysql\|database" -A 5 -B 5Repository: MatheMatrix/zstack
Length of output: 593
🏁 Script executed:
# Also check the Chinese README
cat ./README.zh-CN.md | grep -i "mysql\|数据库" -A 5 -B 5Repository: MatheMatrix/zstack
Length of output: 488
🏁 Script executed:
# Test the REGEXP_SUBSTR pattern logic with Python to verify the concern
python3 << 'EOF'
import re
test_cases = [
"dify_1.11.1.qcow2",
"1.11.1-rc1",
"myapp_2.5.3",
"1.0",
"system_1.2.3.4",
]
# Current pattern: '^[0-9]+\\.[0-9]+'
current_pattern = r'^[0-9]+\.[0-9]+'
# Suggested pattern: '[0-9]+(\.[0-9]+){1,2}'
suggested_pattern = r'[0-9]+(\.[0-9]+){1,2}'
print("Testing current REGEXP pattern: '^[0-9]+\\.[0-9]+'")
print("=" * 60)
for test in test_cases:
match = re.search(current_pattern, test)
print(f"Input: {test:20} | Match: {match.group() if match else 'NO MATCH'}")
print("\n\nTesting suggested REGEXP_SUBSTR pattern: '[0-9]+(\.[0-9]+){1,2}'")
print("=" * 60)
for test in test_cases:
match = re.search(suggested_pattern, test)
print(f"Input: {test:20} | Match: {match.group() if match else 'NO MATCH'}")
print("\n\nSimulating SUBSTRING_INDEX behavior:")
print("=" * 60)
def simulate_substring_index(s, delimiter, count):
"""Simulate MySQL SUBSTRING_INDEX"""
parts = s.split(delimiter)
if count > 0:
return delimiter.join(parts[:count])
else:
return delimiter.join(parts[count:])
test_paths = [
"file:///opt/dify_1.11.1.qcow2",
"file://opt/myapp_1.0.0/bin",
"file:/data/system_2.5.3-rc1",
]
for path in test_paths:
# Simulate the UPDATE logic
cleaned = path.replace("file://", "").replace("file:", "")
# Remove trailing slash
if cleaned.endswith("/"):
cleaned = cleaned[:-1]
# SUBSTRING_INDEX(..., '/', -1) - get last segment
last_segment = simulate_substring_index(cleaned, '/', -1)
# SUBSTRING_INDEX(..., '_', -1) - get part after last underscore
after_underscore = simulate_substring_index(last_segment, '_', -1)
print(f"\nPath: {path}")
print(f" After cleaning: {cleaned}")
print(f" Last segment (by /): {last_segment}")
print(f" After last _ (version): {after_underscore}")
# Check if it matches current REGEXP
matches_current = bool(re.match(current_pattern, after_underscore))
print(f" Matches '^[0-9]+\\.[0-9]+': {matches_current}")
if matches_current:
# Extract with suggested pattern
extracted = re.search(suggested_pattern, after_underscore)
print(f" REGEXP_SUBSTR result: {extracted.group() if extracted else 'NULL'}")
EOFRepository: MatheMatrix/zstack
Length of output: 1439
🏁 Script executed:
# Check migration history to understand MySQL version constraints
ls -la ./conf/db/upgrade/ | head -20Repository: MatheMatrix/zstack
Length of output: 1322
🏁 Script executed:
# Look for any documentation about MySQL version support
cat ./README.md | head -100Repository: MatheMatrix/zstack
Length of output: 5008
🏁 Script executed:
# Check if REGEXP_SUBSTR or similar functions are used elsewhere in SQL files
rg "REGEXP_SUBSTR|JSON_EXTRACT|WINDOW" ./conf/db --type sql | head -20Repository: MatheMatrix/zstack
Length of output: 44
版本提取逻辑缺陷：可能保留文件后缀和版本后缀（如 .qcow2、-rc1）
第 159 行的 WHERE 条件 REGEXP '^[0-9]+\\.[0-9]+' 仅校验字符串是否以版本号格式开头，而非完整匹配。对于形如 dify_1.11.1.qcow2 或 system_2.5.3-rc1 的字符串，SUBSTRING_INDEX 会匹配该条件，但后续 SET 子句会将整个 1.11.1.qcow2 或 2.5.3-rc1 写入 version 字段，而非纯版本号 1.11.1 或 2.5.3。
建议使用 REGEXP_SUBSTR 精确提取版本号部分：
建议改动
SET `version` = REGEXP_SUBSTR(
SUBSTRING_INDEX(
SUBSTRING_INDEX(
TRIM(TRAILING '/' FROM REPLACE(REPLACE(`installPath`, 'file://', ''), 'file:', '')),
'/', -1
), '_', -1
),
'[0-9]+(\\.[0-9]+){1,2}'
)
WHERE ... AND REGEXP_SUBSTR(
SUBSTRING_INDEX(
...
),
'[0-9]+(\\.[0-9]+){1,2}'
) IS NOT NULL;重要：REGEXP_SUBSTR 仅在 MySQL 8.0+ 和 GreatSQL 8.0+ 中支持。请确认目标数据库版本是否满足此要求。 可在升级前执行以下命令验证函数可用性：
SELECT REGEXP_SUBSTR('dify_1.11.1.qcow2', '[0-9]+(\\.[0-9]+){1,2}');🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.6__schema.sql` around lines 145 - 160, The current
UPDATE on zstack.ModelServiceVO sets `version` from the tail of `installPath`
but the REGEXP check only verifies the start, so values like "1.11.1.qcow2" or
"2.5.3-rc1" get written whole; change the assignment and predicate to extract
the pure semantic version using REGEXP_SUBSTR on the same SUBSTRING_INDEX(...)
expression (e.g., pattern '[0-9]+(\\.[0-9]+){1,2}') and only UPDATE rows where
REGEXP_SUBSTR(...) IS NOT NULL; ensure you reference the same expression for
both SET `version` and the WHERE filter and verify target DB supports
REGEXP_SUBSTR (MySQL 8.0+).
DBImpact Resolves: ZSTAC-81309 Change-Id: I6b7461726e6d666161666f7066686f62626c7268
MatheMatrix
force-pushed
the
sync/ye.zou/ZSTAC-81309-p2@@2
branch
from
375c0c8 to
5bc1ec7
Compare
2 participants
DBImpact
Resolves: ZSTAC-81309
Change-Id: I6b7461726e6d666161666f7066686f62626c7268
sync from gitlab !9123