From 1816f0b79338997f0dc2dc43e2b725a8560e80de Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Thu, 7 Sep 2017 09:44:44 -0400 Subject: [PATCH] bigquery: introduce UseLegacySQL options This is the first step in switching to standard SQL as the default. We'll announce this change and ask people who want to use legacy SQL to explictly set these options. Then we can switch the default without breakage. Change-Id: I59c8ae4ace63fa0df59c86f23746ad5aa197857e Reviewed-on: https://code-review.googlesource.com/16630 Reviewed-by: kokoro Reviewed-by: Sarah Adams Reviewed-by: Sai Cheemalapati --- bigquery/create_table_test.go | 12 ++++++ bigquery/integration_test.go | 71 +++++++++++++++++++++++++++++++++++ bigquery/query.go | 15 +++++++- bigquery/query_test.go | 24 ++++++++++++ bigquery/service.go | 8 ++++ bigquery/table.go | 10 +++++ 6 files changed, 139 insertions(+), 1 deletion(-) diff --git a/bigquery/create_table_test.go b/bigquery/create_table_test.go index 4c610c80b551..8f586abf8e23 100644 --- a/bigquery/create_table_test.go +++ b/bigquery/create_table_test.go @@ -108,3 +108,15 @@ func TestCreateTableOptions(t *testing.T) { } } } + +func TestCreateTableOptionsLegacySQL(t *testing.T) { + c := &Client{ + projectID: "p", + service: &bigqueryService{}, + } + ds := c.Dataset("d") + table := ds.Table("t") + if err := table.Create(context.Background(), UseStandardSQL(), UseLegacySQL()); err == nil { + t.Fatal("no error using both standard and legacy SQL options") + } +} diff --git a/bigquery/integration_test.go b/bigquery/integration_test.go index 99604e631b0b..f693070c143e 100644 --- a/bigquery/integration_test.go +++ b/bigquery/integration_test.go @@ -1056,6 +1056,77 @@ func TestIntegration_ReadNullIntoStruct(t *testing.T) { } } +const ( + stdName = "`bigquery-public-data.samples.shakespeare`" + legacyName = "[bigquery-public-data:samples.shakespeare]" +) + +// These tests exploit the fact that the two SQL versions have different syntaxes for +// fully-qualified table names. +var useLegacySqlTests = []struct { + t string // name of table + std, legacy bool // use standard/legacy SQL + err bool // do we expect an error? +}{ + {t: legacyName, std: false, legacy: true, err: false}, + {t: legacyName, std: true, legacy: false, err: true}, + {t: legacyName, std: false, legacy: false, err: false}, // legacy SQL is default + {t: legacyName, std: true, legacy: true, err: true}, + {t: stdName, std: false, legacy: true, err: true}, + {t: stdName, std: true, legacy: false, err: false}, + {t: stdName, std: false, legacy: false, err: true}, // legacy SQL is default + {t: stdName, std: true, legacy: true, err: true}, +} + +func TestIntegration_QueryUseLegacySQL(t *testing.T) { + // Test the UseLegacySQL and UseStandardSQL options for queries. + if client == nil { + t.Skip("Integration tests skipped") + } + ctx := context.Background() + for _, test := range useLegacySqlTests { + q := client.Query(fmt.Sprintf("select word from %s limit 1", test.t)) + q.UseStandardSQL = test.std + q.UseLegacySQL = test.legacy + _, err := q.Read(ctx) + gotErr := err != nil + if gotErr && !test.err { + t.Errorf("%+v:\nunexpected error: %v", test, err) + } else if !gotErr && test.err { + t.Errorf("%+v:\nsucceeded, but want error", test) + } + } +} + +func TestIntegration_TableUseLegacySQL(t *testing.T) { + // Test the UseLegacySQL and UseStandardSQL options for CreateTable. + if client == nil { + t.Skip("Integration tests skipped") + } + ctx := context.Background() + table := newTable(t, schema) + defer table.Delete(ctx) + for i, test := range useLegacySqlTests { + view := dataset.Table(fmt.Sprintf("t_view_%d", i)) + vq := ViewQuery(fmt.Sprintf("SELECT word from %s", test.t)) + opts := []CreateTableOption{vq} + if test.std { + opts = append(opts, UseStandardSQL()) + } + if test.legacy { + opts = append(opts, UseLegacySQL()) + } + err := view.Create(ctx, opts...) + gotErr := err != nil + if gotErr && !test.err { + t.Errorf("%+v:\nunexpected error: %v", test, err) + } else if !gotErr && test.err { + t.Errorf("%+v:\nsucceeded, but want error", test) + } + view.Delete(ctx) + } +} + // Creates a new, temporary table with a unique name and the given schema. func newTable(t *testing.T, s Schema) *Table { name := fmt.Sprintf("t%d", time.Now().UnixNano()) diff --git a/bigquery/query.go b/bigquery/query.go index d9b63a7dfe48..6459541cfb16 100644 --- a/bigquery/query.go +++ b/bigquery/query.go @@ -15,6 +15,8 @@ package bigquery import ( + "errors" + "golang.org/x/net/context" bq "google.golang.org/api/bigquery/v2" ) @@ -89,6 +91,9 @@ type QueryConfig struct { // The default is false (using legacy SQL). UseStandardSQL bool + // UseLegacySQL causes the query to use legacy SQL. + UseLegacySQL bool + // Parameters is a list of query parameters. The presence of parameters // implies the use of standard SQL. // If the query uses positional syntax ("?"), then no parameter may have a name. @@ -177,11 +182,19 @@ func (q *QueryConfig) populateJobQueryConfig(conf *bq.JobConfigurationQuery) err if q.MaxBytesBilled >= 1 { conf.MaximumBytesBilled = q.MaxBytesBilled } + if q.UseStandardSQL && q.UseLegacySQL { + return errors.New("bigquery: cannot provide both UseStandardSQL and UseLegacySQL") + } + if len(q.Parameters) > 0 && q.UseLegacySQL { + return errors.New("bigquery: cannot provide both Parameters (implying standard SQL) and UseLegacySQL") + } if q.UseStandardSQL || len(q.Parameters) > 0 { conf.UseLegacySql = false conf.ForceSendFields = append(conf.ForceSendFields, "UseLegacySql") } - + if q.UseLegacySQL { + conf.UseLegacySql = true + } if q.Dst != nil && !q.Dst.implicitTable() { conf.DestinationTable = q.Dst.tableRefProto() } diff --git a/bigquery/query_test.go b/bigquery/query_test.go index b5f20464ffb1..b66b4a250d13 100644 --- a/bigquery/query_test.go +++ b/bigquery/query_test.go @@ -15,6 +15,7 @@ package bigquery import ( + "fmt" "testing" "cloud.google.com/go/internal/testutil" @@ -304,3 +305,26 @@ func TestConfiguringQuery(t *testing.T) { t.Errorf("querying: got:\n%v\nwant:\n%v", s.Job, want) } } + +func TestQueryLegacySQL(t *testing.T) { + c := &Client{ + projectID: "project-id", + service: &testService{}, + } + q := c.Query("q") + q.UseStandardSQL = true + q.UseLegacySQL = true + _, err := q.Run(context.Background()) + if err == nil { + t.Error("UseStandardSQL and UseLegacySQL: got nil, want error") + } + q = c.Query("q") + q.Parameters = []QueryParameter{{Name: "p", Value: 3}} + q.UseLegacySQL = true + _, err = q.Run(context.Background()) + if err == nil { + t.Error("Parameters and UseLegacySQL: got nil, want error") + } else { + fmt.Println(err) + } +} diff --git a/bigquery/service.go b/bigquery/service.go index dc3f3946cbe4..0bf6bbc7441b 100644 --- a/bigquery/service.go +++ b/bigquery/service.go @@ -15,6 +15,7 @@ package bigquery import ( + "errors" "fmt" "io" "net/http" @@ -501,6 +502,7 @@ type createTableConf struct { viewQuery string schema *bq.TableSchema useStandardSQL bool + useLegacySQL bool timePartitioning *TimePartitioning } @@ -510,6 +512,9 @@ type createTableConf struct { // Note: expiration can only be set during table creation. // Note: after table creation, a view can be modified only if its table was initially created with a view. func (s *bigqueryService) createTable(ctx context.Context, conf *createTableConf) error { + if conf.useStandardSQL && conf.useLegacySQL { + return errors.New("bigquery: cannot provide both UseStandardSQL and UseLegacySQL") + } table := &bq.Table{ // TODO(jba): retry? Is this always idempotent? TableReference: &bq.TableReference{ @@ -530,6 +535,9 @@ func (s *bigqueryService) createTable(ctx context.Context, conf *createTableConf table.View.UseLegacySql = false table.View.ForceSendFields = append(table.View.ForceSendFields, "UseLegacySql") } + if conf.useLegacySQL { + table.View.UseLegacySql = true + } } if conf.schema != nil { table.Schema = conf.schema diff --git a/bigquery/table.go b/bigquery/table.go index 721f5a5d8c22..41aa1fe37ec9 100644 --- a/bigquery/table.go +++ b/bigquery/table.go @@ -206,6 +206,16 @@ func (opt useStandardSQL) customizeCreateTable(conf *createTableConf) { conf.useStandardSQL = true } +type useLegacySQL struct{} + +// UseLegacySQL returns a CreateTableOption to set the table to use legacy SQL. +// This is currently the default. +func UseLegacySQL() CreateTableOption { return useLegacySQL{} } + +func (opt useLegacySQL) customizeCreateTable(conf *createTableConf) { + conf.useLegacySQL = true +} + // TimePartitioning is a CreateTableOption that can be used to set time-based // date partitioning on a table. // For more information see: https://cloud.google.com/bigquery/docs/creating-partitioned-tables