Skip to content

Conversation

@wxiaoguang
Copy link
Contributor

These 2 code blocks are exactly the same, but the one in TestEmptyRepoAddFile fails to update database, the xorm.exec function only generates about 8-9 columns in the SQL (no is_empty column)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 14, 2025
@wxiaoguang
Copy link
Contributor Author

@lunny

@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Jan 14, 2025
@wxiaoguang wxiaoguang marked this pull request as draft January 14, 2025 07:58
@wxiaoguang wxiaoguang changed the title Strange XORM or DB bug demo: strange XORM or DB bug Jan 14, 2025
@lunny
Copy link
Member

lunny commented Jan 14, 2025

If there are no Cols("is_empty", "default_branch"), XORM will generate update columns automatically. Since there is no real zero value for bool type, is_empty will not be generated. Only non-empty strings, non-zero integers and non-nil pointers will be considered as columns needs to be updated.

user30EmptyRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 30, Name: "empty"})
user30EmptyRepo.IsEmpty = true
user30EmptyRepo.DefaultBranch = "no-such"
_, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Update(user30EmptyRepo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Update(user30EmptyRepo)
_, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Cols("is_empty", "default_branch").Update(user30EmptyRepo)

user30EmptyRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 30, Name: "empty"})
user30EmptyRepo.IsEmpty = true
user30EmptyRepo.DefaultBranch = "no-such"
_, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Update(user30EmptyRepo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Update(user30EmptyRepo)
_, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Cols("is_empty", "default_branch").Update(user30EmptyRepo)

@wxiaoguang
Copy link
Contributor Author

If there are no Cols("is_empty", "default_branch"), XORM will generate update columns automatically. Since there is no real zero value for bool type, is_empty will not be generated. Only non-empty strings, non-zero integers and non-nil pointers will be considered as columns needs to be updated.

But true is non-zero value.

@wxiaoguang wxiaoguang closed this Jan 15, 2025
@lunny
Copy link
Member

lunny commented Jan 15, 2025

If there are no Cols("is_empty", "default_branch"), XORM will generate update columns automatically. Since there is no real zero value for bool type, is_empty will not be generated. Only non-empty strings, non-zero integers and non-nil pointers will be considered as columns needs to be updated.

But true is non-zero value.

It's something right. It's also weird if true can be updated but false cannot be. So the decision at that time is boolean will not be generated automatically at any time.

@wxiaoguang wxiaoguang deleted the test-db-bug branch January 21, 2025 11:26
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Apr 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants