jackc / pgx

PostgreSQL driver and toolkit for Go
MIT License
10.86k stars 846 forks source link

RowToStructByName Snake Case Collision #2085

Closed nolandseigler closed 4 months ago

nolandseigler commented 4 months ago

rows.go fieldPosByName currently removes all "_" characters. This causes structs with "db" tags like "account_id" and "accountid" to be treated as the same tag. The index returned by fieldPosByName is always the first field tagged which causes an error like "struct doesn't have corresponding row field accountid".

I have written a test that demonstrates the issue and implemented some proof of concept code to demonstrate a potential fix. All tests pass.

This is my first PR. Please let me know if I am now following the proper procedure. I can certainly open up a discussion or issue as well. Thank you.

jackc commented 4 months ago

I suppose this is more correct behavior when a db tag is provided. Though it's hard to see how having a database table where the columns names only differed by underscores is a good idea...

That said, if this change is going to be made it should cover all normalization. That is, if a db tag is provided all name normalization is disabled. I would suggest also doing a simple == comparison instead of the case insensitive compare.

nolandseigler commented 4 months ago

I suppose this is more correct behavior when a db tag is provided. Though it's hard to see how having a database table where the columns names only differed by underscores is a good idea...

That said, if this change is going to be made it should cover all normalization. That is, if a db tag is provided all name normalization is disabled. I would suggest also doing a simple == comparison instead of the case insensitive compare.

Great idea. Thanks for the quick response. I will do that right now.