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

Strange issue using many .Relation() #524

Open
frederikhors opened this issue Apr 23, 2022 · 26 comments
Open

Strange issue using many .Relation() #524

frederikhors opened this issue Apr 23, 2022 · 26 comments
Labels
question Further information is requested stale

Comments

@frederikhors
Copy link

frederikhors commented Apr 23, 2022

I don't know why but if I use these Relation()` calls I get the below error:

output := make([]Player, 0, limit)

query := db.NewSelect().Model(&output)

query.Relation("Game").Relation("Game.Team").Relation("Game.Team.Friend").Relation("Game.Team.Marketing").Relation("Game.Team.Marketing.Commerce")
ERR  error="sql: Scan error on column index 61, name \"game__team__friend__marketing__evaluator\": bun: Commerce does not have column \"author\""

As you can see there is the term author which is referring to a Commerce struct column, but that is called: AuthorID hence author_id in SQL.

Is there any limit to usable Relation()?

Is there any limit on the length of parsable SQL?

Go 1.18.1
github.com/uptrace/bun v1.1.4
github.com/uptrace/bun/dialect/pgdialect v1.1.4
github.com/uptrace/bun/driver/pgdriver v1.1.4
github.com/uptrace/bun/extra/bundebug v1.1.4
@vmihailenco
Copy link
Member

This does not look like a known error to me. I will need more details to tell more.

@vmihailenco vmihailenco added the question Further information is requested label Apr 24, 2022
@frederikhors
Copy link
Author

I'm on a family trip these days and I don't know how much free time I'll have. Can you write a small test with many structs to reproduce that number of relations?

I think is all good on my side, I mean everything else (and other .Relations() too (though not that much together)) are working perfectly.

@frederikhors
Copy link
Author

If you cannot I will in a few days...

@EllaFoxo
Copy link

EllaFoxo commented Apr 27, 2022

