fraenky8 / tables-to-go

convert your database tables to structs easily
MIT License
233 stars 42 forks source link

Error if column names start with digits #18

Closed ronniegane closed 5 years ago

ronniegane commented 5 years ago

If the column names of a table start with digits, the tool fails to create the struct.

> processing table "MyTable"
        > number of columns: 4
                > id
                > name
                > 1person
                > 2person

run error: could not create struct file for table MyTable: could not format file /home/ronnie/project/models/MyTable.go: 12:1: expected '}', found 1% 

I'm not sure what the best thing to do here would be. I know it's bad practice to have SQL column names start with numbers but it's technically allowed. So we may have to deal with it if we don't have authority to change the actual database schema.

But Golang obviously doesn't allow fieldnames to start with numbers, so what would be a sensible thing to do?

This may open up a whole can of worms because MySQL table and column names can be full unicode but Go only allows letters, underscore, and digits, and each identifier has to start with a letter or underscore.

Then again, it is really absurd to use non-letter characters in table and column names, so it would probably be fair enough to just not deal with them.

fraenky8 commented 5 years ago

Hi @ronniegane thanks for pointing this out. Indeed I never tried and never used column names starting with a digit. Even not sure how the go sql drivers behave in this case.

As you said since go doesn't allow identifiers to start with a digit, we can't do too much here:

The unicode should not be an issue, since go itself supports at least unicode letter codepoints. People who are using emoijs/symbols in table/column names are not allowed to use this tool then 😅 ⚔️

Let me know what you think - I'm fine with one or even both of the options. 👍

ronniegane commented 5 years ago

I think prefixing would be less confusing than swapping the digits, you'd be less surprised if the column 1fish2fish became _1fish2fish than if it became fish2fish1.

I was originally planning to prefix with an underscore (_1fish2fish) because that looks cleanest. But I think that will make the fields private. So maybe we need to make it a capital instead? X1fish2fish or X_1fish2fish

I think just prefixing any digits is an okay default as long as it's documented. I'm not sure if there is much value in a flag for adding a custom prefix or suffix to every column.

I also noticed that there is a similar issue with whitespace in column names (again, I know, terrible practice). I'm planning to just make the tool replace whitespace with underscores.

I'll have a go at these changes on my fork and let you know how it works out.

I've seen some tables in work lately that have had brackets in the table names, but those people can just fix their schema :laughing:

fraenky8 commented 5 years ago

I think just prefixing any digits is an okay default as long as it's documented.

Ok let's do that then! Let's introduce an option for the user to be able the change the prefix (which only has effect on columns starting with a number). That would make it a more flexible.

... with whitespace in column name ... ... had brackets in the table names ...

Oh noo why 🙈

I'm planning to just make the tool replace whitespace with underscores.

This could break the snake_case/camelCase options - so it should be done before the conversion into one of these formats.

I'll have a go at these changes on my fork and let you know how it works out.

I just merged my PR (#17) for the unit tests where I introduced some changes, would be good to update your fork so your code base is up-to-date and has some unit tests! :ok_hand:

Thanks for contributing! :)