gorm icon indicating copy to clipboard operation
gorm copied to clipboard

Embedded nil pointer structs should be retrieved as nil

Open nickpalmer opened this issue 2 years ago • 3 comments

Describe the feature

Gorm should round trip an embedded pointer to a struct as nil when the struct is given as nil.

Motivation

My domain model has optional embedded objects, which then may have required fields on them.

Our validator will validate the embedded object's required fields if the pointer value is non-nil but will ignore the required validations on the embedded struct if it is nil.

Gorm should detect that that the embedded struct has the zero value and not set it to the zero value if only zero values have been loaded into that value.

Without this a save of nil is not preserved on reload for the embedded struct.

Related Issues

  • None that I could find.

Example Failing Test

diff --git a/tests/embedded_struct_test.go b/tests/embedded_struct_test.go
index 312a5c3..e5db47a 100644
--- a/tests/embedded_struct_test.go
+++ b/tests/embedded_struct_test.go
@@ -90,9 +90,15 @@ func TestEmbeddedPointerTypeStruct(t *testing.T) {
                URL   string
        }

+       type Author struct {
+               ID    string
+               Name  string
+               Email string
+       }
+
        type HNPost struct {
                *BasePost
                Upvotes int32
+               *Author `gorm:"EmbeddedPrefix:user_"` // Embedded struct
        }

        DB.Migrator().DropTable(&HNPost{})
@@ -110,6 +116,10 @@ func TestEmbeddedPointerTypeStruct(t *testing.T) {
        if hnPost.Title != "embedded_pointer_type" {
                t.Errorf("Should find correct value for embedded pointer type")
        }
+
+       if hnPost.Author != nil {
+               t.Errorf("Expected to get back a nil Author but got: %v", hnPost.Author)
+       }
 }

 type Content struct {

This change to the existing test outputs:

--- FAIL: TestEmbeddedPointerTypeStruct (0.01s)
    embedded_struct_test.go:121: Expected to get back a nil Author but got: &{  }

This test would pass if this feature were implemented.

nickpalmer avatar Jun 15 '22 18:06 nickpalmer

Cause by #5417 #5388

When we reuse Slice elem, we need to reset its value, which results in constructing a embedded model. This model may also have been instantiated in other subsets of slices.

We can only reset before Field.Set , or set embedded model with zero value after, but both make #5388 no performance gain.

I recommend adding test files and revert them. @jinzhu

a631807682 avatar Jun 16 '22 05:06 a631807682

I recommend adding test files and revert them. @jinzhu

Not sure what you mean here.

As a workaround I wrote a utility that walks the object and nils out any zero values recursively depth first, but of course it would be faster for Gorm to recycle the zero values it allocated in scan.go instead of Field.Set them.

It is probably necessary to add a Config defaulting to off to allow users of Gorm turn on this behavior, since consumers of Gorm may have code that already assumes they get a zero value and not nil.

nickpalmer avatar Jun 16 '22 12:06 nickpalmer

I had to implement a similar utility to work around this. Note that I am using gorm 1.23.4 and I'm seeing different results than those from @nickpalmer . If the Author remains as defined above in his test, then the Author is returned as nil. But if the Author struct is changed to use a pointer within it like so:

type Author struct {
	ID    string
	Name  string
	Email *string
}

Then, the result is &{ }

When upgrading to 1.23.8, I see &{ } either way.

rgcostea avatar Aug 23 '22 22:08 rgcostea

@jinzhu Is this issue being worked upon? Has this been acknowledged?

This is causing regression failure for us. One of our embedded struct which used to be read as nil, is now coming as zero value of the struct.

This was working for us in v1.23.4 and breaking in 1.24.6.

akshay58538 avatar Apr 05 '23 11:04 akshay58538