fix: 修复了当 log 名不存在时，使用指令会出现假 log 的情况#1584
Draft
kenichiLyon wants to merge 7 commits intosealdice:masterfrom
Draft
fix: 修复了当 log 名不存在时，使用指令会出现假 log 的情况#1584kenichiLyon wants to merge 7 commits intosealdice:masterfrom
kenichiLyon wants to merge 7 commits intosealdice:masterfrom
Conversation
Reviewer's GuideEnsures log operations no longer create or read from fake logs with log_id=0 and adds a migration to clean up existing bad data. Sequence diagram for updated log append and read behavior with logID zero handlingsequenceDiagram
actor User
participant Service as LogService
participant DB as LogDB
%% LogAppend with proper logID assignment
User->>Service: LogAppend(groupID, logName, message)
Service->>DB: Query LogInfo by groupID, logName
alt existing log
DB-->>Service: logID
else new log
Service->>DB: Create LogInfo
DB-->>Service: newLog.ID as logID
end
Service->>Service: newLogItem.LogID = logID
Service->>DB: Create LogOneItem(newLogItem)
DB-->>Service: success
Service-->>User: append success
%% Read operations now guard against logID == 0
User->>Service: LogGetAllLines(groupID, missingLogName)
Service->>DB: Lookup logID by groupID, logName
DB-->>Service: logID = 0
alt logID is zero
Service-->>User: return empty LogOneItem list
else
Service->>DB: Query LogOneItem by logID
DB-->>Service: items
Service-->>User: items
end
%% Delete by MsgID also guards against logID == 0
User->>Service: LogMarkDeleteByMsgID(groupID, missingLogName, rawMsgID)
Service->>DB: Lookup logID by groupID, logName
DB-->>Service: logID = 0
alt logID is zero
Service-->>User: no op, return nil
else
Service->>DB: Delete LogOneItem where log_id = logID and raw_msg_id = rawMsgID
DB-->>Service: success
Service-->>User: delete success
end
Class diagram for V160 logID zero cleanup migration integrationclassDiagram
class Upgrade {
+string ID
+string Description
+func Apply(logf, operator) error
}
class V160LogIDZeroCleanMigrationHolder {
+Upgrade V160LogIDZeroCleanMigration
+func V160LogIDZeroCleanMigrate(dboperator, logf) error
}
class DatabaseOperator {
+func GetLogDB(mode) LogDB
}
class LogDB {
+func Model(value) LogDB
+func Where(query, args) LogDB
+func Pluck(column, dest) error
+func Delete(value) DeleteResult
}
class DeleteResult {
+int64 RowsAffected
+error Error
}
class LogOneItem {
+uint64 ID
+uint64 LogID
}
class LogInfo {
+uint64 ID
}
V160LogIDZeroCleanMigrationHolder "1" o-- "1" Upgrade : wraps
V160LogIDZeroCleanMigrationHolder ..> DatabaseOperator : uses
DatabaseOperator ..> LogDB : returns
LogDB ..> LogOneItem : queries
LogDB ..> LogInfo : queries
LogDB ..> DeleteResult : returns
Flow diagram for V160 migration cleaning log_id zero dataflowchart TD
A[Start V160LogIDZeroCleanMigration.Apply] --> B[Get LogDB with constant.WRITE]
B --> C[Query LogOneItem where log_id = 0 and pluck id into itemIDs]
C --> D[Delete from LogOneItem where log_id = 0]
D --> E{itemIDs length > 0?}
E -->|Yes| F[logf: 数据修复 - LogItems表，删除记录<br/>含 RowsAffected 和 itemIDs]
E -->|No| G[logf: 数据修复 - LogItems表，没有需要删除的记录]
F --> H[Query LogInfo where id = 0 and pluck id into logIDs]
G --> H
H --> I[Delete from LogInfo where id = 0]
I --> J{logIDs length > 0?}
J -->|Yes| K[logf: 数据修复 - Logs表，删除记录<br/>含 RowsAffected 和 logIDs]
J -->|No| L[logf: 数据修复 - Logs表，没有需要删除的记录]
K --> M[Return nil]
L --> M[Return nil]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
V160LogIDZeroCleanMigrate, collecting all matching IDs into slices just for logging can be expensive on large datasets; consider logging onlyRowsAffectedor limiting/plucking IDs conditionally (e.g., only when below a threshold). - The various
logID == 0early-return checks are now duplicated across several query functions; consider extracting the log-ID lookup plus zero-handling into a helper to keep the behavior consistent and reduce repetition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `V160LogIDZeroCleanMigrate`, collecting all matching IDs into slices just for logging can be expensive on large datasets; consider logging only `RowsAffected` or limiting/plucking IDs conditionally (e.g., only when below a threshold).
- The various `logID == 0` early-return checks are now duplicated across several query functions; consider extracting the log-ID lookup plus zero-handling into a helper to keep the behavior consistent and reduce repetition.
## Individual Comments
### Comment 1
<location> `migrate/v2/v160/v160_logid0.go:12-51` </location>
<code_context>
+func V160LogIDZeroCleanMigrate(dboperator operator.DatabaseOperator, logf func(string)) error {
+ db := dboperator.GetLogDB(constant.WRITE)
+
+ var itemIDs []uint64
+ if err := db.Model(&model.LogOneItem{}).
+ Where("log_id = 0").
+ Pluck("id", &itemIDs).Error; err != nil {
+ return err
+ }
+ itemResult := db.Where("log_id = 0").Delete(&model.LogOneItem{})
+ if itemResult.Error != nil {
+ return itemResult.Error
+ }
+
+ var logIDs []uint64
+ if err := db.Model(&model.LogInfo{}).
+ Where("id = 0").
+ Pluck("id", &logIDs).Error; err != nil {
+ return err
+ }
+ logResult := db.Where("id = 0").Delete(&model.LogInfo{})
+ if logResult.Error != nil {
+ return logResult.Error
+ }
+
+ if len(itemIDs) > 0 {
+ logf(fmt.Sprintf("数据修复 - LogItems表，删除了 %d 条记录，ID列表: %v", itemResult.RowsAffected, itemIDs))
+ } else {
+ logf("数据修复 - LogItems表，没有需要删除的记录")
+ }
+
+ if len(logIDs) > 0 {
+ logf(fmt.Sprintf("数据修复 - Logs表，删除了 %d 条记录，ID列表: %v", logResult.RowsAffected, logIDs))
+ } else {
</code_context>
<issue_to_address>
**suggestion (performance):** Consider limiting or summarizing logged ID lists to avoid excessive memory usage and log volume.
Loading all IDs into slices and logging the full lists (`itemIDs`, `logIDs`) can be expensive on large tables (thousands+ rows), both in memory and log volume. Instead, consider logging only `RowsAffected`, or at most a capped subset/summary of IDs (e.g., first N or aggregated ranges/counts) to keep the migration stable on large datasets.
```suggestion
// V160LogIDZeroCleanMigrate 清理 log_id = 0 的日志行与日志表残留
func V160LogIDZeroCleanMigrate(dboperator operator.DatabaseOperator, logf func(string)) error {
db := dboperator.GetLogDB(constant.WRITE)
itemResult := db.Where("log_id = 0").Delete(&model.LogOneItem{})
if itemResult.Error != nil {
return itemResult.Error
}
logResult := db.Where("id = 0").Delete(&model.LogInfo{})
if logResult.Error != nil {
return logResult.Error
}
if itemResult.RowsAffected > 0 {
logf(fmt.Sprintf("数据修复 - LogItems表，删除了 %d 条记录", itemResult.RowsAffected))
} else {
logf("数据修复 - LogItems表，没有需要删除的记录")
}
if logResult.RowsAffected > 0 {
logf(fmt.Sprintf("数据修复 - Logs表，删除了 %d 条记录", logResult.RowsAffected))
} else {
logf("数据修复 - Logs表，没有需要删除的记录")
}
return nil
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
kenichiLyon
changed the title
🚨 PR Title Needs FormattingThe title of this PR needs to be formatted correctly.
|
kenichiLyon
changed the title
Contributor
前者是正确的，解释了log_id=0的查询来源。 |
Contributor
Author
的确是我想得少了，已按照此路径进行修改 |
Contributor
There was a problem hiding this comment.
Pull request overview
修复在查询不存在的 log 名称时，会因 log_id=0 的脏数据导致返回“假 log”的问题，并提供一次性迁移用于清理历史遗留数据。
Changes:
- 调整
LogAppend写入流程：确保创建新 log 后再把真实logID写回到newLogItem.LogID，避免写入log_id=0。 getIDByGroupIDAndName在未找到记录时返回明确的ErrLogNotFound，并在多处读取路径中将“未找到”处理为返回空结果。- 新增 v160 迁移：清理
log_items.log_id=0与logs.id=0的历史残留数据，并在升级入口注册。
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| migrate/v2/v160/v160_logid0.go | 新增清理 log_id=0 残留数据的 v160 升级迁移 |
| migrate/v2/enter.go | 注册 v160 迁移到升级管理器 |
| dice/service/log.go | 修复 log_id=0 写入源头，并用显式“未找到”错误避免读回假数据 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
暂转为draft。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #1582
目前找到问题所在为：
本 pr 的解决方案对应为：
已自行测试，图中test-bot为此 commit 后编译的海豹，流明则是此 commit 前