Skip to content

orm: forward-port join-order fix from v9.2.1 to v10 - #2048

Open
OscarYuen wants to merge 1 commit into
go-pg:v10from
OscarYuen:fix/forward-port-join-order-to-v10
Open

orm: forward-port join-order fix from v9.2.1 to v10#2048
OscarYuen wants to merge 1 commit into
go-pg:v10from
OscarYuen:fix/forward-port-join-order-to-v10

Conversation

@OscarYuen

Copy link
Copy Markdown

Summary

83b941300e97 ("Change join order") landed on the v9 branch and shipped in v9.2.1 (2021-04-22). It was never forward-ported to v10, so every v10 release — up to and including v10.15.1 — still has the pre-fix ordering.

This applies the same change to v10.

What the ordering does

In manyQuery and m2mQuery, the user-supplied ApplyQuery hook runs immediately after q.Model(...):

q = q.Model(manyModel)
if j.ApplyQuery != nil {
    q, err = j.ApplyQuery(q)   // <- runs here
}
if len(q.columns) == 0 {
    q.columns = append(q.columns, &hasManyColumnsAppender{j})
}

baseTable := j.BaseModel.Table()
// ... "FK IN (parent ids)" appended here, plus the polymorphic condition

So a callback passed as:

db.Model(&users).Relation("Posts", func(q *orm.Query) (*orm.Query, error) {
    return q.Where("..."), nil
}).Select()

sees a query that has not yet been scoped to the parent rows, and the
len(q.columns) == 0 check that decides whether to add the default
has-many columns is evaluated at the same early point.

After this change both run at the end of the function, once the relation
query is fully constructed — matching v9.2.1.

Diff

Pure code motion in orm/join.go: the ApplyQuery block and the
adjacent default-column block move from just after q.Model(...) to just
before return q, nil, in both manyQuery and m2mQuery. No behaviour
is added or removed beyond the ordering. 24 insertions, 22 deletions, one
file.

Verification

  • go build ./... clean
  • go test ./orm/... ./types/... ./internal/... — all pass
  • gofmt clean
  • Resulting manyQuery / m2mQuery ordering is identical to v9@v9.2.1

I don't have a Postgres instance wired up here, so the DB-backed
integration tests were not exercised — worth a CI run.

Forward-port of 83b9413 ("Change join order"), which landed on v9
for v9.2.1 and was never carried over to v10.

In manyQuery and m2mQuery the user-supplied ApplyQuery hook — the
callback form of Relation("Name", func(q *orm.Query) (*orm.Query, error))
— runs immediately after q.Model(), before the join appends the
"FK IN (parent ids)" predicate and before the polymorphic condition.
The default-column decision (len(q.columns) == 0) is likewise made
before those are in place.

Moving both to the end of the function makes the hook observe the fully
constructed relation query, which is what v9.2.1 does.

orm, types, internal and internal/pool tests pass; gofmt clean.
@OscarYuen
OscarYuen force-pushed the fix/forward-port-join-order-to-v10 branch from 238ee79 to 9f8eabb Compare August 21, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant