Repository: go-gorm/gorm
Pull request #3848 (2020-12-10 23:05:18)

Fix building of `clause.Eq` and `clause.Neq` expressions that fail to handle `(*T)(nil)` use cases correctly
<!--
Make sure these boxes checked before submitting your pull request.

For significant changes, please open an issue to make an agreement on an implementation design/plan first before starting it.
-->

- [x] Do only one thing
- [x] Non breaking API changes
- [x] Tested

### What did this pull request do?

This PR fixes [`clause.Eq`](https://github.com/go-gorm/gorm/blob/e1952924e2a844eca52e5030f7b46b78de6ec135/clause/expression.go#L198) and [`clause.Neq`](https://github.com/go-gorm/gorm/blob/e1952924e2a844eca52e5030f7b46b78de6ec135/clause/expression.go#L219) expressions ability to identify `(*T)(nil)` as `nil` values.

The culprit lies in [./clause/expression.go#L206 `clause.Eq`](https://github.com/go-gorm/gorm/blob/e1952924e2a844eca52e5030f7b46b78de6ec135/clause/expression.go#L206) expression that fails to identify `(*T)(nil)` values as `nil` values.
The same problem is present for the [./clause/expression.go#L224 `clause.Neq`](https://github.com/go-gorm/gorm/blob/e1952924e2a844eca52e5030f7b46b78de6ec135/clause/expression.go#L224) expression.

The problem is discussed on [Stackoverflow: "Check for nil and nil interface in Go"](https://stackoverflow.com/questions/13476349/check-for-nil-and-nil-interface-in-go)

[Go Playground](https://play.golang.org/p/XW6DBEYAfaz) for the problem.

### User Case Description

Previously the following code produced an invalid SQL where clause:
```go
nilStringPtr := (*string)(nil)
DB.Model(SomeModel{}).
    .Where(map[string]interface{} {
        "field1": "some-value",
        "field2": nil,
        "field3": nilStringPtr, // It could be any (*T)(nil) type
    }).
    Update("field2", "some-other-value")
```
Expected: ```UPDATE `some_model` SET `field2` = ? WHERE `field1` = ? AND `field2` IS NULL AND `field3` IS NULL```
Actual: ```UPDATE `some_model` SET `field2` = ? WHERE `field1` = ? AND `field2` IS NULL AND `field3` = ?```



Patch:
diff --git a/clause/expression.go b/clause/expression.go
index b30c46b..3844d66 100644
--- a/clause/expression.go
+++ b/clause/expression.go
@@ -203,7 +203,7 @@ type Eq struct {
 func (eq Eq) Build(builder Builder) {
 	builder.WriteQuoted(eq.Column)
 
-	if eq.Value == nil {
+	if eqNil(eq.Value) {
 		builder.WriteString(" IS NULL")
 	} else {
 		builder.WriteString(" = ")
@@ -221,7 +221,7 @@ type Neq Eq
 func (neq Neq) Build(builder Builder) {
 	builder.WriteQuoted(neq.Column)
 
-	if neq.Value == nil {
+	if eqNil(neq.Value) {
 		builder.WriteString(" IS NOT NULL")
 	} else {
 		builder.WriteString(" <> ")
@@ -299,3 +299,12 @@ func (like Like) NegationBuild(builder Builder) {
 	builder.WriteString(" NOT LIKE ")
 	builder.AddVar(builder, like.Value)
 }
+
+func eqNil(value interface{}) bool {
+	return value == nil || eqNilReflect(value)
+}
+
+func eqNilReflect(value interface{}) bool {
+	reflectValue := reflect.ValueOf(value)
+	return reflectValue.Kind() == reflect.Ptr && reflectValue.IsNil()
+}


Test patch:
diff --git a/clause/expression_test.go b/clause/expression_test.go
index 8308248..9e3d7ba 100644
--- a/clause/expression_test.go
+++ b/clause/expression_test.go
@@ -101,3 +101,52 @@ func TestNamedExpr(t *testing.T) {
 		})
 	}
 }
+
+func TestExpression(t *testing.T) {
+	column := "column-name"
+	results := []struct {
+		Expressions    []clause.Expression
+		Result string
+	}{{
+		Expressions: []clause.Expression{
+			clause.Eq{Column: column, Value: "column-value"},
+		},
+		Result: "`column-name` = ?",
+	},{
+		Expressions: []clause.Expression{
+			clause.Eq{Column: column, Value: nil},
+			clause.Eq{Column: column, Value: (*string)(nil)},
+			clause.Eq{Column: column, Value: (*int)(nil)},
+			clause.Eq{Column: column, Value: (*bool)(nil)},
+			clause.Eq{Column: column, Value: (interface{})(nil)},
+		},
+		Result: "`column-name` IS NULL",
+	},{
+		Expressions: []clause.Expression{
+			clause.Neq{Column: column, Value: "column-value"},
+		},
+		Result: "`column-name` <> ?",
+	},{
+		Expressions: []clause.Expression{
+			clause.Neq{Column: column, Value: nil},
+			clause.Neq{Column: column, Value: (*string)(nil)},
+			clause.Neq{Column: column, Value: (*int)(nil)},
+			clause.Neq{Column: column, Value: (*bool)(nil)},
+			clause.Neq{Column: column, Value: (interface{})(nil)},
+		},
+		Result: "`column-name` IS NOT NULL",
+	}}
+
+	for idx, result := range results {
+		for idy, expression := range result.Expressions {
+			t.Run(fmt.Sprintf("case #%v.%v", idx, idy), func(t *testing.T) {
+				user, _ := schema.Parse(&tests.User{}, &sync.Map{}, db.NamingStrategy)
+				stmt := &gorm.Statement{DB: db, Table: user.Table, Schema: user, Clauses: map[string]clause.Clause{}}
+				expression.Build(stmt)
+				if stmt.SQL.String() != result.Result {
+					t.Errorf("generated SQL is not equal, expects %v, but got %v", result.Result, stmt.SQL.String())
+				}
+			})
+		}
+	}
+}