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

Split big preloading-IN query into multiple queries (#283) #285

Merged
merged 5 commits into from May 27, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cursor.go
Expand Up @@ -81,6 +81,7 @@ func scanMulti(cur Cursor, keyField string, keyType reflect.Type, cols map[inter
}

if !found && fields != nil {
// TODO: why a panic and not just an easily catchable error?
aligator marked this conversation as resolved.
Show resolved Hide resolved
panic("rel: primary key row does not exists")
}

Expand Down
48 changes: 35 additions & 13 deletions repository.go
Expand Up @@ -1030,25 +1030,47 @@ func (r repository) preload(cw contextWrapper, records slice, field string, quer
path = strings.Split(field, ".")
targets, table, keyField, keyType, ddata, loaded = r.mapPreloadTargets(records, path)
ids = r.targetIDs(targets)
query = Build(table, append(queriers, In(keyField, ids...))...)

aligator marked this conversation as resolved.
Show resolved Hide resolved
inClauseLength = 999 // TODO: can this value come from the adapter, as it is dbms specific?
aligator marked this conversation as resolved.
Show resolved Hide resolved
)

if len(targets) == 0 || loaded && !bool(query.ReloadQuery) {
return nil
}
// Create separate queries if the amount of ids is more than inClauseLength.
for {
if len(ids) == 0 {
break
}

var (
cur, err = cw.adapter.Query(cw.ctx, r.withDefaultScope(ddata, query, false))
)
// necessary check to avoid slicing beyond
// slice capacity
if len(ids) < inClauseLength {
inClauseLength = len(ids)
}

if err != nil {
return err
}
idsChunk := ids[0:inClauseLength]
ids = ids[inClauseLength:]

query := Build(table, append(queriers, In(keyField, idsChunk...))...)
if len(targets) == 0 || loaded && !bool(query.ReloadQuery) {
return nil
}

var (
cur, err = cw.adapter.Query(cw.ctx, r.withDefaultScope(ddata, query, false))
)

scanFinish := r.instrumenter.Observe(cw.ctx, "rel-scan-multi", "scanning all records to multiple targets")
defer scanFinish(nil)
if err != nil {
return err
}

return scanMulti(cur, keyField, keyType, targets)
scanFinish := r.instrumenter.Observe(cw.ctx, "rel-scan-multi", "scanning all records to multiple targets")
err = scanMulti(cur, keyField, keyType, targets)
Copy link
Member

Choose a reason for hiding this comment

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

can you confirm calling this multiple time won't reset the slice of association field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's something I am also not sure. In my first test with my example project it works just well, but I will look into it.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this can be added as assertion in the unit test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, as long as the keys do not re-occur in the subsequent calls it works just fine, as each scanMulti call only modifies the occurred keys.
This works as long as each call uses unique keys which is here the case as r.targetIDs returns unique keys.

So it should be fine here. Tell me if you think otherwise.

I added a test to scanMulti for this.

scanFinish(nil)
aligator marked this conversation as resolved.
Show resolved Hide resolved
aligator marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
}

return nil
}

func (r repository) MustPreload(ctx context.Context, records interface{}, field string, queriers ...Querier) {
Expand Down