From 5377c7af8a1837cd6ea9b30e45b91e49a8e2b8a1 Mon Sep 17 00:00:00 2001
From: Tomoaki Fujii <38714779+tomo241@users.noreply.github.com>
Date: Tue, 10 Jan 2023 14:43:53 +0900
Subject: [PATCH] Fix a bug in ParseSpannerType when parsing an ARRAY data type
 and fix related test code.

With this schema, `gcsb load` causes a panic error, e.g. "panic: unknown spanner type 'STRING(256)'"
CREATE TABLE item_list(
  item_list_id STRING(256),
  item_list1 ARRAY<STRING(256)>,
  item_list2 ARRAY<STRING(MAX)>
) PRIMARY KEY(item_list_id)

This is because the current code doesn't take account of the data length of the inner STRING element. Actually, using STRING data type without specifying the data length is not allowed in DDL and you can't create a table with a data type of `STRING` or ARRAY<STRING>.

This change is to fix the code and related test code.
---
 pkg/schema/column_test.go | 24 ++++++++++++------------
 pkg/schema/type.go        | 10 +++++-----
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/pkg/schema/column_test.go b/pkg/schema/column_test.go
index 874cf9b..0eb123c 100644
--- a/pkg/schema/column_test.go
+++ b/pkg/schema/column_test.go
@@ -34,16 +34,6 @@ func TestColumn(t *testing.T) {
 				So(x.Base, ShouldEqual, spansql.Bool)
 			})
 
-			Convey("String", func() {
-				c := NewColumn()
-				c.SetSpannerType("STRING")
-
-				x := c.Type()
-				So(x, ShouldNotBeNil)
-				So(x.Array, ShouldBeFalse)
-				So(x.Base, ShouldEqual, spansql.String)
-			})
-
 			Convey("String(1024)", func() {
 				c := NewColumn()
 				c.SetSpannerType("STRING(1024)")
@@ -116,9 +106,19 @@ func TestColumn(t *testing.T) {
 				So(x.Base, ShouldEqual, spansql.Date)
 			})
 
-			Convey("ARRAY<STRING>", func() {
+			Convey("ARRAY<STRING(1024)>", func() {
+				c := NewColumn()
+				c.SetSpannerType("ARRAY<STRING(1024)>")
+
+				x := c.Type()
+				So(x, ShouldNotBeNil)
+				So(x.Array, ShouldBeTrue)
+				So(x.Base, ShouldEqual, spansql.String)
+			})
+
+			Convey("ARRAY<STRING(MAX)>", func() {
 				c := NewColumn()
-				c.SetSpannerType("ARRAY<STRING>")
+				c.SetSpannerType("ARRAY<STRING(MAX)>")
 
 				x := c.Type()
 				So(x, ShouldNotBeNil)
diff --git a/pkg/schema/type.go b/pkg/schema/type.go
index 8dc1126..5e0932f 100644
--- a/pkg/schema/type.go
+++ b/pkg/schema/type.go
@@ -36,6 +36,11 @@ func ParseSpannerType(spannerType string) spansql.Type {
 
 	dt := spannerType
 
+	if strings.HasPrefix(dt, "ARRAY<") {
+		ret.Array = true
+		dt = strings.TrimSuffix(strings.TrimPrefix(dt, "ARRAY<"), ">")
+	}
+
 	// separate type and length from dt with length such as STRING(32) or BYTES(256)
 	m := lengthRegexp.FindStringSubmatchIndex(dt)
 	if m != nil {
@@ -54,11 +59,6 @@ func ParseSpannerType(spannerType string) spansql.Type {
 		dt = dt[:m[0]] + dt[m[1]:]
 	}
 
-	if strings.HasPrefix(dt, "ARRAY<") {
-		ret.Array = true
-		dt = strings.TrimSuffix(strings.TrimPrefix(dt, "ARRAY<"), ">")
-	}
-
 	ret.Base = parseType(dt)
 
 	// Clip length for certain types