volatiletech / sqlboiler

Generate a Go ORM tailored to your database schema.
BSD 3-Clause "New" or "Revised" License
6.73k stars 544 forks source link

<tablename> redeclared in this block #295

Closed markkuit closed 6 years ago

markkuit commented 6 years ago

If you're having a generation problem please answer these questions before submitting your issue. Thanks!

What version of SQLBoiler are you using (sqlboiler --version)?

v2.7.0

If this happened at generation time what was the full SQLBoiler command you used to generate your models? (if not applicable leave blank)

sqlboiler --wipe -p models -o models mysql

What is the output of the command above with the -d flag added to it? (Provided you are comfortable sharing this, it contains a blueprint of your schema)

Please provide a relevant database schema so we can replicate your issue (Provided you are comfortable sharing this)

Further information. What did you do, what did you expect?

I'm sorry I can't share full schema due to privacy issues; I don't expect it's needed, but if anything I'm willing to share it privately. I've got 3 tables (foos, bars, foo_bar), and runtime generation succeeds. Go test is where it fails (and, consequently, when trying to import) with the following error:

models/foos.go:703:52: Foos redeclared in this block
        previous declaration at models/foos.go:24:6
models/foos.go:709:48: *Foos is not a type
models/foos.go:714:48: *Foos is not a type
models/foos.go:725:67: *Foos is not a type
models/foos.go:750:67: *Foos is not a type
models/foos.go:760:9: *Foos is not a type

In fact, the generated code uses "Foos" both as type struct name and as function name for the "retrieves all the records using an executor" function. Weirdly, the error is fixed if I use singular table names (foo, bar, foo_bar), although that's against naming convention and common logic. This is under Linux, MariaDB 10.0.32. Happy to give whatever further debugging info.

markkuit commented 6 years ago

So, it'd look like I found the culprit, much similar as #168. At this point, it's important to note the original table name is "rsas". Looks like it's yet another tricky word for strmangle/inflect to handle. I can thus confirm this report is strictly related to inflect, and isn't a bug with the code generation itself.

aarondl commented 6 years ago

Yep. Run of the mill inflection problem.

fmt.Println(strmangle.Plural("rsas"))   -> rsas
fmt.Println(strmangle.Singular("rsas")) -> rsas

Did you actually try it when your table name was foo? Because that appears to work correctly so I'd be a little surprised if that failed.

fmt.Println(strmangle.Plural("foos"))   -> foos
fmt.Println(strmangle.Singular("foos")) -> foo
fmt.Println(strmangle.Plural("foo"))    -> foos

I'm not actually sure where we go from here. The table name isn't a real word so adding pluralization rules for it seems a bit odd. Although in the theme of being able to control generation of names a bit better, I think this has given me an idea of being able to override table names and their pluralizations inside the code for v3 (I'm doing a pass on name collisions and up until now I've been thinking of it as only applying to columns/methods but that's clearly not the case as evidenced here).

The painful part is we haven't invested in a mechanism where you could circumvent this with your own code/configuration somehow. So we're at a bit of an impasse other than for you to have to adjust the rules yourself and I'm not sure that'll fly with you. Anyway, happy to talk about different solutions and hear your feedback on the suggestions here.

The place to prod and poke at the rules is here: https://github.com/volatiletech/sqlboiler/blob/master/strmangle/inflect.go

If there's a general rule that could apply to English that we're simply missing I'd be happy to change that though.

markkuit commented 6 years ago

No, foo was just a placeholder to give an example of the scenario. Later on I realized the specific table name was the culprit, that's why I mentioned it.

I ended up using a different table name since it didn't really matter - and it sure was worth it anyway, given the life-changing features sqlboiler provides. I really have no idea how I've lived with just GORM until now.

I looked up english grammar rules about this, and it'd look like the general consensus would be adding "s" (e.g. ATMs, RSAs), or "'s" when dealing with words ending with an "s" themselves (e.g. OS's, SOS's). However, I don't feel like this could be reliably implemented (not to mention the apostrophe often not being legal when dealing with databases), whereas a way to override the generated names sounds much more like an efficient and reliable feature.

Thank you!

aarondl commented 6 years ago

You're welcome. I'm going to close this issue then as we agree it can be covered by manual overrides ala #116 (I'll be working tables into that too). And thank you for passing on your positive feedback, it's the library maintainer's small fuel that we run on to hear those kinds of comments :)