I can reproduce this issue. The problem appears to be something to do with Relation being called with structs deeper than one level (In OP's case, something of .Relation("Game.Team.Marketing")).

The data structure I'm using is roughly as follows:

type BatchRecord struct {
	ID uuid.UUID `bun:"id,pk,type:uuid,default:gen_random_uuid()"`
	UploadDate time.Time `bun:"upload_date,notnull,type:timestamp,default:current_timestamp"`
}

type Supplier struct {
	ID uuid.UUID `bun:"id,pk,type:uuid,default:gen_random_uuid()"`

	BatchID uuid.UUID `bun:"batch_id,pk,type:uuid"`
	Batch *BatchRecord `bun:"rel:has-one,join:batch_id=id"`

	Number int `bun:"supplier_number,notnull"`
}

type Product struct {
	ID uuid.UUID `bun:"id,pk,type:uuid,default:gen_random_uuid()"`

	BatchID uuid.UUID `bun:"batch_id,pk,type:uuid"`
	Batch *BatchRecord `bun:"rel:has-one,join:batch_id=id"`

	Number int `bun:"product_number,unique,notnull"`

	SupplierNumber int `bun:"supplier_number,notnull"`
	Supplier *Supplier `bun:",rel:has-one,join:supplier_number=supplier_number"`
}

type Price struct {
	ID uuid.UUID `bun:"id,pk,type:uuid,default:gen_random_uuid()"`

	BatchID uuid.UUID `bun:"batch_id,pk,type:uuid"`
	Batch *BatchRecord `bun:"rel:has-one,join:batch_id=id"`

	SupplierNumber int `bun:"supplier_number,notnull"`
	Supplier *Supplier `bun:",rel:has-one,join:supplier_number=supplier_number"`

	ProductNumber int `bun:"product_number,notnull"`
	Product *Product `bun:",rel:has-one,join:product_number=product_number"`

	Cost float32 `bun:"cost"`
	Currency string `bun:"currency"`
}

Select query:

id := uuid.MustParse("0000-uuid-here-0000") // google/uuid type, a known value
val := &Price{}

err := NewSelect().
		Model((*Price)(nil)).
		Relation("Batch").
		Relation("Supplier").
		Relation("Supplier.Batch").
		Relation("Product").
		Relation("Product.Batch").
		Relation("Product.Supplier").
		Relation("Product.Supplier.Batch").
		Where("?TableName.id = ?", id).
		Scan(context.Background(), val)

If anyone can advise whether this is a bug or me being a doofus, I'd appreciate it 😄

@lakkinzi
Copy link

lakkinzi commented May 5, 2022

It seems to me that I am facing a similar problem.

Simplified models:

type News struct {
	bun.BaseModel        `bun:"news,alias:news"`
	ID                   uuid.NullUUID `bun:"id,pk,type:uuid,default:uuid_generate_v4()" json:"id" `
	NewsDoctors            NewsDoctors      `bun:"rel:has-many" json:"newsDoctors"`
}

type NewsDoctor struct {
	bun.BaseModel `bun:"news_doctors,alias:news_doctors"`
	ID            uuid.UUID     `bun:"id,pk,type:uuid,default:uuid_generate_v4()" json:"id" `
	DoctorID      uuid.NullUUID `bun:"type:uuid" json:"doctorId"`
	Doctor        *Doctor       `bun:"rel:belongs-to" json:"doctor"`
	NewsID        uuid.NullUUID `bun:"type:uuid" json:"newsId"`
	News          *News         `bun:"rel:belongs-to" json:"news"`
}

type NewsDoctors []*NewsDoctor

type Doctor struct {
	bun.BaseModel `bun:"doctors,select:doctors_view,alias:doctors_view"`
	ID            uuid.NullUUID `bun:"id,pk,type:uuid,default:uuid_generate_v4()" json:"id" `
	FileInfo         *FileInfo       `bun:"rel:belongs-to" json:"fileInfo"`
	FileInfoID       uuid.NullUUID   `bun:"type:uuid" json:"fileInfoId"`
}

type FileInfo struct {
	ID             uuid.NullUUID `bun:"id,pk,type:uuid,default:uuid_generate_v4()" json:"id" `
	OriginalName   string        `json:"originalName"`
	FileSystemPath string        `json:"fileSystemPath"`
}

Simplified query:

err := r.db.NewSelect().
                Model(item).
		Relation("NewsDoctors.Doctor.FileInfo").
		Where("news.slug = ?", slug).Scan(r.ctx)

Generated sql:

SELECT "news_doctors"."id",
       "news_doctors"."doctor_id",
       "news_doctors"."news_id",
       "doctor"."id"                          AS "doctor__id",
       "doctor__file_info"."id"               AS "doctor__file_info__id",
       "doctor__file_info"."original_name"    AS "doctor__file_info__original_name",
       "doctor__file_info"."file_system_path" AS "doctor__file_info__file_system_path"
FROM "news_doctors"
         LEFT JOIN "doctors_view" AS "doctor" ON ("doctor"."id" = "news_doctors"."doctor_id")
         LEFT JOIN "file_infos" AS "doctor__file_info" ON ("doctor__file_info"."id" = "doctor"."file_info_id")
WHERE ("news_doctors"."news_id" IN ('02861e0c-a054-41d8-b106-8c68eae68a1d'))

The generated request is working. But bun gives an error

*fmt.wrapError: sql: Scan error on column index 20, name "doctor__file_info__id": bun: model=NewsDoctor does not have column=doctor__file_info__id 
  1. Many-to-many relationships are registered manually,
  2. The ID has the uuid type.Null UUID

But in other places such relations work fine.
The system has deeper relations nesting - up to 4-5 - they also work well

@EllaFoxo
Copy link

EllaFoxo commented May 5, 2022

I'll fork and write a test for this issue when I find the time this weekend/next week, then work back from there to see if I can fix it

@frederikhors
Copy link
Author

This is HARD to fix for me and I don't know what to do. Can you please help us, @vmihailenco? Sorry to bother you, really, sorry.

@EllaFoxo
Copy link

EllaFoxo commented May 9, 2022

Struggling to reproduce in test. Running low on time to continue testing tonight, but I have a feeling it could be something to do with using UUIDs for the primary key column. I'll give that a try next.

https://github.com/tinyfluffs/bun/commit/670b175504ca2d4ff930010bf5e91303c1e4f992

@EllaFoxo
Copy link

EllaFoxo commented May 9, 2022

Nope, still cannot reproduce in test, even with UUID.

https://github.com/tinyfluffs/bun/blob/fix/524/internal/dbtest/orm_deep_relation_test.go

@frederikhors
Copy link
Author

I'm not using uuid at all.

@EllaFoxo
Copy link

EllaFoxo commented May 9, 2022

No worries, managed to reproduce now. Will dive deeper.

@frederikhors
Copy link
Author

Please post the initial reproduction here for @vmihailenco that can help you.

@EllaFoxo
Copy link

EllaFoxo commented May 9, 2022

Found the culprit. It appears to be down to a difference in behavior between these two calls to .Scan()

// Broken, because a new structTableModel{} is created and the joins are not copied across from the original model
ctx := context.Background()
val := new(Price)
err := NewSelect().
		Model((*Price)(nil)).
		Relation("Batch").
		Relation("Supplier").
		Relation("Supplier.Batch").
		Relation("Product").
		Relation("Product.Batch").
		Relation("Product.Supplier").
		Relation("Product.Supplier.Batch").
		Where("?TableName.id = ?", id).
		Scan(ctx, val)
// This works, because the scan model is referenced in Model() instead of being sent to Scan() as a parameter
ctx := context.Background()
value := new(Price)
err := NewSelect().
		Model(val).
		Relation("Batch").
		Relation("Supplier").
		Relation("Supplier.Batch").
		Relation("Product").
		Relation("Product.Batch").
		Relation("Product.Supplier").
		Relation("Product.Supplier.Batch").
		Where("?TableName.id = ?", id).
		Scan(ctx)

To me, this appears to be unwanted behavior? Surely scan should treat it the same as if the Model was already requested in the select query?

My test branch is over here

Equally, this yields an unexpected error because the model wasn't requested, but is scanned instead:

val := new(Price)
db.NewSelect().
		Where("?TableName.id = ?", price.ID).
		Relation("Batch").
		Relation("Supplier").
		Relation("Supplier.Batch").
		Relation("Product").
		Relation("Product.Batch").
		Relation("Product.Supplier").
		Relation("Product.Supplier.Batch").
		Limit(1).
		Scan(ctx, val)

@frederikhors
Copy link
Author

@tinyfluffs thank you very much!

@frederikhors
Copy link
Author

@vmihailenco this is HUGE. Can you please help us?

@un-versed
Copy link

un-versed commented Jun 13, 2022

I'm facing the same issue here...

I think the error is related to column length in my case.

It looks like bun is shortening the column name:

"sql: Scan error on column index 14, name \"operation_step_instance__operation_instance__****_transaction_u\": bun: OperationInstance does not have column \"****_transaction_u\"",

The "*" is a string related to business logic and I can't share... But the original column name is "****_transaction_uuid".

@un-versed
Copy link

@frederikhors any update on this?

@frederikhors
Copy link
Author

@un-versed you should ask to the maintainer. I'm leaving Bun for this issue too.

@un-versed
Copy link

@frederikhors oh :(

bun is a great tool, but this issue is weird...

@vmihailenco any ideas?

@un-versed
Copy link

@frederikhors btw, just gimme some info: were you using the pgx driver with bun?

@EllaFoxo
Copy link

EllaFoxo commented Jun 14, 2022

I'm facing the same issue here...

I think the error is related to column length in my case.

It looks like bun is shortening the column name:

"sql: Scan error on column index 14, name \"operation_step_instance__operation_instance__****_transaction_u\": bun: OperationInstance does not have column \"****_transaction_u\"",

The "*" is a string related to business logic and I can't share... But the original column name is "****_transaction_uuid".

@un-versed I think this may be a separate bug, but I've also encountered the issue. Got a fix in for a related problem, but if I find time during my midsummer break next week, I'll also dig into this one.

@un-versed
Copy link

I'm facing the same issue here...

I think the error is related to column length in my case.

It looks like bun is shortening the column name:

"sql: Scan error on column index 14, name \"operation_step_instance__operation_instance__****_transaction_u\": bun: OperationInstance does not have column \"****_transaction_u\"",

The "*" is a string related to business logic and I can't share... But the original column name is "****_transaction_uuid".

@un-versed I think this may be a separate bug, but I've also encountered the issue. Got a fix in for a related problem, but if I find time during my midsummer break next week, I'll also dig into this one.

@tinyfluffs I created a discussion with more details after some research #566

I found the same issue in another ORM

@lefelys
Copy link

lefelys commented Jul 24, 2022

The original problem with different behaviour between NewSelect().Model((*X)(nil)).Relation("Y.Z").Scan(ctx, &val) and NewSelect().Model(&val).Relation("Y.Z").Scan(ctx) is very huge.

I think the first variant should never be used at all for now and documentation updated accordingly (now it has many examples like db.NewSelect().Model((*User)(nil)).Where(...).

From my understanding the problem is the following:

  1. Calling NewSelect().Model(&X) initializes model for struct X and stores pointer to X in *bun.SelectQuery
  2. A call to .Relation(...) initializes joins with traversing fields of struct X and storing relations as pointers to fields in struct X
  3. If after that you call .Scan(ctx, &AnotherX) - bun will just ignore everything it did for the initial model (with all relations built and stored) and will again initialize a model for &AnotherX missing all relations

Possible fixes that I can think of:
Option 1: Don't initialize relations on .Relation(...) call - only store the string passed. Do an actual job on building relations only after .Scan(...) is called
Option 2: Perform model and relations initializations without having strong references to the initial model. Storing only field names and use of FieldByName is something that can help here, but it can be very inefficient for big structs since it just iterates over struct fields on every call

@frederikhors
Copy link
Author

Option 3: do not use ORM at all. 😄

@lakkinzi
Copy link

lakkinzi commented Aug 31, 2022

Perhaps there is some connection with the order of the fields in the original structure or the connection of two foreign keys to one table?

Copy link

github-actions bot commented Nov 7, 2024

This issue has been automatically marked as stale because it has not had activity in the last 30 days. If there is no update within the next 7 days, this issue will be closed.

@github-actions github-actions bot added the stale label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested stale
Projects
None yet
Development

No branches or pull requests

6 participants