Repository: googleapis/google-cloud-go
Pull request #5017 (2021-10-21 20:55:47)

feat(bigquery): expose identifiers using a variety of formats
This PR adds an Identifier() method to common BQ resources so
that users can get an identifier that is formatted appropriately
for their use case (legacy sql, standard sql, referencing in storage
API, etc).

Existing instances of FullyQualifiedName() have been migrated to the
new method.

Fixes: https://github.com/googleapis/google-cloud-go/issues/1955


Patch:
diff --git a/bigquery/dataset.go b/bigquery/dataset.go
index 91b649eed8..7f9115a5c2 100644
--- a/bigquery/dataset.go
+++ b/bigquery/dataset.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"cloud.google.com/go/internal/optional"
@@ -88,6 +89,25 @@ func (c *Client) DatasetInProject(projectID, datasetID string) *Dataset {
 	}
 }
 
+// Identifier returns the ID of the dataset in the requested format.
+//
+// For Standard SQL format, the identifier will be quoted if the
+// ProjectID contains dash (-) characters.
+func (d *Dataset) Identifier(f IdentifierFormat) (string, error) {
+	switch f {
+	case LegacySQLID:
+		return fmt.Sprintf("%s:%s", d.ProjectID, d.DatasetID), nil
+	case StandardSQLID:
+		// Quote project identifiers if they have a dash character.
+		if strings.Contains(d.ProjectID, "-") {
+			return fmt.Sprintf("`%s`.%s", d.ProjectID, d.DatasetID), nil
+		}
+		return fmt.Sprintf("%s.%s", d.ProjectID, d.DatasetID), nil
+	default:
+		return "", ErrUnknownIdentifierFormat
+	}
+}
+
 // Create creates a dataset in the BigQuery service. An error will be returned if the
 // dataset already exists. Pass in a DatasetMetadata value to configure the dataset.
 func (d *Dataset) Create(ctx context.Context, md *DatasetMetadata) (err error) {
diff --git a/bigquery/model.go b/bigquery/model.go
index 363133bf66..56260efe9c 100644
--- a/bigquery/model.go
+++ b/bigquery/model.go
@@ -17,6 +17,7 @@ package bigquery
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"cloud.google.com/go/internal/optional"
@@ -41,9 +42,31 @@ type Model struct {
 	c *Client
 }
 
+// Identifier returns the ID of the model in the requested format.
+//
+// For Standard SQL format, the identifier will be quoted if the
+// ProjectID contains dash (-) characters.
+func (m *Model) Identifier(f IdentifierFormat) (string, error) {
+	switch f {
+	case LegacySQLID:
+		return fmt.Sprintf("%s:%s.%s", m.ProjectID, m.DatasetID, m.ModelID), nil
+	case StandardSQLID:
+		// Per https://cloud.google.com/bigquery-ml/docs/reference/standard-sql/bigqueryml-syntax-create#model_name
+		// we quote the entire identifier.
+		out := fmt.Sprintf("%s.%s.%s", m.ProjectID, m.DatasetID, m.ModelID)
+		if strings.Contains(out, "-") {
+			out = fmt.Sprintf("`%s`", out)
+		}
+		return out, nil
+	default:
+		return "", ErrUnknownIdentifierFormat
+	}
+}
+
 // FullyQualifiedName returns the ID of the model in projectID:datasetID.modelid format.
 func (m *Model) FullyQualifiedName() string {
-	return fmt.Sprintf("%s:%s.%s", m.ProjectID, m.DatasetID, m.ModelID)
+	s, _ := m.Identifier(LegacySQLID)
+	return s
 }
 
 func (m *Model) toBQ() *bq.ModelReference {
diff --git a/bigquery/routine.go b/bigquery/routine.go
index 46c8ca398f..20011cf3a1 100644
--- a/bigquery/routine.go
+++ b/bigquery/routine.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"cloud.google.com/go/internal/optional"
@@ -44,9 +45,26 @@ func (r *Routine) toBQ() *bq.RoutineReference {
 	}
 }
 
+// Identifier returns the ID of the routine in the requested format.
+//
+// For Standard SQL format, the identifier will be quoted if the
+// ProjectID contains dash (-) characters.
+func (r *Routine) Identifier(f IdentifierFormat) (string, error) {
+	switch f {
+	case StandardSQLID:
+		if strings.Contains(r.ProjectID, "-") {
+			return fmt.Sprintf("`%s`.%s.%s", r.ProjectID, r.DatasetID, r.RoutineID), nil
+		}
+		return fmt.Sprintf("%s.%s.%s", r.ProjectID, r.DatasetID, r.RoutineID), nil
+	default:
+		return "", ErrUnknownIdentifierFormat
+	}
+}
+
 // FullyQualifiedName returns an identifer for the routine in project.dataset.routine format.
 func (r *Routine) FullyQualifiedName() string {
-	return fmt.Sprintf("%s.%s.%s", r.ProjectID, r.DatasetID, r.RoutineID)
+	s, _ := r.Identifier(StandardSQLID)
+	return s
 }
 
 // Create creates a Routine in the BigQuery service.
diff --git a/bigquery/table.go b/bigquery/table.go
index 202b2f737f..764e6b27a7 100644
--- a/bigquery/table.go
+++ b/bigquery/table.go
@@ -471,9 +471,46 @@ func (t *Table) toBQ() *bq.TableReference {
 	}
 }
 
