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

Cannot set primary key ID when id type is int or int64 #586

Open
sebd71 opened this issue Aug 21, 2020 · 1 comment
Open

Cannot set primary key ID when id type is int or int64 #586

sebd71 opened this issue Aug 21, 2020 · 1 comment
Labels
breaking change This feature / fix introduces breaking changes proposal A suggestion for a change, feature, enhancement, etc

Comments

@sebd71
Copy link

sebd71 commented Aug 21, 2020

Hello,

with a postgres connection (same issue at least with sqlite), if ID field is type int or int64 (ID int 'db:"id"' ); calling tx.Create with a preset ID is not possible. ID will be overwritten by create method. ( see dialect_postgresql.go#L64 and dialect_postgresql.go#L87 )

Is it possible to change and have the same behavior as UUID ?

if model.ID() == emptyUUID {

A possible fix, would be to add a check like :

	switch keyType {
	case "int", "int64":
            if model.ID() != 0 {
               // keep current logic that allocate a new id value
            }

Here is a testcase that can be added to executors_test.go file that illustrate this problem :

func Test_Create_Single_Set_ID(t *testing.T) {
	if PDB == nil {
		t.Skip("skipping integration tests")
	}
	r := require.New(t)
	validationLogs = []string{}
	transaction(func(tx *Connection) {
		singleID := &SingleID{
			ID: 123456,
		}
		err := tx.Create(singleID)
		r.NoError(err)
		r.Equal(123456, singleID.ID)
	})
}

And here is the test output:

--- FAIL: Test_Create_Single_Set_ID (0.00s)
    executors_test.go:185: 
                Error Trace:    executors_test.go:185
                                                        pop_test.go:66
                                                        connection.go:166
                                                        pop_test.go:65
                                                        executors_test.go:179
                Error:          Not equal: 
                                expected: 123456
                                actual  : 1
                Test:           Test_Create_Single_Set_ID
FAIL
exit status 1
FAIL    github.com/gobuffalo/pop/v5     0.043s
philband added a commit to philband/pop that referenced this issue Dec 23, 2020
@sio4 sio4 modified the milestones: v6.1.0, v6.0.7 Sep 20, 2022
@sio4 sio4 added the s: triage Some tests need to be run to confirm the issue label Sep 20, 2022
@sio4 sio4 modified the milestones: v6.0.7, Backlog Sep 20, 2022
@sio4
Copy link
Member

sio4 commented Sep 20, 2022

Yeah, I think it could be considerable. However, also with the current behavior, the ID is a concept of rowid when it is an integer type basically. If you want to store your own meaningful ID in your database, having a separate identifier could be a workaround for now.

Something like

type Car struct {
    ID            int
    FactoryNumber int
}

@sio4 sio4 added breaking change This feature / fix introduces breaking changes proposal A suggestion for a change, feature, enhancement, etc and removed s: triage Some tests need to be run to confirm the issue labels Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This feature / fix introduces breaking changes proposal A suggestion for a change, feature, enhancement, etc
Projects
None yet
Development

No branches or pull requests

2 participants