Skip to content

fix : fail to alter column from smallint to boolean #165

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

Merged
merged 6 commits into from
Mar 6, 2023

Conversation

jeffry-luqman
Copy link
Contributor

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Fix USING ?::? sql for boolean field on AlterColumn func

Without this, it will report error as I described in the issue #164

User Case Description

If we want to change column from smallint to boolean, it will report error like below:

2023/02/28 14:03:35 /home/user/go/pkg/mod/gorm.io/driver/postgres@v1.4.8/migrator.go:318 ERROR: cannot cast type smallint to boolean (SQLSTATE 42846)
[0.374ms] [rows:0] ALTER TABLE "column_structs" ALTER COLUMN "is_active" TYPE boolean USING "is_active"::boolean

We only need to fix sql become USING xxx::int::boolean for boolean field in func AlterColumn in postgres@v1.4.8/migrator.go

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
raed667 Raed
Copy link
Member

@a631807682 a631807682 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, it should throw an error if the default rules fail, instead of us defining the conversion rules.

for example

type ColumnStruct struct {
name string
IsActive string // set IsActive to "true" and alter column using `::INT::boolean`
}


@jeffry-luqman
Copy link
Contributor Author

In fact, it should throw an error if the default rules fail, instead of us defining the conversion rules.

for example

type ColumnStruct struct {
name string
IsActive string // set IsActive to "true" and alter column using `::INT::boolean`
}

OK, I just sent the fix with this sql, please review again

ALTER TABLE "column_structs" ALTER COLUMN "is_active" TYPE boolean USING CASE WHEN "is_active" IN ('1','TRUE','true','T','t') THEN true ELSE false END

Copy link
Member

@a631807682 a631807682 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may not have made it clear, we can automatically convert from string to boolean by ::boolean instead of ::INT:boolean.
We may need a function, possibly called genUsingExpression, to handle expressions that cannot be converted by default from type a to type b.
When type string to type boolean, use default conversion ::boolean, when type smallint to type boolean, use the specified conversion ::INT:boolean. If there are other types that have the same problem, we just need to fill in this method.

@jeffry-luqman
Copy link
Contributor Author

ok, I've fixed it

@jeffry-luqman
Copy link
Contributor Author

if there's nothing else to be fixed, can it be merged @a631807682 @jinzhu , please?

@jeffry-luqman
Copy link
Contributor Author

close #164 at v1.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants