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

Make TryInsert functions within the packages module use INSERT ... ON CONFLICT #21063

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open
Changes from 7 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
7f4e851
Make TryInsert functions within the packages module try to insert first
zeripath Sep 4, 2022
b29ab42
partial still broken
zeripath Sep 6, 2022
470064c
more
zeripath Sep 6, 2022
9c83ab8
Merge remote-tracking branch 'origin/main' into fix-19586-insert-firs…
zeripath Sep 7, 2022
bf94b55
Merge remote-tracking branch 'origin/main' into fix-19586-insert-firs…
zeripath Sep 7, 2022
8d3864a
ensure that timestamps are also set
zeripath Sep 7, 2022
71522ea
oops lets try to get the mysql syntax right
zeripath Sep 7, 2022
7843fe9
Merge remote-tracking branch 'origin/main' into fix-19586-insert-firs…
zeripath Feb 2, 2023
abcf334
attempt to fix mysql/mssql
zeripath Feb 2, 2023
430f964
Merge remote-tracking branch 'origin/main' into fix-19586-insert-firs…
zeripath Feb 6, 2023
5dff21a
fix insert on conflict for sqlite
zeripath Feb 7, 2023
f934a98
fix unit-test
zeripath Feb 7, 2023
b3db6da
attempt to fix postgres
zeripath Feb 7, 2023
5bc4924
placate lint
zeripath Feb 7, 2023
8f7987c
fix mssql?
zeripath Feb 7, 2023
fc5d9aa
hopefully fix psql
zeripath Feb 7, 2023
7ec881f
get more info mssql
zeripath Feb 7, 2023
d8aa794
perhaps inserted should be INSERTED?
zeripath Feb 7, 2023
3f39045
fix mssql bug
zeripath Feb 7, 2023
15855df
add comments and slight restructure
zeripath Feb 7, 2023
38d540b
slight adjustments
zeripath Feb 7, 2023
a941cba
placate the linter
zeripath Feb 7, 2023
5ef7902
as per wxiaoguang
zeripath Feb 7, 2023
04efbf9
more comments
zeripath Feb 7, 2023
2283b23
missed setting sql
zeripath Feb 7, 2023
a282e66
add testcases
zeripath Feb 7, 2023
62b1e20
fix fmt
zeripath Feb 7, 2023
f1222e8
slight bug in mysql
zeripath Feb 7, 2023
25abc72
as per wxiaoguang
zeripath Feb 8, 2023
028b5a6
split functions out and just use ignore
zeripath Feb 8, 2023
1c17006
Merge remote-tracking branch 'origin/main' into fix-19586-insert-firs…
zeripath Feb 8, 2023
ac6862a
remove unnecessary mutex
zeripath Feb 9, 2023
dc4638c
Merge remote-tracking branch 'origin/main' into fix-19586-insert-firs…
zeripath Feb 9, 2023
ecd3eea
Merge remote-tracking branch 'origin/main' into fix-19586-insert-firs…
zeripath Mar 12, 2023
bf18cf8
add a few comments to test;
zeripath Mar 12, 2023
1a6f5df
fix broken merge
zeripath Mar 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 176 additions & 0 deletions models/db/common.go
Original file line number Diff line number Diff line change
@@ -5,12 +5,18 @@
package db

import (
"context"
"fmt"
"reflect"
"strings"
"time"

"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
"xorm.io/xorm/dialects"
"xorm.io/xorm/schemas"
)

// BuildCaseInsensitiveLike returns a condition to check if the given value is like the given key case-insensitively.
@@ -21,3 +27,173 @@ func BuildCaseInsensitiveLike(key, value string) builder.Cond {
}
return builder.Like{"UPPER(" + key + ")", strings.ToUpper(value)}
}

