Skip to content
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

fix(create): fix insert column order #6855

Merged
merged 3 commits into from Mar 18, 2024
Merged

Conversation

archever
Copy link
Contributor

@archever archever commented Feb 28, 2024

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

What did this pull request do?

make insert sql column order stable for unittest

User Case Description

gorm create() interface generated sql not stable for each test run. I changed the map interater to a stable slice in order to keep the column order.

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.

Can you add some unit tests to describe what is being fixed?

@archever
Copy link
Contributor Author

archever commented Mar 1, 2024

Can you add some unit tests to describe what is being fixed?

add a ut case

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.

The order of defaultValueFieldsHavingValue elements is based on FieldsWithDefaultDBValue. https://github.com/go-gorm/gorm/pull/6855/files#diff-7539aa7c170a85138fa67c7846b65fda95a51169bcf8637d5961ea570307d755R284
I don't understand why the order needs to be maintained here again, and the unit test passed before the change, can you describe why we need it?

@archever
Copy link
Contributor Author

archever commented Mar 7, 2024

the type of defaultValueFieldsHavingValue is a map. before the change, column values is appended by iterating a map, which is not in a stable order. after the change, column values is appended following the order of FieldsWithDefaultDBValue, which is in stable order.

@jinzhu
Copy link
Member

jinzhu commented Mar 9, 2024

Hi @archever,

Thank you for your PR. Could you please remove those dependencies in go.mod? We aim to keep GORM's dependencies clean and tidy.

	github.com/stretchr/testify v1.8.4
)

require (
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	gopkg.in/yaml.v3 v3.0.1 // indirect
)

@archever
Copy link
Contributor Author

@jinzhu done

Comment on lines +26 to +27
t.Errorf("parse schema error: %v, is not expected", err)
return
Copy link
Member

Choose a reason for hiding this comment

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

use t.Fatalf

@jinzhu jinzhu merged commit f7ebf04 into go-gorm:master Mar 18, 2024
34 checks passed
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

4 participants