sql: add proper telemetry for arrays of JSON

Arrays of JSON are not supported but the errors did not link attempted uses to issue cockroachdb#23468. This patch fixes it. Release note: None
knz · Mar 14, 2019 · c04ff44 · c04ff44
1 parent 8f0a7a6
commit c04ff44
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 22 deletions.
diff --git a/pkg/sql/coltypes/aliases.go b/pkg/sql/coltypes/aliases.go
@@ -143,8 +143,9 @@ func NewFloat(prec int64) (*TFloat, error) {
 
 // ArrayOf creates a type alias for an array of the given element type and fixed bounds.
 func ArrayOf(colType T, bounds []int32) (T, error) {
-	if !canBeInArrayColType(colType) {
-		return nil, pgerror.NewErrorf(pgerror.CodeFeatureNotSupportedError, "arrays of %s not allowed", colType)
+	if ok, issueNum := canBeInArrayColType(colType); !ok {
+		return nil, pgerror.UnimplementedWithIssueDetailErrorf(issueNum,
+			colType.String(), "arrays of %s not allowed", colType)
 	}
 	return &TArray{ParamType: colType, Bounds: bounds}, nil
 }

diff --git a/pkg/sql/coltypes/arrays.go b/pkg/sql/coltypes/arrays.go
@@ -50,12 +50,12 @@ func (node *TArray) Format(buf *bytes.Buffer, f lex.EncodeFlags) {
 
 // canBeInArrayColType returns true if the given T is a valid
 // element type for an array column type.
-func canBeInArrayColType(t T) bool {
+func canBeInArrayColType(t T) (valid bool, issueNum int) {
 	switch t.(type) {
 	case *TJSON:
-		return false
+		return false, 23468
 	default:
-		return true
+		return true, 0
 	}
 }
 

diff --git a/pkg/sql/logictest/testdata/logic_test/json b/pkg/sql/logictest/testdata/logic_test/json
@@ -86,13 +86,13 @@ SELECT NULL::JSON
 ----
 NULL
 
-statement error arrays of jsonb not allowed
+statement error arrays of jsonb not allowed.*\nHINT:.*23468
 SELECT ARRAY['"hello"'::JSON]
 
-statement error arrays of JSONB not allowed
+statement error arrays of JSONB not allowed.*\nHINT:.*23468
 SELECT '{}'::JSONB[]
 
-statement error arrays of JSONB not allowed
+statement error arrays of JSONB not allowed.*\nHINT:.*23468
 CREATE TABLE x (y JSONB[])
 
 statement ok

diff --git a/pkg/sql/opt/optbuilder/scalar.go b/pkg/sql/opt/optbuilder/scalar.go
@@ -31,8 +31,8 @@ import (
 )
 
 func checkArrayElementType(t types.T) error {
-	if !types.IsValidArrayElementType(t) {
-		return pgerror.NewErrorf(pgerror.CodeFeatureNotSupportedError,
+	if ok, issueNum := types.IsValidArrayElementType(t); !ok {
+		return pgerror.UnimplementedWithIssueDetailErrorf(issueNum, t.String(),
 			"arrays of %s not allowed", t)
 	}
 	return nil
@@ -129,8 +129,8 @@ func (b *Builder) buildScalar(
 			panic(builderError{fmt.Errorf("can't execute a correlated ARRAY(...) over %s", typ)})
 		}
 
-		if !types.IsValidArrayElementType(typ) {
-			panic(builderError{fmt.Errorf("arrays of %s not allowed", typ)})
+		if err := checkArrayElementType(typ); err != nil {
+			panic(builderError{err})
 		}
 
 		// Perform correctness checks on the outer cols, update colRefs and

diff --git a/pkg/sql/opt/optbuilder/testdata/scalar b/pkg/sql/opt/optbuilder/testdata/scalar
@@ -756,12 +756,12 @@ concat [type=oid[]]
 build-scalar
 ARRAY['"foo"'::jsonb]
 ----
-error: arrays of jsonb not allowed
+error: unimplemented: arrays of jsonb not allowed
 
 build-scalar
 ARRAY['"foo"'::json]
 ----
-error: arrays of jsonb not allowed
+error: unimplemented: arrays of jsonb not allowed
 
 opt
 SELECT -((-9223372036854775808):::int)
@@ -907,7 +907,7 @@ project
 build
 SELECT ARRAY(VALUES ('{}'::JSONB))
 ----
-error: arrays of jsonb not allowed
+error (0A000): unimplemented: arrays of jsonb not allowed
 
 build
 SELECT ARRAY(SELECT 1, 2)

diff --git a/pkg/sql/sem/builtins/builtins.go b/pkg/sql/sem/builtins/builtins.go
@@ -3558,7 +3558,7 @@ var jsonArrayLengthImpl = tree.Overload{
 func arrayBuiltin(impl func(types.T) tree.Overload) builtinDefinition {
 	overloads := make([]tree.Overload, 0, len(types.AnyNonArray))
 	for _, typ := range types.AnyNonArray {
-		if types.IsValidArrayElementType(typ) {
+		if ok, _ := types.IsValidArrayElementType(typ); ok {
 			overloads = append(overloads, impl(typ))
 		}
 	}

diff --git a/pkg/sql/sem/tree/eval.go b/pkg/sql/sem/tree/eval.go
@@ -3885,8 +3885,9 @@ func arrayOfType(typ types.T) (*DArray, error) {
 	if !ok {
 		return nil, pgerror.NewAssertionErrorf("array node type (%v) is not types.TArray", typ)
 	}
-	if !types.IsValidArrayElementType(arrayTyp.Typ) {
-		return nil, pgerror.NewErrorf(pgerror.CodeFeatureNotSupportedError, "arrays of %s not allowed", arrayTyp.Typ)
+	if ok, issueNum := types.IsValidArrayElementType(arrayTyp.Typ); !ok {
+		return nil, pgerror.UnimplementedWithIssueDetailErrorf(issueNum, arrayTyp.Typ.String(),
+			"arrays of %s not allowed", arrayTyp.Typ)
 	}
 	return NewDArray(arrayTyp.Typ), nil
 }

diff --git a/pkg/sql/sem/types/invariants_test.go b/pkg/sql/sem/types/invariants_test.go
@@ -22,7 +22,7 @@ import (
 
 func TestArraysHaveOids(t *testing.T) {
 	for _, typ := range AnyNonArray {
-		if !IsValidArrayElementType(typ) {
+		if ok, _ := IsValidArrayElementType(typ); !ok {
 			continue
 		}
 

diff --git a/pkg/sql/sem/types/types.go b/pkg/sql/sem/types/types.go
@@ -525,12 +525,12 @@ func IsStringType(t T) bool {
 
 // IsValidArrayElementType returns true if the T
 // can be used in TArray.
-func IsValidArrayElementType(t T) bool {
+func IsValidArrayElementType(t T) (valid bool, issueNum int) {
 	switch t {
 	case JSON:
-		return false
+		return false, 23468
 	default:
-		return true
+		return true, 0
 	}
 }