+// IdentifierFormat represents a how certain resource identifiers such as table references
+// are formatted.
+type IdentifierFormat string
+
+var (
+	// StandardSQLID returns an identifier suitable for use with Standard SQL.
+	StandardSQLID IdentifierFormat = "SQL"
+
+	// LegacySQLID returns an identifier suitable for use with Legacy SQL.
+	LegacySQLID IdentifierFormat = "LEGACY_SQL"
+
+	// StorageAPIResourceID returns an identifier suitable for use with the Storage API.  Namely, it's for formatting
+	// a table resource for invoking read and write functionality.
+	StorageAPIResourceID IdentifierFormat = "STORAGE_API_RESOURCE"
+
+	// ErrUnknownIdentifierFormat is indicative of requesting an identifier in a format that is
+	// not supported.
+	ErrUnknownIdentifierFormat = errors.New("unknown identifier format")
+)
+
+// Identifier returns the ID of the table in the requested format.
+func (t *Table) Identifier(f IdentifierFormat) (string, error) {
+	switch f {
+	case LegacySQLID:
+		return fmt.Sprintf("%s:%s.%s", t.ProjectID, t.DatasetID, t.TableID), nil
+	case StorageAPIResourceID:
+		return fmt.Sprintf("projects/%s/datasets/%s/tables/%s", t.ProjectID, t.DatasetID, t.TableID), nil
+	case StandardSQLID:
+		// Note we don't need to quote the project ID here, as StandardSQL has special rules to allow
+		// dash identifiers for projects without issue in table identifiers.
+		return fmt.Sprintf("%s.%s.%s", t.ProjectID, t.DatasetID, t.TableID), nil
+	default:
+		return "", ErrUnknownIdentifierFormat
+	}
+}
+
 // FullyQualifiedName returns the ID of the table in projectID:datasetID.tableID format.
 func (t *Table) FullyQualifiedName() string {
-	return fmt.Sprintf("%s:%s.%s", t.ProjectID, t.DatasetID, t.TableID)
+	s, _ := t.Identifier(LegacySQLID)
+	return s
 }
 
 // implicitTable reports whether Table is an empty placeholder, which signifies that a new table should be created with an auto-generated Table ID.


Test patch:
diff --git a/bigquery/dataset_test.go b/bigquery/dataset_test.go
index 351947b73e..fb4d3ea2ee 100644
--- a/bigquery/dataset_test.go
+++ b/bigquery/dataset_test.go
@@ -476,3 +476,63 @@ func TestConvertAccessEntry(t *testing.T) {
 		t.Error("got nil, want error")
 	}
 }
+
+func TestDatasetIdentifiers(t *testing.T) {
+	testDataset := &Dataset{
+		ProjectID: "p",
+		DatasetID: "d",
+		c:         nil,
+	}
+	for _, tc := range []struct {
+		description string
+		in          *Dataset
+		format      IdentifierFormat
+		want        string
+		wantErr     bool
+	}{
+		{
+			description: "empty format string",
+			in:          testDataset,
+			format:      "",
+			wantErr:     true,
+		},
+		{
+			description: "legacy",
+			in:          testDataset,
+			format:      LegacySQLID,
+			want:        "p:d",
+		},
+		{
+			description: "standard unquoted",
+			in:          testDataset,
+			format:      StandardSQLID,
+			want:        "p.d",
+		},
+		{
+			description: "standard w/quoting",
+			in:          &Dataset{ProjectID: "p-p", DatasetID: "d"},
+			format:      StandardSQLID,
+			want:        "`p-p`.d",
+		},
+		{
+			description: "api resource",
+			in:          testDataset,
+			format:      StorageAPIResourceID,
+			wantErr:     true,
+		},
+	} {
+		got, err := tc.in.Identifier(tc.format)
+		if tc.wantErr && err == nil {
+			t.Errorf("case %q: wanted err, was success", tc.description)
+		}
+		if !tc.wantErr {
+			if err != nil {
+				t.Errorf("case %q: wanted success, got err: %v", tc.description, err)
+			} else {
+				if got != tc.want {
+					t.Errorf("case %q:  got %s, want %s", tc.description, got, tc.want)
+				}
+			}
+		}
+	}
+}
diff --git a/bigquery/integration_test.go b/bigquery/integration_test.go
index 766c56639a..bc8b96a137 100644
--- a/bigquery/integration_test.go
+++ b/bigquery/integration_test.go
@@ -371,12 +371,12 @@ func TestIntegration_TableCreateView(t *testing.T) {
 	}
 	ctx := context.Background()
 	table := newTable(t, schema)
+	tableIdentifier, _ := table.Identifier(StandardSQLID)
 	defer table.Delete(ctx)
 
 	// Test that standard SQL views work.
 	view := dataset.Table("t_view_standardsql")
