Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Foreign ID conflicts if ID is 0 for each item #21271

Merged
merged 7 commits into from
Oct 2, 2022

Conversation

techknowlogick
Copy link
Member

The default is 0 if not defined, and that causes dupe index errors

@techknowlogick techknowlogick added the backport/done All backports for this PR have been created label Sep 27, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Sep 27, 2022
@lafriks
Copy link
Member

lafriks commented Sep 28, 2022

Doesn't xorm supports null values? Somehow all these 0 ID's seems quite hacky 😕

@6543
Copy link
Member

6543 commented Sep 28, 2022

I guess it's more about how gitea convert int64 to string etc ... ?

@lafriks
Copy link
Member

lafriks commented Sep 28, 2022

No, I don't think strings should have effect in this. Normally in standard sql drivers you would user *int64 or sql.NullInt64. I don't know how xorm does handle these

@techknowlogick
Copy link
Member Author

techknowlogick commented Sep 28, 2022

So specifically, foreign id will be 0 for multiple issues causing the DB to error out due to the unique index requirements. This just defaults to having the foreign ID be the same as the index should no foreign ID exist.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 28, 2022
@techknowlogick techknowlogick merged commit 9e2f374 into go-gitea:main Oct 2, 2022
@techknowlogick techknowlogick deleted the fix-import branch October 2, 2022 21:43
techknowlogick added a commit that referenced this pull request Oct 2, 2022
The default is 0 if not defined, and that causes dupe index errors

Backport of #21271
tyroneyeh added a commit to tyroneyeh/gitea that referenced this pull request Oct 24, 2022
…tea#21272)

The default is 0 if not defined, and that causes dupe index errors

Backport of go-gitea#21271
lunny pushed a commit that referenced this pull request Feb 7, 2023
Fix #22581

TLDR: #18446 made a mess with ForeignIndex and triggered a design
flaw/bug of #16356, then a quick patch #21271 helped #18446, then the
the bug was re-triggered by #21721 .

Related:
* #16356
* BasicIssueContext
https://github.com/go-gitea/gitea/pull/16356/files#diff-7938eb670d42a5ead6b08121e16aa4537a4d716c1cf37923c70470020fb9d036R16-R27
* #18446 
* If some issues were dumped without ForeignIndex, then they would be
imported as ForeignIndex=0
https://github.com/go-gitea/gitea/pull/18446/files#diff-1624a3e715d8fc70edf2db1630642b7d6517f8c359cc69d58c3958b34ba4ce5eR38-R39
* #21271
* It patched the above bug (somewhat), made the issues without
ForeignIndex could have the same value as LocalIndex
* #21721 
    * It re-triggered the zero-ForeignIndex bug.


ps: I am not sure whether the changes in `GetForeignIndex` are ideal (at
least, now it has almost the same behavior as BasicIssueContext in
#16356), it's just a quick fix. Feel free to edit on this PR directly or
replace it.

Co-authored-by: zeripath <art27@cantab.net>
yardenshoham pushed a commit to yardenshoham/gitea that referenced this pull request Feb 7, 2023
…2776)

Fix go-gitea#22581

TLDR: go-gitea#18446 made a mess with ForeignIndex and triggered a design
flaw/bug of go-gitea#16356, then a quick patch go-gitea#21271 helped go-gitea#18446, then the
the bug was re-triggered by go-gitea#21721 .

Related:
* go-gitea#16356
* BasicIssueContext
https://github.com/go-gitea/gitea/pull/16356/files#diff-7938eb670d42a5ead6b08121e16aa4537a4d716c1cf37923c70470020fb9d036R16-R27
* go-gitea#18446 
* If some issues were dumped without ForeignIndex, then they would be
imported as ForeignIndex=0
https://github.com/go-gitea/gitea/pull/18446/files#diff-1624a3e715d8fc70edf2db1630642b7d6517f8c359cc69d58c3958b34ba4ce5eR38-R39
* go-gitea#21271
* It patched the above bug (somewhat), made the issues without
ForeignIndex could have the same value as LocalIndex
* go-gitea#21721 
    * It re-triggered the zero-ForeignIndex bug.


ps: I am not sure whether the changes in `GetForeignIndex` are ideal (at
least, now it has almost the same behavior as BasicIssueContext in
go-gitea#16356), it's just a quick fix. Feel free to edit on this PR directly or
replace it.

Co-authored-by: zeripath <art27@cantab.net>
zeripath pushed a commit that referenced this pull request Feb 8, 2023
…22794)

Backport #22776

Fix #22581

TLDR: #18446 made a mess with ForeignIndex and triggered a design
flaw/bug of #16356, then a quick patch #21271 helped #18446, then the
the bug was re-triggered by #21721 .

Related:
* #16356
* BasicIssueContext
https://github.com/go-gitea/gitea/pull/16356/files#diff-7938eb670d42a5ead6b08121e16aa4537a4d716c1cf37923c70470020fb9d036R16-R27
* #18446 
* If some issues were dumped without ForeignIndex, then they would be
imported as ForeignIndex=0
https://github.com/go-gitea/gitea/pull/18446/files#diff-1624a3e715d8fc70edf2db1630642b7d6517f8c359cc69d58c3958b34ba4ce5eR38-R39
* #21271
* It patched the above bug (somewhat), made the issues without
ForeignIndex could have the same value as LocalIndex
* #21721 
    * It re-triggered the zero-ForeignIndex bug.


ps: I am not sure whether the changes in `GetForeignIndex` are ideal (at
least, now it has almost the same behavior as BasicIssueContext in
#16356), it's just a quick fix. Feel free to edit on this PR directly or
replace it.

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants