WIP: association-preloading #2241

sogari · 2023-04-01T16:35:54Z

sogari commented

2023-04-01 16:35:54 +00:00

First-time contributor

This PR is an attempt to implement the association preload feature. There are some restrictions at the moment (but maybe we can address them in a later version):

Only pointers to structs are allowed in the association fields.
Only single-column primary keys are allowed in related models.
Foreign key fields must be declared in the model, except for many_to_many associations. This means that an FK for belongs_to must be in the owning (aka, parent or referencing) model, while an FK for has_one or has_many must be in the referenced model.
The order of elements in the resulting association arrays is unspecified and non-deterministic (may change from run to run).
Required columns will be included as needed in the preload query, which means the PK or FK fields will be populated with database values.
The join table for many_to_many associations cannot be declared as a model in the code. Its struct type and table schema are generated at runtime with two fields.

One additional note: intermediate associations that have no selected columns and were not filled with any nested association are considered "empty" (except for the PK and FK fields) and are removed from the result by default. To get around this behavior, one can add the PK column to the list of selected columns of the intermediate preload (since this column will be fetched anyway).

Solves #2240

This PR is an attempt to implement the association preload feature. There are some restrictions at the moment (but maybe we can address them in a later version): - Only pointers to structs are allowed in the association fields. - Only single-column primary keys are allowed in related models. - Foreign key fields must be declared in the model, except for `many_to_many` associations. This means that an FK for `belongs_to` must be in the owning (aka, parent or referencing) model, while an FK for `has_one` or `has_many` must be in the referenced model. - The order of elements in the resulting association arrays is unspecified and non-deterministic (may change from run to run). - Required columns will be included as needed in the preload query, which means the PK or FK fields will be populated with database values. - The join table for `many_to_many` associations cannot be declared as a model in the code. Its struct type and table schema are generated at runtime with two fields. One additional note: intermediate associations that have no selected columns and were not filled with any nested association are considered "empty" (except for the PK and FK fields) and are removed from the result by default. To get around this behavior, one can add the PK column to the list of selected columns of the intermediate preload (since this column will be fetched anyway). Solves #2240

sogari reviewed 2023-04-01 16:38:33 +00:00

tags/parser.go Outdated

						
				@ -85,3 +85,3 @@

				// ParseWithCache parse a struct with cache

				func (parser *Parser) ParseWithCache(v reflect.Value) (*schemas.Table, error) {

					t := v.Type()

					t := reflect.Indirect(v).Type()

sogari commented

2023-04-01 16:38:33 +00:00

First-time contributor

Without this, the same struct type was being parsed twice.

lunny added the

kind

feature

label 2023-04-02 08:24:09 +00:00

lunny reviewed 2023-04-02 08:43:45 +00:00

session_find.go Outdated

						
				@ -144,0 +152,4 @@

							return err

						}

						// we need the columns required for the preloads

						if !session.statement.ColumnMap.IsEmpty() {

lunny commented

2023-04-02 08:43:45 +00:00

If it's empty, could we generate all the columns from the struct?

sogari commented

2023-04-06 18:37:38 +00:00

First-time contributor

Hi @lunny, I’m sorry for the late response. I was involved in a motorcycle accident recently, and have been off for a few days.

About the ColumnMap being empty, if I’m not mistaken, xorm’s behavior in this case is to select all columns from the struct. In that case, the required columns for preloading will already be included and we don’t need to worry about it. Does this make sense?

Hi @lunny, I’m sorry for the late response. I was involved in a motorcycle accident recently, and have been off for a few days. About the `ColumnMap` being empty, if I’m not mistaken, xorm’s behavior in this case is to select all columns from the struct. In that case, the required columns for preloading will already be included and we don’t need to worry about it. Does this make sense?

lunny commented

2023-04-06 13:49:16 +00:00

Could we ignore Cols of the engine.Proload.Cols which means all the columns of the table?

Could we ignore `Cols` of the `engine.Proload.Cols` which means all the columns of the table?

sogari force-pushed association-preloading from 71389d298f to c763b72c65

2023-04-06 18:41:17 +00:00

Compare

sogari commented

2023-04-06 21:54:59 +00:00

First-time contributor

Could we ignore Cols of the engine.Proload.Cols which means all the columns of the table?

I'm not sure if I understood your question correctly. By "ignore", do you mean the same functionality as the engine.Omit function?

> Could we ignore `Cols` of the `engine.Proload.Cols` which means all the columns of the table? I'm not sure if I understood your question correctly. By "ignore", do you mean the same functionality as the `engine.Omit` function?

lunny commented

2023-04-06 23:54:05 +00:00

No, I mean whether we can use like below.

var employees []*Employee
err = engine.Preloads(
    // 1. preload the names of all subordinates of this employee's manager
    engine.Preload("Manager.Subordinates"),
    // 2. preload the name of the buddy of each employee indicated by this employee
    engine.Preload("Indications.Buddy"),
    // 3. preload the names of all subordinates who indicated this employee's apprentice
    engine.Preload("Apprentice.IndicatedBy").Where("manager_id is not null"),
    // 4. preload the names of:
    // 	    all employees who don't have a maanger and were indicated by:
    engine.Preload("Subordinates.IndicatedBy.Indications").Where("manager_id is null"),
    // 	    employees whose name is 4 letters long and indicated:
    engine.Preload("Subordinates.IndicatedBy").Where("name like '____'"),
    // 	    this employee's subordinates who don't have a buddy
    engine.Preload("Subordinates").Where("buddy_id is null"),
    // 0. find the names of all employees
    ).Find(&employees)

No, I mean whether we can use like below. ```go var employees []*Employee err = engine.Preloads( // 1. preload the names of all subordinates of this employee's manager engine.Preload("Manager.Subordinates"), // 2. preload the name of the buddy of each employee indicated by this employee engine.Preload("Indications.Buddy"), // 3. preload the names of all subordinates who indicated this employee's apprentice engine.Preload("Apprentice.IndicatedBy").Where("manager_id is not null"), // 4. preload the names of: // all employees who don't have a maanger and were indicated by: engine.Preload("Subordinates.IndicatedBy.Indications").Where("manager_id is null"), // employees whose name is 4 letters long and indicated: engine.Preload("Subordinates.IndicatedBy").Where("name like '____'"), // this employee's subordinates who don't have a buddy engine.Preload("Subordinates").Where("buddy_id is null"), // 0. find the names of all employees ).Find(&employees) ```

sogari commented

2023-04-09 17:53:00 +00:00

First-time contributor

Thanks for the clarification. I see what you mean now.

In the way I implemented it, a preload must have a Cols clause to select at least one column. Otherwise, the default behavior is to omit all columns, in order to avoid polluting intermediate preloads.

I’ll see if I can change it to allow not specifying any column, and internally selecting all columns for a leaf preload.

Thanks for the clarification. I see what you mean now. In the way I implemented it, a preload must have a `Cols` clause to select at least one column. Otherwise, the default behavior is to omit all columns, in order to avoid polluting intermediate preloads. I’ll see if I can change it to allow not specifying any column, and internally selecting all columns for a leaf preload.

👍 1

sogari force-pushed association-preloading from c763b72c65 to 89a266addf

2023-04-09 18:28:59 +00:00

Compare

lunny commented

2023-07-04 05:13:19 +00:00

Please resolve the conflicts.

lunny added 1 commit 2023-07-26 01:20:16 +00:00

Merge branch 'association-preloading' of gitea.com:sogari/xorm into sogari-association-preloading

test mariadb / test mariadb (pull_request) Successful in 4m34s

Details

test cockroach / test cockroach (pull_request) Successful in 7m6s

Details

test mysql / test mysql (pull_request) Successful in 4m58s

Details

test mssql / test mssql (pull_request) Successful in 5m45s

Details

test mysql8 / test mysql8 (pull_request) Successful in 5m4s

Details

test postgres / test postgres (pull_request) Successful in 5m41s

Details

test tidb / test tidb (pull_request) Successful in 5m28s

Details

test sqlite / unit test & test sqlite (pull_request) Successful in 8m20s

Details

7f472fc317

lunny added this to the 2.0.0 milestone 2023-07-26 01:22:05 +00:00

lunny referenced this pull request

2023-07-26 01:31:11 +00:00

Implement one associate belongs to #2006

lunny added 1 commit 2023-07-27 06:34:58 +00:00

Merge branch 'master' into sogari-association-preloading

test cockroach / test cockroach (pull_request) Has been cancelled

Details

test mariadb / test mariadb (pull_request) Has been cancelled

Details

test mssql / test mssql (pull_request) Has been cancelled

Details

test mysql / test mysql (pull_request) Has been cancelled

Details

test mysql8 / test mysql8 (pull_request) Has been cancelled

Details

test postgres / test postgres (pull_request) Has been cancelled

Details

test sqlite / unit test & test sqlite (pull_request) Has been cancelled

Details

test tidb / test tidb (pull_request) Has been cancelled

Details

6014c7aa32

lunny closed this pull request

2023-10-27 07:29:51 +00:00

lunny reopened this pull request

2023-10-27 07:43:51 +00:00

lunny changed target branch from master to main

2023-10-27 07:43:58 +00:00

lunny added 1 commit 2023-10-30 01:38:19 +00:00

Merge branch 'association-preloading' of gitea.com:sogari/xorm into sogari-association-preloading

test mariadb / test mariadb (pull_request) Successful in 3m28s

Details

test mysql / test mysql (pull_request) Successful in 2m56s

Details

test mssql / test mssql (pull_request) Successful in 3m40s

Details

test cockroach / test cockroach (pull_request) Successful in 5m56s

Details

test sqlite / unit test & test sqlite (pull_request) Failing after 2m26s

Details

test mysql8 / test mysql8 (pull_request) Successful in 3m59s

Details

test postgres / test postgres (pull_request) Successful in 3m15s

Details

test tidb / test tidb (pull_request) Successful in 2m41s

Details

e4a88e57ed

lunny added 1 commit 2023-10-30 01:42:39 +00:00

Fix import sequence

test mariadb / test mariadb (pull_request) Successful in 3m3s

Details

test mysql / test mysql (pull_request) Successful in 3m16s

Details

test mssql / test mssql (pull_request) Successful in 4m49s

Details

test cockroach / test cockroach (pull_request) Successful in 5m48s

Details

test mysql8 / test mysql8 (pull_request) Successful in 3m22s

Details

test postgres / test postgres (pull_request) Successful in 3m12s

Details

test sqlite / unit test & test sqlite (pull_request) Failing after 2m13s

Details

test tidb / test tidb (pull_request) Successful in 2m45s

Details

d4402e7956

lunny added 1 commit 2023-10-30 01:50:04 +00:00

Add comment header

test mariadb / test mariadb (pull_request) Successful in 3m22s

Details

test mysql / test mysql (pull_request) Successful in 3m10s

Details

test mssql / test mssql (pull_request) Successful in 4m4s

Details

test cockroach / test cockroach (pull_request) Successful in 5m57s

Details

test mysql8 / test mysql8 (pull_request) Successful in 3m1s

Details

test postgres / test postgres (pull_request) Successful in 2m20s

Details

test tidb / test tidb (pull_request) Successful in 1m53s

Details

test sqlite / unit test & test sqlite (pull_request) Failing after 4m50s

Details

849554204a

lunny added 1 commit 2023-10-30 05:22:09 +00:00

Merge branch 'main' into sogari-association-preloading

test mariadb / test mariadb (pull_request) Successful in 3m25s

Details

test mysql / test mysql (pull_request) Successful in 3m32s

Details

test mssql / test mssql (pull_request) Successful in 4m46s

Details

test cockroach / test cockroach (pull_request) Successful in 6m32s

Details

test postgres / test postgres (pull_request) Successful in 3m5s

Details

test mysql8 / test mysql8 (pull_request) Successful in 3m17s

Details

test sqlite / unit test & test sqlite (pull_request) Successful in 3m37s

Details

test tidb / test tidb (pull_request) Successful in 2m1s

Details

c49c709096

lunny referenced this pull request

2024-02-11 02:43:05 +00:00

Add support for belongs to relationships #2409

test mariadb / test mariadb (pull_request) Successful in 3m25s

Required

Details

test mysql / test mysql (pull_request) Successful in 3m32s

Required

Details

test mssql / test mssql (pull_request) Successful in 4m46s

Required

Details

test cockroach / test cockroach (pull_request) Successful in 6m32s

Required

Details

test postgres / test postgres (pull_request) Successful in 3m5s

Required

Details

test mysql8 / test mysql8 (pull_request) Successful in 3m17s

Required

Details

test sqlite / unit test & test sqlite (pull_request) Successful in 3m37s

Required

Details

test tidb / test tidb (pull_request) Successful in 2m1s

Required

Details

This pull request is marked as a work in progress.

This branch is out-of-date with the base branch

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u association-preloading:sogari-association-preloading

git checkout sogari-association-preloading

Sign in to join this conversation.

No reviewers

No Milestone

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: xorm/xorm#2241