-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Add Namer-based column lookup to Schema.LookUpField #7619
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
Conversation
|
Introduce Namer-based fallback in Adds a small, non-breaking enhancement to Key Changes• Modified Affected Areas• This summary was automatically generated by @propel-code-bot |
|
Thank you for your PR, But I think we could simply set a default namer that uses uppercase when initializing the Oracle driver and it won't introduce any runtime performance overhead. |
|
That would work if the caser worked better than the current one. Currently the option for that behavior exists via |
|
Also,
That won't work if someone specifies type Bar struct {
gorm.Model
Foo string `gorm:"column:FOO"`
}The default namer would change that to Then when the user would attempt something like: var bar &Bar{}
db.Model(&Bar{}).Where("foo = ?", "bar").Find(bar)the user would be sad. Because the string I get the concern about performance. My suspicion is that it would have little impact because of how often Since "case insensitive" was the expectation (no quotes used even though gorm FORCES quotes whenever Otherwise, as-is, the query will fail unexpectedly because Incidentally, thank you for this fantastic library. I greatly appreciate the work you have put into this. I enjoy using gorm and will use it for many years. :) |
What did this pull request do?
Currently,
func (schema *Schema) LookUpField(name string) *Fieldchecks:and returns
nilin the event that the field is not found in either case.This enhances the behavior slightly. The original function is intact, in addition, if a
schema.Namerhas been defined,schema.FieldsByDBNameis checked once more with the namer-driven column name.User Case Description
Admittedly, this is quite self-serving. I've
writtenheavily modified so much that it may as well not be a fork agorm-oracleplugin. One of the many idiosyncrasies of oracle is that it stores all non-quoted, non-reserved-word identifiers asUPPERCASE. This breaks from sanity, logic, good common-sense, most language conventions, and gorm's default handling. If acolumn:tag is set for a field, this value is used (without Namer.ColumnName transformation) exactly as-is if theNamingCaseSensitiveflag is set tofalse. IfNamingCaseSensitiveis set totrue, gorm will quote the exact value of thecolumn:tag. This wouldn't be an issue if oracle treated "foo" and foo the same. But, alas, oracle does not equate "foo" and FOO. However, "FOO" and FOO are considered "same" in oracle-ese. It bears noting that my gorm-oracle plugin provides the necessaryschema.Namerlogic that will handle the proper quote handling of identifiers that must be quoted (reserved words, user-provided quoted identifiers, etc). But without this change, the column metadata returned by oracle (and used by gorm.Scan) will never match thecolumn:foounless the tag is specifiedcolumn:FOO."Why not just use
column:FOO?"Because it's ugly. I don't want to litter my code with
column:SOME_STUPID_UPPER_CASE_NAME. It's also unnecessary to require two different structures for oracle and any other db when one structure would work just fine; if gorm supported it.As an aside, for some weird reason, gorm does not include a "default naming case" option. When
NoLowerCaseis set to true, theDBNamebecomes somethinguselessless-than-useful:UserIDfield name becomesUSERIDin oracle with default gorm. Arguably, this can easily be solved by gorm providing a default case handler:ScreamingSnakeCase,CamelCase(orPascalCasewhichever you prefer), orSnakeCaseor whatever. This would allow users to avoid forcibly setting thecolumn:tag when it otherwise would not have been necessary had gorm provided a means to convert the field name with more precision."Why not just omit
column:altogether and live with gorm's default naming?"No.