Merge pull request #10309 from knz/fix-concat-ws

sql: fix a panic in concat_ws() and support a NULL separator.

pkg/sql/parser/builtins.go

@@ -53,6 +53,7 @@ var (
 	errSqrtOfNegNumber  = errors.New("cannot take square root of a negative number")
 	errLogOfNegNumber   = errors.New("cannot take logarithm of a negative number")
 	errLogOfZero        = errors.New("cannot take logarithm of zero")
+	errInsufficientArgs = errors.New("unknown signature for CONCAT_WS: CONCAT_WS()")
 )
 
 // FunctionClass specifies the class of the builtin function.
@@ -214,19 +215,26 @@ var Builtins = map[string][]Builtin{
 			Types:      VariadicType{TypeString},
 			ReturnType: TypeString,
 			fn: func(_ *EvalContext, args DTuple) (Datum, error) {
-				dstr, ok := args[0].(*DString)
-				if !ok {
-					return DNull, fmt.Errorf("unknown signature for concat_ws: concat_ws(%s, ...)", args[0])
+				if len(args) == 0 {
+					return DNull, errInsufficientArgs
 				}
-				sep := string(*dstr)
-				var ss []string
+				if args[0] == DNull {
+					return DNull, nil
+				}
+				sep := string(*args[0].(*DString))
+				var buf bytes.Buffer
+				prefix := ""
 				for _, d := range args[1:] {
 					if d == DNull {
 						continue
 					}
-					ss = append(ss, string(*d.(*DString)))
+					// Note: we can't use the range index here because that
+					// would break when the 2nd argument is NULL.
+					buf.WriteString(prefix)
+					prefix = sep
+					buf.WriteString(string(*d.(*DString)))
 				}
-				return NewDString(strings.Join(ss, sep)), nil
+				return NewDString(buf.String()), nil
 			},
 		},
 	},

pkg/sql/testdata/builtin_function

@@ -161,6 +161,14 @@ SELECT SUBSTRING('f(oabaroob' from '\(o(.)b' for '+')
 query error unknown signature for SUBSTRING: SUBSTRING()
 SELECT SUBSTRING()
 
+query error unknown signature for CONCAT_WS: CONCAT_WS()
+SELECT CONCAT_WS()
+
+query T
+SELECT CONCAT_WS(NULL::STRING, 'a', 'b')
+----
+NULL
+
 query T
 SELECT CONCAT_WS(',', 'abcde', NULL)
 ----