Repository: googleapis/google-cloud-go
Pull request #4225 (2021-06-07 01:58:21)

feat(bigquery): support nullable params and geography params
Details:

* Support the passing of Null<type> values for query parameters.

* Add support for GEOGGRAPHY param type, which was added after the initial
      GEOGRAPHY type support launched.

Fixes: https://github.com/googleapis/google-cloud-go/issues/4146


shollyman: Revamping this currently, value propagation had some issues
shollyman: > Looks good to me, but would prefer two smaller PRs in the future. 😄

Reasonable request.  :) 

My rationale was that the delta from geography was small, and touched all the same areas so I lumped them together this time.

Patch:
diff --git a/bigquery/params.go b/bigquery/params.go
index 68bb3fa01a..1171d50cbb 100644
--- a/bigquery/params.go
+++ b/bigquery/params.go
@@ -76,6 +76,7 @@ var (
 	timestampParamType  = &bq.QueryParameterType{Type: "TIMESTAMP"}
 	numericParamType    = &bq.QueryParameterType{Type: "NUMERIC"}
 	bigNumericParamType = &bq.QueryParameterType{Type: "BIGNUMERIC"}
+	geographyParamType  = &bq.QueryParameterType{Type: "GEOGRAPHY"}
 )
 
 var (
@@ -108,9 +109,8 @@ type QueryParameter struct {
 	// Arrays and slices of the above.
 	// Structs of the above. Only the exported fields are used.
 	//
-	// BigQuery does not support params of type GEOGRAPHY.  For users wishing
-	// to parameterize Geography values, use string parameters and cast in the
-	// SQL query, e.g. `SELECT ST_GeogFromText(@string_param) as geo`
+	// For scalar values, you can supply the Null types within this library
+	// to send the appropriate NULL values (e.g. NullInt64, NullString, etc).
 	//
 	// When a QueryParameter is returned inside a QueryConfig from a call to
 	// Job.Config:
@@ -118,6 +118,10 @@ type QueryParameter struct {
 	// Floating-point values are of type float64.
 	// Arrays are of type []interface{}, regardless of the array element type.
 	// Structs are of type map[string]interface{}.
+	//
+	// When valid (non-null) Null types are sent, they come back as the Go types indicated
+	// above.  Null strings will report in query statistics as a valid empty
+	// string.
 	Value interface{}
 }
 
@@ -132,7 +136,7 @@ func (p QueryParameter) toBQ() (*bq.QueryParameter, error) {
 	}
 	return &bq.QueryParameter{
 		Name:           p.Name,
-		ParameterValue: &pv,
+		ParameterValue: pv,
 		ParameterType:  pt,
 	}, nil
 }
@@ -142,16 +146,26 @@ func paramType(t reflect.Type) (*bq.QueryParameterType, error) {
 		return nil, errors.New("bigquery: nil parameter")
 	}
 	switch t {
-	case typeOfDate:
+	case typeOfDate, typeOfNullDate:
 		return dateParamType, nil
-	case typeOfTime:
+	case typeOfTime, typeOfNullTime:
 		return timeParamType, nil
-	case typeOfDateTime:
+	case typeOfDateTime, typeOfNullDateTime:
 		return dateTimeParamType, nil
-	case typeOfGoTime:
+	case typeOfGoTime, typeOfNullTimestamp:
 		return timestampParamType, nil
 	case typeOfRat:
 		return numericParamType, nil
+	case typeOfNullBool:
+		return boolParamType, nil
+	case typeOfNullFloat64:
+		return float64ParamType, nil
+	case typeOfNullInt64:
+		return int64ParamType, nil
+	case typeOfNullString:
+		return stringParamType, nil
+	case typeOfNullGeography:
+		return geographyParamType, nil
 	}
 	switch t.Kind() {
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint8, reflect.Uint16, reflect.Uint32:
@@ -207,17 +221,64 @@ func paramType(t reflect.Type) (*bq.QueryParameterType, error) {
 	return nil, fmt.Errorf("bigquery: Go type %s cannot be represented as a parameter type", t)
 }
 
-func paramValue(v reflect.Value) (bq.QueryParameterValue, error) {
-	var res bq.QueryParameterValue
+func paramValue(v reflect.Value) (*bq.QueryParameterValue, error) {
+	res := &bq.QueryParameterValue{}
 	if !v.IsValid() {
 		return res, errors.New("bigquery: nil parameter")
 	}
 	t := v.Type()
 	switch t {
+
+	// Handle all the custom null types as a group first, as they all have the same logic when invalid.
+	case typeOfNullInt64,
+		typeOfNullString,
+		typeOfNullGeography,
+		typeOfNullFloat64,
+		typeOfNullBool,
+		typeOfNullTimestamp,
+		typeOfNullDate,
+		typeOfNullTime,
+		typeOfNullDateTime:
+		// Shared:  If the Null type isn't valid, we have no value to send.
+		// However, the backend requires us to send the QueryParameterValue with
+		// the fields empty.
+		if !v.FieldByName("Valid").Bool() {
+			// Ensure we don't send a default value by using NullFields in the JSON
+			// serialization.
+			res.NullFields = append(res.NullFields, "Value")
+			return res, nil
+		}
+		// For cases where the Null type is valid, populate the scalar value as needed.
+		switch t {
+		case typeOfNullInt64:
+			res.Value = fmt.Sprint(v.FieldByName("Int64").Interface())
+		case typeOfNullString:
+			res.Value = fmt.Sprint(v.FieldByName("StringVal").Interface())
+		case typeOfNullGeography:
+			res.Value = fmt.Sprint(v.FieldByName("GeographyVal").Interface())
+		case typeOfNullFloat64:
+			res.Value = fmt.Sprint(v.FieldByName("Float64").Interface())
+		case typeOfNullBool:
+			res.Value = fmt.Sprint(v.FieldByName("Bool").Interface())
+		case typeOfNullTimestamp:
+			res.Value = v.FieldByName("Timestamp").Interface().(time.Time).Format(timestampFormat)
+		case typeOfNullDate:
+			res.Value = v.FieldByName("Date").Interface().(civil.Date).String()
+		case typeOfNullTime:
+			res.Value = CivilTimeString(v.FieldByName("Time").Interface().(civil.Time))
+		case typeOfNullDateTime:
+			res.Value = CivilDateTimeString(v.FieldByName("DateTime").Interface().(civil.DateTime))
+		}
+		// We expect to produce a value in all these cases, so force send if the result is the empty
+		// string.
+		if res.Value == "" {
+			res.ForceSendFields = append(res.ForceSendFields, "Value")
+		}
+		return res, nil
+
 	case typeOfDate:
 		res.Value = v.Interface().(civil.Date).String()
 		return res, nil
-
 	case typeOfTime:
 		// civil.Time has nanosecond resolution, but BigQuery TIME only microsecond.
 		// (If we send nanoseconds, then when we try to read the result we get "query job
@@ -253,11 +314,11 @@ func paramValue(v reflect.Value) (bq.QueryParameterValue, error) {
 		for i := 0; i < v.Len(); i++ {
 			val, err := paramValue(v.Index(i))
 			if err != nil {
-				return bq.QueryParameterValue{}, err
+				return nil, err
 			}
-			vals = append(vals, &val)
+			vals = append(vals, val)
 		}
-		return bq.QueryParameterValue{ArrayValues: vals}, nil
+		return &bq.QueryParameterValue{ArrayValues: vals}, nil
 
 	case reflect.Ptr:
 		if t.Elem().Kind() != reflect.Struct {
@@ -274,16 +335,16 @@ func paramValue(v reflect.Value) (bq.QueryParameterValue, error) {
 	case reflect.Struct:
 		fields, err := fieldCache.Fields(t)
 		if err != nil {
-			return bq.QueryParameterValue{}, err
+			return nil, err
 		}
 		res.StructValues = map[string]bq.QueryParameterValue{}
 		for _, f := range fields {
 			fv := v.FieldByIndex(f.Index)
 			fp, err := paramValue(fv)
 			if err != nil {
-				return bq.QueryParameterValue{}, err
+				return nil, err
 			}
-			res.StructValues[f.Name] = fp
+			res.StructValues[f.Name] = *fp
 		}
 		return res, nil
 	}
@@ -317,10 +378,12 @@ var paramTypeToFieldType = map[string]FieldType{
 	timeParamType.Type:       TimeFieldType,
 	numericParamType.Type:    NumericFieldType,
 	bigNumericParamType.Type: BigNumericFieldType,
+	geographyParamType.Type:  GeographyFieldType,
 }
 
 // Convert a parameter value from the service to a Go value. This is similar to, but
-// not quite the same as, converting data values.
+// not quite the same as, converting data values.  Namely, rather than returning nil
+// directly, we wrap them in the appropriate Null types (NullInt64, etc).
 func convertParamValue(qval *bq.QueryParameterValue, qtype *bq.QueryParameterType) (interface{}, error) {
 	switch qtype.Type {
 	case "ARRAY":
@@ -334,14 +397,53 @@ func convertParamValue(qval *bq.QueryParameterValue, qtype *bq.QueryParameterTyp
 		}
 		return convertParamStruct(qval.StructValues, qtype.StructTypes)
 	case "TIMESTAMP":
+		if isNullScalar(qval) {
+			return NullTimestamp{Valid: false}, nil
+		}
 		return time.Parse(timestampFormat, qval.Value)
 	case "DATETIME":
+		if isNullScalar(qval) {
+			return NullDateTime{Valid: false}, nil
+		}
 		return parseCivilDateTime(qval.Value)
 	default:
+		if isNullScalar(qval) {
+			switch qtype.Type {
+			case "INT64":
+				return NullInt64{Valid: false}, nil
+			case "STRING":
+				return NullString{Valid: false}, nil
+			case "FLOAT64":
+				return NullFloat64{Valid: false}, nil
+			case "BOOL":
+				return NullBool{Valid: false}, nil
+			case "DATE":
+				return NullDate{Valid: false}, nil
+			case "TIME":
+				return NullTime{Valid: false}, nil
+			case "GEOGRAPHY":
+				return NullGeography{Valid: false}, nil
+			}
+
+		}
 		return convertBasicType(qval.Value, paramTypeToFieldType[qtype.Type])
 	}
 }
 
+// isNullScalar determines if the input is meant to represent a null scalar
+// value.
+func isNullScalar(qval *bq.QueryParameterValue) bool {
+	if qval == nil {
+		return true
+	}
+	for _, v := range qval.NullFields {
+		if v == "Value" {
+			return true
+		}
+	}
+	return false
+}
+
 // convertParamArray converts a query parameter array value to a Go value. It
 // always returns a []interface{}.
 func convertParamArray(elVals []*bq.QueryParameterValue, elType *bq.QueryParameterType) ([]interface{}, error) {


Test patch:
diff --git a/bigquery/params_test.go b/bigquery/params_test.go
index 57a3b8d38f..d9233cad6e 100644
--- a/bigquery/params_test.go
+++ b/bigquery/params_test.go
@@ -30,27 +30,94 @@ import (
 )
 
 var scalarTests = []struct {
-	val      interface{}            // The Go value
-	wantVal  string                 // paramValue's desired output
+	val      interface{}            // input value sent as query param
+	wantNil  bool                   // whether the value returned in a query field should be nil.
+	wantVal  string                 // the string form of the scalar value in QueryParameterValue.
 	wantType *bq.QueryParameterType // paramType's desired output
+	wantStat interface{}            // val when roundtripped and represented as part of job statistics.
 }{
-	{int64(0), "0", int64ParamType},
-	{3.14, "3.14", float64ParamType},
-	{3.14159e-87, "3.14159e-87", float64ParamType},
-	{true, "true", boolParamType},
-	{"string", "string", stringParamType},
-	{"\u65e5\u672c\u8a9e\n", "\u65e5\u672c\u8a9e\n", stringParamType},
-	{math.NaN(), "NaN", float64ParamType},
-	{[]byte("foo"), "Zm9v", bytesParamType}, // base64 encoding of "foo"
+	{int64(0), false, "0", int64ParamType, int64(0)},
+	{NullInt64{Int64: 3, Valid: true}, false, "3", int64ParamType, int64(3)},
+	{NullInt64{Valid: false}, true, "", int64ParamType, NullInt64{Valid: false}},
+	{3.14, false, "3.14", float64ParamType, 3.14},
+	{3.14159e-87, false, "3.14159e-87", float64ParamType, 3.14159e-87},
+	{NullFloat64{Float64: 3.14, Valid: true}, false, "3.14", float64ParamType, 3.14},
+	{NullFloat64{Valid: false}, true, "", float64ParamType, NullFloat64{Valid: false}},
+	{math.NaN(), false, "NaN", float64ParamType, math.NaN()},
+	{true, false, "true", boolParamType, true},
+	{NullBool{Bool: true, Valid: true}, false, "true", boolParamType, true},
+	{NullBool{Valid: false}, true, "", boolParamType, NullBool{Valid: false}},
+	{"string", false, "string", stringParamType, "string"},
+	{"\u65e5\u672c\u8a9e\n", false, "\u65e5\u672c\u8a9e\n", stringParamType, "\u65e5\u672c\u8a9e\n"},
+	{NullString{StringVal: "string2", Valid: true}, false, "string2", stringParamType, "string2"},
+	{NullString{Valid: false}, true, "", stringParamType, NullString{Valid: false}},
+	{[]byte("foo"), false, "Zm9v", bytesParamType, []byte("foo")}, // base64 encoding of "foo"
 	{time.Date(2016, 3, 20, 4, 22, 9, 5000, time.FixedZone("neg1-2", -3720)),
+		false,
 		"2016-03-20 04:22:09.000005-01:02",
-		timestampParamType},
-	{civil.Date{Year: 2016, Month: 3, Day: 20}, "2016-03-20", dateParamType},
-	{civil.Time{Hour: 4, Minute: 5, Second: 6, Nanosecond: 789000000}, "04:05:06.789000", timeParamType},
+		timestampParamType,
+		time.Date(2016, 3, 20, 4, 22, 9, 5000, time.FixedZone("neg1-2", -3720))},
+	{NullTimestamp{Timestamp: time.Date(2016, 3, 22, 4, 22, 9, 5000, time.FixedZone("neg1-2", -3720)), Valid: true},
+		false,
+		"2016-03-22 04:22:09.000005-01:02",
+		timestampParamType,
+		time.Date(2016, 3, 22, 4, 22, 9, 5000, time.FixedZone("neg1-2", -3720))},
+	{NullTimestamp{Valid: false},
+		true,
+		"",
+		timestampParamType,
+		NullTimestamp{Valid: false}},
+	{civil.Date{Year: 2016, Month: 3, Day: 20},
+		false,
+		"2016-03-20",
+		dateParamType,
+		civil.Date{Year: 2016, Month: 3, Day: 20}},
+	{NullDate{
+		Date: civil.Date{Year: 2016, Month: 3, Day: 24}, Valid: true},
+		false,
+		"2016-03-24",
+		dateParamType,
+		civil.Date{Year: 2016, Month: 3, Day: 24}},
+	{NullDate{Valid: false},
+		true,
+		"",
+		dateParamType,
+		NullDate{Valid: false}},
+	{civil.Time{Hour: 4, Minute: 5, Second: 6, Nanosecond: 789000000},
+		false,
+		"04:05:06.789000",
+		timeParamType,
+		civil.Time{Hour: 4, Minute: 5, Second: 6, Nanosecond: 789000000}},
+	{NullTime{
+		Time: civil.Time{Hour: 6, Minute: 7, Second: 8, Nanosecond: 789000000}, Valid: true},
+		false,
+		"06:07:08.789000",
+		timeParamType,
+		civil.Time{Hour: 6, Minute: 7, Second: 8, Nanosecond: 789000000}},
+	{NullTime{Valid: false},
+		true,
+		"",
+		timeParamType,
+		NullTime{Valid: false}},
 	{civil.DateTime{Date: civil.Date{Year: 2016, Month: 3, Day: 20}, Time: civil.Time{Hour: 4, Minute: 5, Second: 6, Nanosecond: 789000000}},
+		false,
 		"2016-03-20 04:05:06.789000",
-		dateTimeParamType},
-	{big.NewRat(12345, 1000), "12.345000000", numericParamType},
+		dateTimeParamType,
+		civil.DateTime{Date: civil.Date{Year: 2016, Month: 3, Day: 20}, Time: civil.Time{Hour: 4, Minute: 5, Second: 6, Nanosecond: 789000000}}},
+	{NullDateTime{
+		DateTime: civil.DateTime{Date: civil.Date{Year: 2016, Month: 3, Day: 21}, Time: civil.Time{Hour: 4, Minute: 5, Second: 6, Nanosecond: 789000000}}, Valid: true},
+		false,
+		"2016-03-21 04:05:06.789000",
+		dateTimeParamType,
+		civil.DateTime{Date: civil.Date{Year: 2016, Month: 3, Day: 21}, Time: civil.Time{Hour: 4, Minute: 5, Second: 6, Nanosecond: 789000000}}},
+	{NullDateTime{Valid: false},
+		true,
+		"",
+		dateTimeParamType,
+		NullDateTime{Valid: false}},
+	{big.NewRat(12345, 1000), false, "12.345000000", numericParamType, big.NewRat(12345, 1000)},
+	{NullGeography{GeographyVal: "POINT(-122.335503 47.625536)", Valid: true}, false, "POINT(-122.335503 47.625536)", geographyParamType, "POINT(-122.335503 47.625536)"},
+	{NullGeography{Valid: false}, true, "", geographyParamType, NullGeography{Valid: false}},
 }
 
 type (
@@ -109,31 +176,41 @@ func sval(s string) bq.QueryParameterValue {
 }
 
 func TestParamValueScalar(t *testing.T) {
+
+	nilValue := &bq.QueryParameterValue{
+		NullFields: []string{"Value"},
+	}
+
 	for _, test := range scalarTests {
 		got, err := paramValue(reflect.ValueOf(test.val))
 		if err != nil {
-			t.Errorf("%v: got %v, want nil", test.val, err)
-			continue
+			t.Errorf("%v: got err %v", test.val, err)
 		}
-		want := sval(test.wantVal)
-		if !testutil.Equal(got, want) {
-			t.Errorf("%v:\ngot  %+v\nwant %+v", test.val, got, want)
+		if test.wantNil {
+			if !testutil.Equal(got, nilValue) {
+				t.Errorf("%#v: wanted empty QueryParameterValue, got %v", test.val, got)
+			}
+		} else {
+			want := sval(test.wantVal)
+			if !testutil.Equal(got, &want) {
+				t.Errorf("%#v:\ngot  %+v\nwant %+v", test.val, got, want)
+			}
 		}
 	}
 }
 
 func TestParamValueArray(t *testing.T) {
-	qpv := bq.QueryParameterValue{ArrayValues: []*bq.QueryParameterValue{
+	qpv := &bq.QueryParameterValue{ArrayValues: []*bq.QueryParameterValue{
 		{Value: "1"},
 		{Value: "2"},
 	},
 	}
 	for _, test := range []struct {
 		val  interface{}
-		want bq.QueryParameterValue
+		want *bq.QueryParameterValue
 	}{
-		{[]int(nil), bq.QueryParameterValue{}},
-		{[]int{}, bq.QueryParameterValue{}},
+		{[]int(nil), &bq.QueryParameterValue{}},
+		{[]int{}, &bq.QueryParameterValue{}},
 		{[]int{1, 2}, qpv},
 		{[2]int{1, 2}, qpv},
 	} {
@@ -152,8 +229,8 @@ func TestParamValueStruct(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !testutil.Equal(got, s1ParamValue) {
-		t.Errorf("got  %+v\nwant %+v", got, s1ParamValue)
+	if !testutil.Equal(got, &s1ParamValue) {
+		t.Errorf("got  %+v\nwant %+v", got, &s1ParamValue)
 	}
 }
 
@@ -220,13 +297,14 @@ func TestConvertParamValue(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		got, err := convertParamValue(&pval, ptype)
+		got, err := convertParamValue(pval, ptype)
 		if err != nil {
 			t.Fatalf("convertParamValue(%+v, %+v): %v", pval, ptype, err)
 		}
-		if !testutil.Equal(got, test.val) {
-			t.Errorf("%#v: got %#v", test.val, got)
+		if !testutil.Equal(got, test.wantStat) {
+			t.Errorf("%#v: wanted stat as %#v, got %#v", test.val, test.wantStat, got)
 		}
+
 	}
 	// Arrays.
 	for _, test := range []struct {
@@ -270,13 +348,21 @@ func TestIntegration_ScalarParam(t *testing.T) {
 	for _, test := range scalarTests {
 		gotData, gotParam, err := paramRoundTrip(c, test.val)
 		if err != nil {
-			t.Fatal(err)
+			t.Errorf("input %#v errored: %v", test.val, err)
 		}
-		if !testutil.Equal(gotData, test.val, roundToMicros) {
-			t.Errorf("\ngot  %#v (%T)\nwant %#v (%T)", gotData, gotData, test.val, test.val)
+		// first, check the returned query value
+		if test.wantNil {
+			if gotData != nil {
+				t.Errorf("data value %#v expected nil, got %#v", test.val, gotData)
+			}
+		} else {
+			if !testutil.Equal(gotData, test.wantStat, roundToMicros) {
+				t.Errorf("\ngot data value %#v (%T)\nwant %#v (%T)", gotData, gotData, test.wantStat, test.wantStat)
+			}
 		}
-		if !testutil.Equal(gotParam, test.val, roundToMicros) {
-			t.Errorf("\ngot  %#v (%T)\nwant %#v (%T)", gotParam, gotParam, test.val, test.val)
+		// then, check the stat value
+		if !testutil.Equal(gotParam, test.wantStat, roundToMicros) {
+			t.Errorf("\ngot param stat %#v (%T)\nwant %#v (%T)", gotParam, gotParam, test.wantStat, test.wantStat)
 		}
 	}
 }