feat(mysql): handle unsigned integers

[roccoblues]

Hi,

first of all: thanks for your work on sqlc, it's really amazing!

This adds support for the UNSIGNED attribute in MySQL. Unfortunately we can only support NOT NULL columns as there are no sql.NullUInt* types and most likely never will.

We recently ran into this problem when a value from an INT UNSIGNED NOT NULL was to big for the int32 sqlc currently generates.

Let me know what you think and if something is missing.

Thanks!

[roccoblues]

Thanks for running the CI tests. I only ran make test 🙈 . Looks I missed the actual query part of the code. I'll have another look at this.

[roccoblues]

Ok, I've updated the branch to correctly return uint* from queries.

The tests pass and the generated code changes also look correct (no more Venue -> ListVenueRow but just int64 -> uint64.

[kyleconroy]

@roccoblues I recently merged #1759 which is probably the source of your merge conflicts.

I was surprised by the number of changes in the generated code. A quick search lead me to the MySQL docs:

SERIAL is an alias for BIGINT UNSIGNED NOT NULL AUTO_INCREMENT UNIQUE

The fact that we don't have a good solution for NULL uint64 types does make me want to wait on this change.

[roccoblues]

@roccoblues I recently merged #1759 which is probably the source of your merge conflicts.

I've updated my branch.

I was surprised by the number of changes in the generated code. A quick search lead me to the MySQL docs:

SERIAL is an alias for BIGINT UNSIGNED NOT NULL AUTO_INCREMENT UNIQUE

Same here. 😄

The fact that we don't have a good solution for NULL uint64 types does make me want to wait on this change.

I understand that it's not optimal to have only the NOT NULL version generate the correct type. On the other hand this change fixes at least one case. And given the fact that SERIAL is the preferred way for primary keys, probably the more common one.

But it's your call. Maybe with generics out there will be a solution for NULL types at some point.

At least we have the PR here for people to find if they run into the same problem.

Thanks again for your work!

[shawdon]

Hi @kyleconroy , may I ask why this PR is not merged?

[kounoike]

Hi @kyleconroy , any updates?

[roccoblues]

I've updated my branch with the latest changes from main.

[shawdon]

I've updated my branch with the latest changes from main.

@kyleconroy Hi, could you merge this PR?

[SebastienMelki]

Hello @kyleconroy , any reason why this isn't merged yet? Are there any concerns to address?

Thank you for the awesome tool!

[kyleconroy]

I've merged in main to fix a merge conflict with some generated proto code, so this is technically ready to merge.

I'm still concerned about the backwards incompatible changes and the fact that we don't have an answer for null types. For users with a large number of tables and queries, this could make upgrading to v1.19.0 difficult.

A question for those of you waiting for this change: are you using SERIAL columns or are you using columns with a specific UNSIGNED attribute?

[kyleconroy]

This has merge conflicts again, but don't worry @roccoblues, I'm going to take this over the finish line.

The plan is to update the overrides so that you can specify overrides for UNSIGNED types specifically. This will give people an escape hatch if they aren't happy with the new default output.

[roccoblues]

A question for those of you waiting for this change: are you using SERIAL columns or are you using columns with a specific UNSIGNED attribute?

We have tables with explicit UNSIGNED attributes.

This has merge conflicts again, but don't worry @roccoblues, I'm going to take this over the finish line.

🙏🏼 thanks!

[kyleconroy]

Alright, I've fixed the merge conflict and added support for targeting overrides at unsigned columns. If a user would like to back out of these changes, they can add the following to their configuration.

{
    "version": "2",
    "sql": [
        {
            "schema": "query.sql",
            "queries": "query.sql",
            "engine": "mysql",
            "gen": {
                "go": {
                    "package": "db",
                    "out": "go",
                    "overrides": [
                        {
                            "db_type": "bigint",
                            "go_type": "int64",
                            "unsigned": true
                        }
                    ]
                }
            }
        }
    ]
}

[abh]

I haven't tried it yet, but https://github.com/lomsa-dev/gonull seems like it will work for BIGINT NULL / nullable uint64 columns.

        overrides:
          - db_type: "bigint"
            go_type: "github.com/lomsa-dev/gonull.Nullable[uint64]"
            unsigned: true
            nullable: true