-	query := fmt.Sprintf("SELECT APPROX_COUNT_DISTINCT(name) FROM `%s.%s.%s`",
-		dataset.ProjectID, dataset.DatasetID, table.TableID)
+	query := fmt.Sprintf("SELECT APPROX_COUNT_DISTINCT(name) FROM %s", tableIdentifier)
 	err := view.Create(context.Background(), &TableMetadata{
 		ViewQuery:      query,
 		UseStandardSQL: true,
@@ -936,10 +936,11 @@ func TestIntegration_DatasetUpdateAccess(t *testing.T) {
 	// Create a sample UDF so we can verify adding authorized UDFs
 	routineID := routineIDs.New()
 	routine := dataset.Routine(routineID)
+	routineSQLID, _ := routine.Identifier(StandardSQLID)
 
 	sql := fmt.Sprintf(`
-			CREATE FUNCTION `+"`%s`"+`(x INT64) AS (x * 3);`,
-		routine.FullyQualifiedName())
+			CREATE FUNCTION %s(x INT64) AS (x * 3);`,
+		routineSQLID)
 	if _, _, err := runQuerySQL(ctx, sql); err != nil {
 		t.Fatal(err)
 	}
@@ -1348,13 +1349,14 @@ func TestIntegration_RoutineStoredProcedure(t *testing.T) {
 	// Define a simple stored procedure via DDL.
 	routineID := routineIDs.New()
 	routine := dataset.Routine(routineID)
+	routineSQLID, _ := routine.Identifier(StandardSQLID)
 	sql := fmt.Sprintf(`
-		CREATE OR REPLACE PROCEDURE `+"`%s`"+`(val INT64)
+		CREATE OR REPLACE PROCEDURE %s(val INT64)
 		BEGIN
 			SELECT CURRENT_TIMESTAMP() as ts;
 			SELECT val * 2 as f2;
 		END`,
-		routine.FullyQualifiedName())
+		routineSQLID)
 
 	if _, _, err := runQuerySQL(ctx, sql); err != nil {
 		t.Fatal(err)
@@ -1363,8 +1365,8 @@ func TestIntegration_RoutineStoredProcedure(t *testing.T) {
 
 	// Invoke the stored procedure.
 	sql = fmt.Sprintf(`
-	CALL `+"`%s`"+`(5)`,
-		routine.FullyQualifiedName())
+	CALL %s(5)`,
+		routineSQLID)
 
 	q := client.Query(sql)
 	it, err := q.Read(ctx)
@@ -2354,8 +2356,10 @@ func TestIntegration_QueryExternalHivePartitioning(t *testing.T) {
 	}
 	defer customTable.Delete(ctx)
 
+	customTableSQLID, _ := customTable.Identifier(StandardSQLID)
+
 	// Issue a test query that prunes based on the custom hive partitioning key, and verify the result is as expected.
-	sql := fmt.Sprintf("SELECT COUNT(*) as ct FROM `%s`.%s.%s WHERE pkey=\"foo\"", customTable.ProjectID, customTable.DatasetID, customTable.TableID)
+	sql := fmt.Sprintf("SELECT COUNT(*) as ct FROM %s WHERE pkey=\"foo\"", customTableSQLID)
 	q := client.Query(sql)
 	it, err := q.Read(ctx)
 	if err != nil {
@@ -3227,10 +3231,10 @@ func TestIntegration_ModelLifecycle(t *testing.T) {
 	// Create a model via a CREATE MODEL query
 	modelID := modelIDs.New()
 	model := dataset.Model(modelID)
-	modelRef := fmt.Sprintf("%s.%s.%s", dataset.ProjectID, dataset.DatasetID, modelID)
+	modelSQLID, _ := model.Identifier(StandardSQLID)
 
 	sql := fmt.Sprintf(`
-		CREATE MODEL `+"`%s`"+`
+		CREATE MODEL %s
 		OPTIONS (
 			model_type='linear_reg',
 			max_iteration=1,
@@ -3240,7 +3244,7 @@ func TestIntegration_ModelLifecycle(t *testing.T) {
 			SELECT 'a' AS f1, 2.0 AS label
 			UNION ALL
 			SELECT 'b' AS f1, 3.8 AS label
-		)`, modelRef)
+		)`, modelSQLID)
 	if _, _, err := runQuerySQL(ctx, sql); err != nil {
 		t.Fatal(err)
 	}
@@ -3417,13 +3421,14 @@ func TestIntegration_RoutineComplexTypes(t *testing.T) {
 
 	routineID := routineIDs.New()
 	routine := dataset.Routine(routineID)
+	routineSQLID, _ := routine.Identifier(StandardSQLID)
 	sql := fmt.Sprintf(`
-		CREATE FUNCTION `+"`%s`("+`
+		CREATE FUNCTION %s(
 			arr ARRAY<STRUCT<name STRING, val INT64>>
 		  ) AS (
 			  (SELECT SUM(IF(elem.name = "foo",elem.val,null)) FROM UNNEST(arr) AS elem)
 		  )`,
-		routine.FullyQualifiedName())
+		routineSQLID)
 	if _, _, err := runQuerySQL(ctx, sql); err != nil {
 		t.Fatal(err)
 	}
@@ -3480,10 +3485,11 @@ func TestIntegration_RoutineLifecycle(t *testing.T) {
 	// Create a scalar UDF routine via a CREATE FUNCTION query
 	routineID := routineIDs.New()
 	routine := dataset.Routine(routineID)
+	routineSQLID, _ := routine.Identifier(StandardSQLID)
 
 	sql := fmt.Sprintf(`
-		CREATE FUNCTION `+"`%s`"+`(x INT64) AS (x * 3);`,
-		routine.FullyQualifiedName())
+		CREATE FUNCTION %s(x INT64) AS (x * 3);`,
+		routineSQLID)
 	if _, _, err := runQuerySQL(ctx, sql); err != nil {
 		t.Fatal(err)
 	}
diff --git a/bigquery/model_test.go b/bigquery/model_test.go
index 98b38c864e..d46bd29631 100644
--- a/bigquery/model_test.go
+++ b/bigquery/model_test.go
@@ -120,3 +120,64 @@ func TestModelMetadataUpdateToBQ(t *testing.T) {
 		}
 	}
 }
+
+func TestModelIdentifiers(t *testing.T) {
+	testModel := &Model{
+		ProjectID: "p",
+		DatasetID: "d",
+		ModelID:   "m",
+		c:         nil,
+	}
+	for _, tc := range []struct {
+		description string
+		in          *Model
+		format      IdentifierFormat
+		want        string
+		wantErr     bool
+	}{
+		{
+			description: "empty format string",
+			in:          testModel,
+			format:      "",
+			wantErr:     true,
+		},
+		{
+			description: "legacy",
+			in:          testModel,
+			format:      LegacySQLID,
+			want:        "p:d.m",
+		},
+		{
+			description: "standard unquoted",
+			in:          testModel,
+			format:      StandardSQLID,
+			want:        "p.d.m",
+		},
+		{
+			description: "standard w/dash",
+			in:          &Model{ProjectID: "p-p", DatasetID: "d", ModelID: "m"},
+			format:      StandardSQLID,
+			want:        "`p-p.d.m`",
+		},
+		{
+			description: "api resource",
+			in:          testModel,
+			format:      StorageAPIResourceID,
+			wantErr:     true,
+		},
+	} {
+		got, err := tc.in.Identifier(tc.format)
+		if tc.wantErr && err == nil {
+			t.Errorf("case %q: wanted err, was success", tc.description)
+		}
+		if !tc.wantErr {
+			if err != nil {
+				t.Errorf("case %q: wanted success, got err: %v", tc.description, err)
+			} else {
+				if got != tc.want {
+					t.Errorf("case %q:  got %s, want %s", tc.description, got, tc.want)
+				}
+			}
+		}
+	}
+}
diff --git a/bigquery/routine_test.go b/bigquery/routine_test.go
index 697713c86d..59210ff9a0 100644
--- a/bigquery/routine_test.go
+++ b/bigquery/routine_test.go
@@ -167,3 +167,63 @@ func TestRoutineTypeConversions(t *testing.T) {
 		})
 	}
 }
