gorm icon indicating copy to clipboard operation
gorm copied to clipboard

[Bug] Got wrong value in Find/Scan with JOIN and same column name

Open StephanoGeorge opened this issue 2 years ago • 8 comments

GORM Playground Link

https://github.com/go-gorm/playground/pull/516

Description

Got wrong value in Find/Scan with JOIN and same column name.

See pull request. Thanks!

StephanoGeorge avatar Sep 21 '22 06:09 StephanoGeorge

  1. Gorm will not parse sql, so it cannot determine which model the same column name belongs to, you need to use an alias.
  2. Similarly, when the embedded struct uses the same field name, it is impossible to know the corresponding relationship between it and the column, unless you specify it by tag.

a631807682 avatar Sep 21 '22 08:09 a631807682

@a631807682 But according to https://github.com/go-gorm/gorm/issues/5142, GORM should handle this, isn't it?

StephanoGeorge avatar Sep 21 '22 08:09 StephanoGeorge

@StephanoGeorge Indeed, this seems to be supported, it seems to be a match attempt, but as soon as the order is changed, the test must fail, so I don't recommend it, but I'll follow up on it.

https://github.com/go-gorm/gorm/blob/master/tests/scan_test.go#L179

a631807682 avatar Sep 21 '22 09:09 a631807682

Thanks! Select("people.*, addresses.*") will make result column name has table name as prefix, so the Scan can be done without ambiguity

StephanoGeorge avatar Sep 21 '22 12:09 StephanoGeorge

It only works if the defined field and the query column are exactly the same. It cause errors if we select more column like:

Select("people.*,people.id, addresses.*")

It cause errors if we defined more field like your case do:

type User struct {
	gorm.Model
	Name      string
	Pets      []Pet	// cause error
	Languages []Language `gorm:"many2many:user_language;"` // cause error
}

type Result struct {
	User
	UserLanguage
	Language
	Pet
}

https://github.com/go-gorm/gorm/blob/master/scan.go#L206

I think this is an unsolvable problem, so this may not be a fully implemented feature.

a631807682 avatar Sep 21 '22 13:09 a631807682

I think GORM can parse Select("user.*", "user_language.skilled", "language.*") if each string in Select satisfy `?(.+)`?\..+, and scan first 6 fields to Result.User, then scan skilled to Result.UserLanguage, and so on. What do you think?

StephanoGeorge avatar Sep 22 '22 07:09 StephanoGeorge

I don't think that's a good idea

  1. Parsing SQL affects efficiency, even if users are sure their SQL is fine.

  2. This requires the support of all drivers, even for different versions, to ensure consistency with the database parser.

  3. Even if we parse the SQL, we still don't know their correspondence, for example, I have a sharding table, does it need to be filled into user?

Select("user_1.* ...")

a631807682 avatar Sep 22 '22 07:09 a631807682

  1. I meant only parsing strings in Select, they are only table or column names.
  2. GORM already needs to support all drivers for SELECT, INSERT, UPDATE, DELETE.
  3. If we have sharding table, we will Select("user.*"), GORM will find which table we will use, right?

StephanoGeorge avatar Sep 22 '22 07:09 StephanoGeorge

@a631807682 I can make a PR if you accept this

StephanoGeorge avatar Sep 26 '22 01:09 StephanoGeorge

@StephanoGeorge Welcome, the maintainer will review it

a631807682 avatar Sep 26 '22 05:09 a631807682

I found that GORM doesn't have to parse Select(). @jinzhu Please review the PR.

StephanoGeorge avatar Sep 28 '22 02:09 StephanoGeorge

complete via https://github.com/go-gorm/gorm/commit/a3cc6c6088c1e2aa8cbd174f4714e7fc6d0acd59

a631807682 avatar Oct 20 '22 07:10 a631807682