Repository: snowflakedb/gosnowflake
Pull request #761 (2023-03-21 05:19:19)

SNOW-755844: Pointer datatype *time.Time returns <nil> value
Issue: https://github.com/snowflakedb/snowflake-sdks-drivers-issues-teamwork/issues/246

Starting with gosnowflake 1.6.13, the use of a pointer *time.Time type in user-defined structures used to perform updates/inserts has silently stopped working, and nil value is used instead of the value it points to.
This PR fixes the regression introduced by PR #642. All array binding insertion using []interface{} will now be wrapped in the  interfaceArrayBinding struct, instead of assuming them as the reflect.Slice and reflect.Struct types.

```
type interfaceArrayBinding struct {
	hasTimezone       bool
	tzType            timezoneType
	timezoneTypeArray interface{}
}
```

### Checklist
- [X] Code compiles correctly
- [X] Run ``make fmt`` to fix inconsistent formats
- [X] Run ``make lint`` to get lint errors and fix all of them
- [X] Created tests which fail without the change (if possible)
- [X] All tests passing
- [ ] Extended the README / documentation, if necessary



Patch:
diff --git a/bind_uploader.go b/bind_uploader.go
index 191dea7..a840720 100644
--- a/bind_uploader.go
+++ b/bind_uploader.go
@@ -292,12 +292,8 @@ func supportedArrayBind(nv *driver.NamedValue) bool {
 		// TODO SNOW-176486 variant, object, array
 
 		// Support for bulk array binding insertion using []interface{}
-		nvValue := reflect.ValueOf(nv)
-		if nvValue.Kind() == reflect.Ptr {
-			value := reflect.Indirect(reflect.ValueOf(nv.Value))
-			if value.Kind() == reflect.Slice || value.Kind() == reflect.Struct {
-				return true
-			}
+		if isInterfaceArrayBinding(nv.Value) {
+			return true
 		}
 		return false
 	}
diff --git a/converter.go b/converter.go
index cb07f3b..373909f 100644
--- a/converter.go
+++ b/converter.go
@@ -37,11 +37,23 @@ const (
 	TimeType
 )
 
-type timezoneTypeArrayBinding struct {
+type interfaceArrayBinding struct {
+	hasTimezone       bool
 	tzType            timezoneType
 	timezoneTypeArray interface{}
 }
 
+func isInterfaceArrayBinding(t interface{}) bool {
+	switch t.(type) {
+	case interfaceArrayBinding:
+		return true
+	case *interfaceArrayBinding:
+		return true
+	default:
+		return false
+	}
+}
+
 // goTypeToSnowflake translates Go data type to Snowflake data type.
 func goTypeToSnowflake(v driver.Value, tsmode snowflakeType) snowflakeType {
 	switch t := v.(type) {
@@ -637,10 +649,14 @@ func Array(a interface{}, typ ...timezoneType) interface{} {
 	case []interface{}, *[]interface{}:
 		// Support for bulk array binding insertion using []interface{}
 		if len(typ) < 1 {
-			return a
+			return interfaceArrayBinding{
+				hasTimezone:       false,
+				timezoneTypeArray: a}
 		}
-
-		return timezoneTypeArrayBinding{typ[0], a}
+		return interfaceArrayBinding{
+			hasTimezone:       true,
+			tzType:            typ[0],
+			timezoneTypeArray: a}
 	default:
 		return a
 	}
@@ -764,16 +780,14 @@ func snowflakeArrayToString(nv *driver.NamedValue, stream bool) (snowflakeType,
 		nvValue := reflect.ValueOf(nv)
 		if nvValue.Kind() == reflect.Ptr {
 			value := reflect.Indirect(reflect.ValueOf(nv.Value))
-			if value.Kind() == reflect.Slice {
-				return interfaceSliceToString(value, stream)
-			} else if value.Kind() == reflect.Struct {
-				timeStruct, ok := value.Interface().(timezoneTypeArrayBinding)
+			if isInterfaceArrayBinding(value.Interface()) {
+				timeStruct, ok := value.Interface().(interfaceArrayBinding)
 				if ok {
 					timeInterfaceSlice := reflect.Indirect(reflect.ValueOf(timeStruct.timezoneTypeArray))
-					if timeInterfaceSlice.Kind() == reflect.Slice {
-						timeArray := reflect.Indirect(reflect.ValueOf(timeInterfaceSlice.Interface()))
-						return interfaceSliceToString(timeArray, stream, timeStruct.tzType)
+					if timeStruct.hasTimezone {
+						return interfaceSliceToString(timeInterfaceSlice, stream, timeStruct.tzType)
 					}
+					return interfaceSliceToString(timeInterfaceSlice, stream)
 				}
 			}
 		}


Test patch:
diff --git a/bindings_test.go b/bindings_test.go
index b8bc1da..0ad7d5a 100644
--- a/bindings_test.go
+++ b/bindings_test.go
@@ -198,6 +198,100 @@ func TestBindingTimestampTZ(t *testing.T) {
 	})
 }
 
+// SNOW-755844: Test the use of a pointer *time.Time type in user-defined structures to perform updates/inserts
+func TestBindingTimePtrInStruct(t *testing.T) {
+	runTests(t, dsn, func(dbt *DBTest) {
+		type timePtrStruct struct {
+			id      *int
+			timeVal *time.Time
+		}
+		var expectedID int = 1
+		var expectedTime time.Time = time.Now()
+		var testStruct timePtrStruct = timePtrStruct{id: &expectedID, timeVal: &expectedTime}
+		dbt.mustExec("CREATE OR REPLACE TABLE timeStructTest (id int, tz timestamp_tz)")
+
+		runInsertQuery := false
+		for i := 0; i < 2; i++ {
+			if !runInsertQuery {
+				_, err := dbt.db.Exec("INSERT INTO timeStructTest(id,tz) VALUES(?, ?)", testStruct.id, testStruct.timeVal)
+				if err != nil {
+					dbt.Fatal(err.Error())
+				}
+				runInsertQuery = true
+			} else {
+				// Update row with a new time value
+				expectedTime = time.Now().Add(1)
+				testStruct.timeVal = &expectedTime
+				_, err := dbt.db.Exec("UPDATE timeStructTest SET tz = ? where id = ?", testStruct.timeVal, testStruct.id)
+				if err != nil {
+					dbt.Fatal(err.Error())
+				}
+			}
+
+			rows := dbt.mustQuery("SELECT tz FROM timeStructTest WHERE id=?", &expectedID)
+			defer rows.Close()
+			var v time.Time
+			if rows.Next() {
+				rows.Scan(&v)
+				if expectedTime.UnixNano() != v.UnixNano() {
+					dbt.Errorf("returned value didn't match. expected: %v:%v, got: %v:%v",
+						expectedTime.UnixNano(), expectedTime, v.UnixNano(), v)
+				}
+			} else {
+				dbt.Error("no data")
+			}
+		}
+		dbt.mustExec("DROP TABLE timeStructTest")
+	})
+}
+
+// SNOW-755844: Test the use of a time.Time type in user-defined structures to perform updates/inserts
+func TestBindingTimeInStruct(t *testing.T) {
+	runTests(t, dsn, func(dbt *DBTest) {
+		type timeStruct struct {
+			id      int
+			timeVal time.Time
+		}
+		var expectedID int = 1
+		var expectedTime time.Time = time.Now()
+		var testStruct timeStruct = timeStruct{id: expectedID, timeVal: expectedTime}
+		dbt.mustExec("CREATE OR REPLACE TABLE timeStructTest (id int, tz timestamp_tz)")
+
+		runInsertQuery := false
+		for i := 0; i < 2; i++ {
+			if !runInsertQuery {
+				_, err := dbt.db.Exec("INSERT INTO timeStructTest(id,tz) VALUES(?, ?)", testStruct.id, testStruct.timeVal)
+				if err != nil {
+					dbt.Fatal(err.Error())
+				}
+				runInsertQuery = true
+			} else {
+				// Update row with a new time value
+				expectedTime = time.Now().Add(1)
+				testStruct.timeVal = expectedTime
+				_, err := dbt.db.Exec("UPDATE timeStructTest SET tz = ? where id = ?", testStruct.timeVal, testStruct.id)
+				if err != nil {
+					dbt.Fatal(err.Error())
+				}
+			}
+
+			rows := dbt.mustQuery("SELECT tz FROM timeStructTest WHERE id=?", &expectedID)
+			defer rows.Close()
+			var v time.Time
+			if rows.Next() {
+				rows.Scan(&v)
+				if expectedTime.UnixNano() != v.UnixNano() {
+					dbt.Errorf("returned value didn't match. expected: %v:%v, got: %v:%v",
+						expectedTime.UnixNano(), expectedTime, v.UnixNano(), v)
+				}
+			} else {
+				dbt.Error("no data")
+			}
+		}
+		dbt.mustExec("DROP TABLE timeStructTest")
+	})
+}
+
 func TestBindingInterface(t *testing.T) {
 	runTests(t, dsn, func(dbt *DBTest) {
 		rows := dbt.mustQueryContext(
diff --git a/converter_test.go b/converter_test.go
index 24a17e3..f23368e 100644
--- a/converter_test.go
+++ b/converter_test.go
@@ -65,7 +65,6 @@ func TestGoTypeToSnowflake(t *testing.T) {
 		{in: time.Now(), tmode: timestampTzType, out: timestampTzType},
 		{in: time.Now(), tmode: timestampLtzType, out: timestampLtzType},
 		{in: []byte{1, 2, 3}, tmode: binaryType, out: binaryType},
-		{in: []int{1}, tmode: nullType, out: sliceType},
 		{in: Array([]interface{}{int64(123)}), tmode: nullType, out: sliceType},
 		{in: Array([]interface{}{float64(234.56)}), tmode: nullType, out: sliceType},
 		{in: Array([]interface{}{true}), tmode: nullType, out: sliceType},
@@ -76,6 +75,11 @@ func TestGoTypeToSnowflake(t *testing.T) {
 		{in: Array([]interface{}{time.Now()}), tmode: timestampLtzType, out: sliceType},
 		{in: Array([]interface{}{time.Now()}), tmode: dateType, out: sliceType},
 		{in: Array([]interface{}{time.Now()}), tmode: timeType, out: sliceType},
+		{in: Array([]interface{}{time.Now()}, TimestampNTZType), tmode: timestampLtzType, out: sliceType},
+		{in: Array([]interface{}{time.Now()}, TimestampLTZType), tmode: dateType, out: sliceType},
+		{in: Array([]interface{}{time.Now()}, TimestampTZType), tmode: timeType, out: sliceType},
+		{in: Array([]interface{}{time.Now()}, DateType), tmode: timestampNtzType, out: sliceType},
+		{in: Array([]interface{}{time.Now()}, TimeType), tmode: timestampTzType, out: sliceType},
 		// negative
 		{in: 123, tmode: nullType, out: unSupportedType},
 		{in: int8(12), tmode: nullType, out: unSupportedType},
@@ -84,6 +88,7 @@ func TestGoTypeToSnowflake(t *testing.T) {
 		{in: uint8(12), tmode: nullType, out: unSupportedType},
 		{in: uint64(456), tmode: nullType, out: unSupportedType},
 		{in: []byte{100}, tmode: nullType, out: unSupportedType},
+		{in: []int{1}, tmode: nullType, out: unSupportedType},
 		{in: nil, tmode: nullType, out: unSupportedType},
 	}
 	for _, test := range testcases {