func InsertOnConflictDoNothing(ctx context.Context, bean interface{}) (int64, error) {
e := GetEngine(ctx)

tableName := x.TableName(bean, true)
table, err := x.TableInfo(bean)
if err != nil {
return 0, err
}

autoIncrCol := table.AutoIncrColumn()

cols := table.Columns()
colNames := make([]string, 0, len(cols))
args := make([]interface{}, 1, len(cols))

val := reflect.ValueOf(bean)
elem := val.Elem()
for _, col := range cols {
if fieldIdx := col.FieldIndex; fieldIdx != nil {
fieldVal := elem.FieldByIndex(fieldIdx)
if col.IsCreated || col.IsUpdated {
t := time.Now()
result, err := dialects.FormatColumnTime(x.Dialect(), x.DatabaseTZ, col, t)
if err != nil {
return 0, err
}

switch fieldVal.Type().Kind() {
case reflect.Struct:
fieldVal.Set(reflect.ValueOf(t).Convert(fieldVal.Type()))
case reflect.Int, reflect.Int64, reflect.Int32:
fieldVal.SetInt(t.Unix())
case reflect.Uint, reflect.Uint64, reflect.Uint32:
fieldVal.SetUint(uint64(t.Unix()))
}

colNames = append(colNames, col.Name)
args = append(args, result)
continue
}

if fieldVal.IsZero() {
continue
}
colNames = append(colNames, col.Name)
args = append(args, fieldVal.Interface())
}
}

if len(colNames) == 0 {
return 0, fmt.Errorf("empty bean")
}

uniqueCols := make([]string, 0, len(cols))
uniqueArgs := make([]interface{}, 1, len(cols))
for _, index := range table.Indexes {
if index.Type != schemas.UniqueType {
continue
}
indexCol:
for _, iCol := range index.Cols {
for _, uCol := range uniqueCols {
if uCol == iCol {
continue indexCol
}
}
for i, col := range colNames {
if col == iCol {
uniqueCols = append(uniqueCols, col)
uniqueArgs = append(uniqueArgs, args[i+1])
continue indexCol
}
}
}
}

if len(uniqueCols) == 0 {
return 0, fmt.Errorf("empty bean")
}

sb := &strings.Builder{}
switch {
case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL || setting.Database.UseMySQL:
_, _ = sb.WriteString("INSERT ")
if setting.Database.UseMySQL && autoIncrCol == nil {
_, _ = sb.WriteString("IGNORE ")
}
_, _ = sb.WriteString("INTO ")
_, _ = sb.WriteString(x.Dialect().Quoter().Quote(tableName))
_, _ = sb.WriteString(" (")
_, _ = sb.WriteString(colNames[0])
zeripath marked this conversation as resolved.
Show resolved Hide resolved
for _, colName := range colNames[1:] {
_, _ = sb.WriteString(",")
_, _ = sb.WriteString(colName)
zeripath marked this conversation as resolved.
Show resolved Hide resolved
}
_, _ = sb.WriteString(") VALUES (")
_, _ = sb.WriteString("?")
for range colNames[1:] {
_, _ = sb.WriteString(",?")
}
switch {
case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL:
_, _ = sb.WriteString(") ON CONFLICT DO NOTHING")
case setting.Database.UseMySQL:
if autoIncrCol != nil {
_, _ = sb.WriteString(") ON CONFLICT DO DUPLICATE KEY ")
_, _ = sb.WriteString(autoIncrCol.Name)
_, _ = sb.WriteString(" = ")
_, _ = sb.WriteString(autoIncrCol.Name)
}
}
case setting.Database.UseMSSQL:
_, _ = sb.WriteString("MERGE ")
_, _ = sb.WriteString(x.Dialect().Quoter().Quote(tableName))
_, _ = sb.WriteString(" WITH (HOLDLOCK) AS target USING (SELECT ")

_, _ = sb.WriteString("? AS ")
_, _ = sb.WriteString(uniqueCols[0])
for _, uniqueCol := range uniqueCols[1:] {
_, _ = sb.WriteString(", ? AS ")
_, _ = sb.WriteString(uniqueCol)
}
_, _ = sb.WriteString(") AS src ON src.")
_, _ = sb.WriteString(uniqueCols[0])
_, _ = sb.WriteString("= target.")
_, _ = sb.WriteString(uniqueCols[0])
for _, uniqueCol := range uniqueCols[1:] {
_, _ = sb.WriteString(" AND src.")
_, _ = sb.WriteString(uniqueCol)
_, _ = sb.WriteString("= target.")
_, _ = sb.WriteString(uniqueCols[0])
}
_, _ = sb.WriteString(" WHEN NOT MATCHED THEN INSERT (")
_, _ = sb.WriteString(colNames[0])
for _, colName := range colNames[1:] {
_, _ = sb.WriteString(",")
_, _ = sb.WriteString(colName)
}
_, _ = sb.WriteString(") VALUES (")
_, _ = sb.WriteString("?")
for range colNames[1:] {
_, _ = sb.WriteString(",?")
}
_, _ = sb.WriteString(")")
args = append(uniqueArgs, args[1:]...)
default:
return 0, fmt.Errorf("database type not supported")
}
args[0] = sb.String()
res, err := e.Exec(args...)
if err != nil {
return 0, err
}

n, err := res.RowsAffected()
if err != nil {
return n, err
}

if n != 0 && autoIncrCol != nil {
id, err := res.LastInsertId()
if err != nil {
return n, err
}
elem.FieldByName(autoIncrCol.FieldName).SetInt(id)
}

return res.RowsAffected()
}
25 changes: 15 additions & 10 deletions models/packages/package.go
Original file line number Diff line number Diff line change
@@ -120,25 +120,30 @@ type Package struct {

// TryInsertPackage inserts a package. If a package exists already, ErrDuplicatePackage is returned
func TryInsertPackage(ctx context.Context, p *Package) (*Package, error) {
e := db.GetEngine(ctx)
n, err := db.InsertOnConflictDoNothing(ctx, p)
if err != nil {
return nil, err
}
if n != 0 {
return p, nil
Copy link
Member

Choose a reason for hiding this comment

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

The insert methods need to return the filled bean. For example it's expected that ID has a value in the return value which is not the case here.

Copy link
Contributor Author

@zeripath zeripath Feb 7, 2023

Choose a reason for hiding this comment

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

The code auto fills the ID of the bean in the same way that xorm's Insert() does.

}

key := &Package{
OwnerID: p.OwnerID,
Type: p.Type,
LowerName: p.LowerName,
}

has, err := e.Get(key)
if err != nil {
return nil, err
}
has, err := db.GetEngine(ctx).Get(key)
if has {
return key, ErrDuplicatePackage
if n == 0 {
err = ErrDuplicatePackage
}
return key, err
} else if err == nil {
return TryInsertPackage(ctx, p)
}
if _, err = e.Insert(p); err != nil {
return nil, err
}
return p, nil
return nil, err
}

// DeletePackageByID deletes a package by id
27 changes: 16 additions & 11 deletions models/packages/package_file.go
Original file line number Diff line number Diff line change
@@ -45,25 +45,30 @@ type PackageFile struct {

// TryInsertFile inserts a file. If the file exists already ErrDuplicatePackageFile is returned
func TryInsertFile(ctx context.Context, pf *PackageFile) (*PackageFile, error) {
e := db.GetEngine(ctx)
n, err := db.InsertOnConflictDoNothing(ctx, pf)
if err != nil {
return nil, err
}
if n != 0 {
return pf, nil
}

key := &PackageFile{
VersionID: pf.VersionID,
LowerName: pf.LowerName,
CompositeKey: pf.CompositeKey,
}

has, err := e.Get(key)
if err != nil {
return nil, err
}
has, err := db.GetEngine(ctx).Get(key)
if has {
return pf, ErrDuplicatePackageFile
}
if _, err = e.Insert(pf); err != nil {
return nil, err
if n == 0 {
err = ErrDuplicatePackageFile
}
return key, err
} else if err == nil {
return TryInsertFile(ctx, pf)
}
return pf, nil

return key, err
}

// GetFilesByVersionID gets all files of a version
25 changes: 15 additions & 10 deletions models/packages/package_version.go
Original file line number Diff line number Diff line change
@@ -39,24 +39,29 @@ type PackageVersion struct {

// GetOrInsertVersion inserts a version. If the same version exist already ErrDuplicatePackageVersion is returned
func GetOrInsertVersion(ctx context.Context, pv *PackageVersion) (*PackageVersion, error) {
e := db.GetEngine(ctx)
n, err := db.InsertOnConflictDoNothing(ctx, pv)
if err != nil {
return nil, err
}
if n != 0 {
return pv, nil
}

key := &PackageVersion{
PackageID: pv.PackageID,
LowerVersion: pv.LowerVersion,
}

has, err := e.Get(key)
if err != nil {
return nil, err
}
has, err := db.GetEngine(ctx).Get(key)
if has {
return key, ErrDuplicatePackageVersion
}
if _, err = e.Insert(pv); err != nil {
return nil, err
if n == 0 {
err = ErrDuplicatePackageVersion
}
return key, err
} else if err == nil {
return GetOrInsertVersion(ctx, pv)
}
return pv, nil
return nil, err
}

// UpdateVersion updates a version