+
+func TestRoutineIdentifiers(t *testing.T) {
+	testRoutine := &Routine{
+		ProjectID: "p",
+		DatasetID: "d",
+		RoutineID: "r",
+		c:         nil,
+	}
+	for _, tc := range []struct {
+		description string
+		in          *Routine
+		format      IdentifierFormat
+		want        string
+		wantErr     bool
+	}{
+		{
+			description: "empty format string",
+			in:          testRoutine,
+			format:      "",
+			wantErr:     true,
+		},
+		{
+			description: "legacy",
+			in:          testRoutine,
+			wantErr:     true,
+		},
+		{
+			description: "standard unquoted",
+			in:          testRoutine,
+			format:      StandardSQLID,
+			want:        "p.d.r",
+		},
+		{
+			description: "standard w/dash",
+			in:          &Routine{ProjectID: "p-p", DatasetID: "d", RoutineID: "r"},
+			format:      StandardSQLID,
+			want:        "`p-p`.d.r",
+		},
+		{
+			description: "api resource",
+			in:          testRoutine,
+			format:      StorageAPIResourceID,
+			wantErr:     true,
+		},
+	} {
+		got, err := tc.in.Identifier(tc.format)
+		if tc.wantErr && err == nil {
+			t.Errorf("case %q: wanted err, was success", tc.description)
+		}
+		if !tc.wantErr {
+			if err != nil {
+				t.Errorf("case %q: wanted success, got err: %v", tc.description, err)
+			} else {
+				if got != tc.want {
+					t.Errorf("case %q:  got %s, want %s", tc.description, got, tc.want)
+				}
+			}
+		}
+	}
+}
diff --git a/bigquery/table_test.go b/bigquery/table_test.go
index bc0d47e66a..d56c71c2bc 100644
--- a/bigquery/table_test.go
+++ b/bigquery/table_test.go
@@ -441,3 +441,64 @@ func TestTableMetadataToUpdateToBQErrors(t *testing.T) {
 		}
 	}
 }
+
+func TestTableIdentifiers(t *testing.T) {
+	testTable := &Table{
+		ProjectID: "p",
+		DatasetID: "d",
+		TableID:   "t",
+		c:         nil,
+	}
+	for _, tc := range []struct {
+		description string
+		in          *Table
+		format      IdentifierFormat
+		want        string
+		wantErr     bool
+	}{
+		{
+			description: "empty format string",
+			in:          testTable,
+			format:      "",
+			wantErr:     true,
+		},
+		{
+			description: "legacy",
+			in:          testTable,
+			format:      LegacySQLID,
+			want:        "p:d.t",
+		},
+		{
+			description: "standard unquoted",
+			in:          testTable,
+			format:      StandardSQLID,
+			want:        "p.d.t",
+		},
+		{
+			description: "standard w/dash",
+			in:          &Table{ProjectID: "p-p", DatasetID: "d", TableID: "t"},
+			format:      StandardSQLID,
+			want:        "p-p.d.t",
+		},
+		{
+			description: "api resource",
+			in:          testTable,
+			format:      StorageAPIResourceID,
+			want:        "projects/p/datasets/d/tables/t",
+		},
+	} {
+		got, err := tc.in.Identifier(tc.format)
+		if tc.wantErr && err == nil {
+			t.Errorf("case %q: wanted err, was success", tc.description)
+		}
+		if !tc.wantErr {
+			if err != nil {
+				t.Errorf("case %q: wanted success, got err: %v", tc.description, err)
+			} else {
+				if got != tc.want {
+					t.Errorf("case %q:  got %s, want %s", tc.description, got, tc.want)
+				}
+			}
+		}
+	}
+}