diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 1b2486b20c40..d74c17775f8a 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -976,6 +976,7 @@ unreserved_keyword ::= | 'NO' | 'NORMAL' | 'NO_INDEX_JOIN' + | 'NO_ZIGZAG_JOIN' | 'NOCREATEDB' | 'NOCREATELOGIN' | 'NOCANCELQUERY' @@ -2752,6 +2753,7 @@ materialize_clause ::= index_flags_param ::= 'FORCE_INDEX' '=' index_name | 'NO_INDEX_JOIN' + | 'NO_ZIGZAG_JOIN' opt_asc_desc ::= 'ASC' diff --git a/pkg/ccl/logictestccl/testdata/logic_test/as_of b/pkg/ccl/logictestccl/testdata/logic_test/as_of index 5a327e6ce246..30ad26818c39 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/as_of +++ b/pkg/ccl/logictestccl/testdata/logic_test/as_of @@ -1,7 +1,7 @@ # LogicTest: local statement ok -CREATE TABLE t (i INT) +CREATE TABLE t (i INT PRIMARY KEY, j INT UNIQUE, k INT, UNIQUE (k) STORING (j)) statement ok INSERT INTO t VALUES (2) @@ -114,12 +114,143 @@ true statement error interval duration for with_max_staleness must be greater or equal to 0 SELECT with_max_staleness(-'1s') -statement ok +# +# Tests for optimizer bounded staleness checks. +# + +statement error unimplemented: cannot use bounded staleness for queries that may touch more than one range or require an index join SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') -statement ok +statement error unimplemented: cannot use bounded staleness for queries that may touch more than one range or require an index join SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms') +statement error unimplemented: cannot use bounded staleness for MERGE JOIN +SELECT * FROM t AS t1 JOIN t AS t2 ON t1.i = t2.i AS OF SYSTEM TIME with_max_staleness('1ms') + +statement error unimplemented: cannot use bounded staleness for INNER JOIN +SELECT * FROM t AS t1 INNER HASH JOIN t AS t2 ON t1.i = t2.i AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms') + +statement error unimplemented: cannot use bounded staleness for LOOKUP JOIN +SELECT * FROM t AS t1 LEFT LOOKUP JOIN t AS t2 ON t1.i = t2.i AS OF SYSTEM TIME with_max_staleness('1ms') + +statement error unimplemented: cannot use bounded staleness for UNION +SELECT * FROM (SELECT * FROM t UNION SELECT * FROM t) AS OF SYSTEM TIME with_max_staleness('1ms') + +statement error unimplemented: cannot use bounded staleness for INTERSECT ALL +SELECT * FROM (SELECT * FROM t INTERSECT ALL SELECT * FROM t) AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms') + +statement ok +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE i = 2 + +statement ok +SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms') WHERE i = 1 + +# Projections are supported. +statement ok +SELECT i+2 FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE i = 1 + +# Select is supported. +statement ok +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE i = 2 AND j > 5 + +# Aggregations are supported. +statement ok +SELECT sum(i) FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms') WHERE i = 2 + +# Scan from a secondary index is supported. +statement ok +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE k = 2 + +# Scan from a secondary index is not supported if it requires an index join. +statement error unimplemented: cannot use bounded staleness for queries that may touch more than one range or require an index join +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE j = 2 + +# No index join or zigzag join is produced. +query T +EXPLAIN (OPT, MEMO) SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE j = 2 AND i = 1 +---- +memo (optimized, ~7KB, required=[presentation: info:6]) + ├── G1: (explain G2 [presentation: i:1,j:2,k:3]) + │ └── [presentation: info:6] + │ ├── best: (explain G2="[presentation: i:1,j:2,k:3]" [presentation: i:1,j:2,k:3]) + │ └── cost: 5.17 + ├── G2: (select G3 G4) (select G5 G6) + │ └── [presentation: i:1,j:2,k:3] + │ ├── best: (select G5 G6) + │ └── cost: 5.16 + ├── G3: (scan t,cols=(1-3)) (scan t@t_k_key,cols=(1-3)) + │ └── [] + │ ├── best: (scan t,cols=(1-3)) + │ └── cost: 1146.41 + ├── G4: (filters G7 G8) + ├── G5: (scan t,cols=(1-3),constrained) + │ └── [] + │ ├── best: (scan t,cols=(1-3),constrained) + │ └── cost: 5.13 + ├── G6: (filters G7) + ├── G7: (eq G9 G10) + ├── G8: (eq G11 G12) + ├── G9: (variable j) + ├── G10: (const 2) + ├── G11: (variable i) + └── G12: (const 1) +select + ├── scan t + │ ├── constraint: /1: [/1 - /1] + │ └── flags: no-index-join no-zigzag-join + └── filters + └── j = 2 + +# Scan may produce multiple rows. +statement error unimplemented: cannot use bounded staleness for queries that may touch more than one range or require an index join +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE k IS NULL + +# Scan may produce multiple rows. +statement error unimplemented: cannot use bounded staleness for queries that may touch more than one range or require an index join +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE k IS NULL LIMIT 10 + +# Even though the scan is limited to 1 row, from KV's perspective, this is a +# multi-row scan with a limit. That means that the scan can span multiple +# ranges, but we expect it to short-circuit once it hits the first row. In +# practice, we expect that to very often be in the first range we hit, but +# there's no guarantee of that - we could have empty ranges. +statement error unimplemented: cannot use bounded staleness for queries that may touch more than one range or require an index join +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE k IS NULL LIMIT 1 + +# Subquery contains the only scan, so it succeeds. +statement ok +SELECT (SELECT k FROM t WHERE i = 1) FROM generate_series(1, 100) AS OF SYSTEM TIME with_max_staleness('1ms') + +# Subquery does not scan data, so it succeeds. +statement ok +SELECT (SELECT random()) FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE k = 1 + +# Subqueries that perform an additional scan are not supported. +statement error unimplemented: cannot use bounded staleness for queries that may touch more than one range or require an index join +SELECT (SELECT k FROM t WHERE i = 1) FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE k = 1 + +# Bounded staleness function must match outer query if used in subquery. +statement ok +SELECT ( + SELECT k FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE i = 1 +) FROM generate_series(1, 100) AS OF SYSTEM TIME with_max_staleness('1ms') + +# Bounded staleness function must match outer query if used in subquery. +statement error unimplemented: cannot specify AS OF SYSTEM TIME with different timestamps +SELECT ( + SELECT k FROM t AS OF SYSTEM TIME with_max_staleness('2ms') WHERE i = 1 +) FROM generate_series(1, 100) AS OF SYSTEM TIME with_max_staleness('1ms') + +# Bounded staleness function must match outer query if used in subquery. +statement error AS OF SYSTEM TIME must be provided on a top-level statement +SELECT ( + SELECT k FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE i = 1 +) FROM generate_series(1, 100) + +# +# Tests for nearest_only argument. +# + statement error with_max_staleness: expected bool argument for nearest_only SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms', 5) @@ -127,16 +258,20 @@ statement error with_min_timestamp: expected bool argument for nearest_only SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp(), 5) statement ok -SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms', false) +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms', false) WHERE i = 2 statement ok -SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms', false) +SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms', false) WHERE i = 2 statement ok -SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms', true) +SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms', true) WHERE i = 2 statement ok -SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms', true) +SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '1ms', true) WHERE i = 2 + +# +# Tests for running bounded staleness queries in an explicit transaction. +# statement error AS OF SYSTEM TIME: only constant expressions or follower_read_timestamp are allowed BEGIN AS OF SYSTEM TIME with_max_staleness('1ms') @@ -147,6 +282,10 @@ BEGIN; SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') statement ok ROLLBACK +# +# Tests for bounded staleness with prepared statements. +# + statement error bounded staleness queries do not yet work with prepared statements PREPARE prep_stmt AS SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp() - '10s'::interval) diff --git a/pkg/cmd/roachprod/install/cluster_synced.go b/pkg/cmd/roachprod/install/cluster_synced.go index 1c7b34652061..148652f19727 100644 --- a/pkg/cmd/roachprod/install/cluster_synced.go +++ b/pkg/cmd/roachprod/install/cluster_synced.go @@ -68,6 +68,7 @@ type SyncedCluster struct { // all other fields are populated in newCluster. Nodes []int Secure bool + CertsDir string Env string Args []string Tag string @@ -894,6 +895,7 @@ rm -fr certs mkdir -p certs %[1]s cert create-ca --certs-dir=certs --ca-key=certs/ca.key %[1]s cert create-client root --certs-dir=certs --ca-key=certs/ca.key +%[1]s cert create-client testuser --certs-dir=certs --ca-key=certs/ca.key %[1]s cert create-node localhost %[2]s --certs-dir=certs --ca-key=certs/ca.key tar cvf certs.tar certs `, cockroachNodeBinary(c, 1), strings.Join(nodeNames, " ")) diff --git a/pkg/cmd/roachprod/install/cockroach.go b/pkg/cmd/roachprod/install/cockroach.go index 0095f3a3ab96..2bfa54e0cbe9 100644 --- a/pkg/cmd/roachprod/install/cockroach.go +++ b/pkg/cmd/roachprod/install/cockroach.go @@ -14,6 +14,7 @@ import ( _ "embed" // required for go:embed "fmt" "log" + "net/url" "os" "os/exec" "path/filepath" @@ -265,15 +266,21 @@ func (Cockroach) CertsDir(c *SyncedCluster, index int) string { // NodeURL implements the ClusterImpl.NodeDir interface. func (Cockroach) NodeURL(c *SyncedCluster, host string, port int) string { - url := fmt.Sprintf("'postgres://root@%s:%d", host, port) + var u url.URL + u.User = url.User("root") + u.Scheme = "postgres" + u.Host = fmt.Sprintf("%s:%d", host, port) + v := url.Values{} if c.Secure { - url += "?sslcert=certs%2Fclient.root.crt&sslkey=certs%2Fclient.root.key&" + - "sslrootcert=certs%2Fca.crt&sslmode=verify-full" + v.Add("sslcert", c.CertsDir+"/client.root.crt") + v.Add("sslkey", c.CertsDir+"/client.root.key") + v.Add("sslrootcert", c.CertsDir+"/ca.crt") + v.Add("sslmode", "verify-full") } else { - url += "?sslmode=disable" + v.Add("sslmode", "disable") } - url += "'" - return url + u.RawQuery = v.Encode() + return "'" + u.String() + "'" } // NodePort implements the ClusterImpl.NodeDir interface. diff --git a/pkg/cmd/roachprod/main.go b/pkg/cmd/roachprod/main.go index 0b26e2618b34..b17abcbece94 100644 --- a/pkg/cmd/roachprod/main.go +++ b/pkg/cmd/roachprod/main.go @@ -96,6 +96,7 @@ var ( nodeArgs []string tag string external = false + certsDir string adminurlOpen = false adminurlPath = "" adminurlIPs = false @@ -181,6 +182,7 @@ Available clusters: } c.Nodes = nodes c.Secure = secure + c.CertsDir = certsDir c.Env = strings.Join(nodeEnv, " ") c.Args = nodeArgs if tag != "" { @@ -1943,6 +1945,8 @@ func main() { pgurlCmd.Flags().BoolVar( &external, "external", false, "return pgurls for external connections") + pgurlCmd.Flags().StringVar( + &certsDir, "certs-dir", "./certs", "cert dir to use for secure connections") pprofCmd.Flags().DurationVar( &pprofOptions.duration, "duration", 30*time.Second, "Duration of profile to capture") diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index 4c6dba4eeab5..90b356f4e7e5 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/pkg/cmd/roachtest/cluster.go @@ -16,6 +16,7 @@ import ( "encoding/json" "fmt" "io" + "io/fs" "io/ioutil" "math/rand" "net" @@ -624,8 +625,11 @@ type clusterImpl struct { // l is the logger used to log various cluster operations. // DEPRECATED for use outside of cluster methods: Use a test's t.L() instead. // This is generally set to the current test's logger. - l *logger.Logger - expiration time.Time + l *logger.Logger + // localCertsDir is a local copy of the certs for this cluster. If this is empty, + // the cluster is running in insecure mode. + localCertsDir string + expiration time.Time // encryptDefault is true if the cluster should default to having encryption // at rest enabled. The default only applies if encryption is not explicitly // enabled or disabled by options passed to Start. @@ -1749,7 +1753,36 @@ func (c *clusterImpl) StartE(ctx context.Context, opts ...option.Option) error { } } } - return execCmd(ctx, c.l, args...) + if err := execCmd(ctx, c.l, args...); err != nil { + return err + } + if argExists(args, "--secure") { + var err error + c.localCertsDir, err = ioutil.TempDir("", "roachtest-certs") + if err != nil { + return err + } + // `roachprod get` behaves differently with `--local` depending on whether + // the target dir exists. With `--local`, it'll put the files into the + // existing dir. Without `--local`, it'll create a new subdir to house the + // certs. Bypass that distinction (which should be fixed independently, but + // that might cause fallout) by using a non-existing dir here. + c.localCertsDir = filepath.Join(c.localCertsDir, "certs") + // Get the certs from the first node. + if err := c.Get(ctx, c.l, "./certs", c.localCertsDir, c.Node(1)); err != nil { + return err + } + // Need to prevent world readable files or lib/pq will complain. + if err := filepath.Walk(c.localCertsDir, func(path string, info fs.FileInfo, err error) error { + if info.IsDir() { + return nil + } + return os.Chmod(path, 0600) + }); err != nil { + return err + } + } + return nil } // Start is like StartE() except that it will fatal the test on error. @@ -1979,6 +2012,9 @@ func (c *clusterImpl) pgURLErr( if external { args = append(args, `--external`) } + if c.localCertsDir != "" { + args = append(args, "--secure", "--certs-dir", c.localCertsDir) + } nodes := c.MakeNodes(node) args = append(args, nodes) cmd := execCmdEx(ctx, c.l, args...) @@ -2020,23 +2056,6 @@ func (c *clusterImpl) ExternalPGUrl( return c.pgURLErr(ctx, node, true /* external */) } -// ExternalPGUrlSecure returns the external Postgres endpoint for the specified -// nodes. -func (c *clusterImpl) ExternalPGUrlSecure( - ctx context.Context, node option.NodeListOption, user string, certsDir string, port int, -) ([]string, error) { - urlTemplate := "postgres://%s@%s:%d?sslcert=%s/client.%s.crt&sslkey=%s/client.%s.key&sslrootcert=%s/ca.crt&sslmode=require" - ips, err := c.ExternalIP(ctx, node) - if err != nil { - return nil, err - } - var urls []string - for _, ip := range ips { - urls = append(urls, fmt.Sprintf(urlTemplate, user, ip, port, certsDir, user, certsDir, user, certsDir)) - } - return urls, nil -} - func addrToAdminUIAddr(c *clusterImpl, addr string) (string, error) { host, port, err := net.SplitHostPort(addr) if err != nil { @@ -2225,21 +2244,6 @@ func (c *clusterImpl) ConnE(ctx context.Context, node int) (*gosql.DB, error) { return db, nil } -// ConnSecure returns a secure SQL connection to the specified node. -func (c *clusterImpl) ConnSecure( - ctx context.Context, node int, user string, certsDir string, port int, -) (*gosql.DB, error) { - urls, err := c.ExternalPGUrlSecure(ctx, c.Node(node), user, certsDir, port) - if err != nil { - return nil, err - } - db, err := gosql.Open("postgres", urls[0]) - if err != nil { - return nil, err - } - return db, nil -} - func (c *clusterImpl) MakeNodes(opts ...option.Option) string { var r option.NodeListOption for _, o := range opts { diff --git a/pkg/cmd/roachtest/cluster/cluster_interface.go b/pkg/cmd/roachtest/cluster/cluster_interface.go index 4bb567291976..1ec97c76d0a8 100644 --- a/pkg/cmd/roachtest/cluster/cluster_interface.go +++ b/pkg/cmd/roachtest/cluster/cluster_interface.go @@ -66,17 +66,11 @@ type Cluster interface { InternalPGUrl(ctx context.Context, node option.NodeListOption) ([]string, error) ExternalPGUrl(ctx context.Context, node option.NodeListOption) ([]string, error) - ExternalPGUrlSecure( - ctx context.Context, node option.NodeListOption, user string, certsDir string, port int, - ) ([]string, error) // SQL clients to nodes. Conn(ctx context.Context, node int) *gosql.DB ConnE(ctx context.Context, node int) (*gosql.DB, error) - ConnSecure( - ctx context.Context, node int, user string, certsDir string, port int, - ) (*gosql.DB, error) // URLs for the Admin UI. diff --git a/pkg/cmd/roachtest/tests/BUILD.bazel b/pkg/cmd/roachtest/tests/BUILD.bazel index 825273cb7301..1f1dfebc5344 100644 --- a/pkg/cmd/roachtest/tests/BUILD.bazel +++ b/pkg/cmd/roachtest/tests/BUILD.bazel @@ -101,6 +101,7 @@ go_library( "scrub.go", "secondary_indexes.go", "sequelize.go", + "smoketest_secure.go", "split.go", "sqlalchemy.go", "sqlalchemy_blocklist.go", diff --git a/pkg/cmd/roachtest/tests/activerecord.go b/pkg/cmd/roachtest/tests/activerecord.go index 43419a872526..dd7469d25dda 100644 --- a/pkg/cmd/roachtest/tests/activerecord.go +++ b/pkg/cmd/roachtest/tests/activerecord.go @@ -46,14 +46,12 @@ func registerActiveRecord(r registry.Registry) { } c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/canary.go b/pkg/cmd/roachtest/tests/canary.go index f09dda6a71d3..ebff0a176e03 100644 --- a/pkg/cmd/roachtest/tests/canary.go +++ b/pkg/cmd/roachtest/tests/canary.go @@ -12,7 +12,6 @@ package tests import ( "context" - gosql "database/sql" "encoding/json" "fmt" "net/http" @@ -54,26 +53,6 @@ type blocklistForVersion struct { type blocklistsForVersion []blocklistForVersion -// SecureDBConnectionParams contains information used to create a secure -// connection to CRDB. -type SecureDBConnectionParams struct { - username string - certsDir string - port int -} - -// NewSecureDBConnectionParams creates a SecureDBConnectionParams struct for creating -// a secure connection. -func NewSecureDBConnectionParams( - username string, certsDir string, port int, -) *SecureDBConnectionParams { - return &SecureDBConnectionParams{ - username: username, - certsDir: certsDir, - port: port, - } -} - // getLists returns the appropriate blocklist and ignorelist based on the // cockroach version. This check only looks to ensure that the prefix that // matches. @@ -86,27 +65,10 @@ func (b blocklistsForVersion) getLists(version string) (string, blocklist, strin return "", nil, "", nil } -func fetchCockroachVersion( - ctx context.Context, - c cluster.Cluster, - nodeIndex int, - dbConnectionParams *SecureDBConnectionParams, -) (string, error) { - var db *gosql.DB - var err error - if dbConnectionParams != nil { - db, err = c.ConnSecure( - ctx, nodeIndex, dbConnectionParams.username, - dbConnectionParams.certsDir, dbConnectionParams.port, - ) - if err != nil { - return "", err - } - } else { - db, err = c.ConnE(ctx, nodeIndex) - if err != nil { - return "", err - } +func fetchCockroachVersion(ctx context.Context, c cluster.Cluster, nodeIndex int) (string, error) { + db, err := c.ConnE(ctx, nodeIndex) + if err != nil { + return "", err } defer db.Close() var version string diff --git a/pkg/cmd/roachtest/tests/connection_latency.go b/pkg/cmd/roachtest/tests/connection_latency.go index 283707cf88eb..019b63db89e5 100644 --- a/pkg/cmd/roachtest/tests/connection_latency.go +++ b/pkg/cmd/roachtest/tests/connection_latency.go @@ -71,11 +71,9 @@ func runConnectionLatencyTest( } urlString = strings.Join(urls, " ") + // NB: certs for `testuser` are created by `roachprod start --secure`. err = c.RunE(ctx, c.Node(1), `./cockroach sql --certs-dir certs -e "CREATE USER testuser CREATEDB"`) require.NoError(t, err) - err = c.RunE(ctx, c.All(), - fmt.Sprintf(`./cockroach cert create-client testuser --certs-dir %s --ca-key=%s/ca.key`, - certsDir, certsDir)) require.NoError(t, err) err = c.RunE(ctx, c.Node(1), "./workload init connectionlatency --user testuser --secure") require.NoError(t, err) diff --git a/pkg/cmd/roachtest/tests/django.go b/pkg/cmd/roachtest/tests/django.go index 2f9818ad2653..80e86f92d170 100644 --- a/pkg/cmd/roachtest/tests/django.go +++ b/pkg/cmd/roachtest/tests/django.go @@ -41,14 +41,12 @@ func registerDjango(r registry.Registry) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - err = alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ) + err = alterZoneConfigAndClusterSettings(ctx, version, c, node[0]) if err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/gopg.go b/pkg/cmd/roachtest/tests/gopg.go index 79e538ec269c..b260d2ac66f6 100644 --- a/pkg/cmd/roachtest/tests/gopg.go +++ b/pkg/cmd/roachtest/tests/gopg.go @@ -49,14 +49,12 @@ func registerGopg(r registry.Registry) { t.Status("setting up cockroach") c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/gorm.go b/pkg/cmd/roachtest/tests/gorm.go index 5ccd168cee6a..ea6066ed693c 100644 --- a/pkg/cmd/roachtest/tests/gorm.go +++ b/pkg/cmd/roachtest/tests/gorm.go @@ -33,11 +33,11 @@ func registerGORM(r registry.Registry) { t.Status("setting up cockroach") c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0], nil); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/hibernate.go b/pkg/cmd/roachtest/tests/hibernate.go index 39b630e30174..c377dd25b31c 100644 --- a/pkg/cmd/roachtest/tests/hibernate.go +++ b/pkg/cmd/roachtest/tests/hibernate.go @@ -87,14 +87,12 @@ func registerHibernate(r registry.Registry, opt hibernateOptions) { opt.dbSetupFunc(ctx, t, c) } - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/libpq.go b/pkg/cmd/roachtest/tests/libpq.go index ded4c5f0caa9..c3752be0be94 100644 --- a/pkg/cmd/roachtest/tests/libpq.go +++ b/pkg/cmd/roachtest/tests/libpq.go @@ -35,13 +35,11 @@ func registerLibPQ(r registry.Registry) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/liquibase.go b/pkg/cmd/roachtest/tests/liquibase.go index b4af3a6f1e7f..ed80d33d3b6c 100644 --- a/pkg/cmd/roachtest/tests/liquibase.go +++ b/pkg/cmd/roachtest/tests/liquibase.go @@ -36,12 +36,12 @@ func registerLiquibase(r registry.Registry) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0], nil); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/nodejs_postgres.go b/pkg/cmd/roachtest/tests/nodejs_postgres.go index dceddc603dbb..f8e99516ff0c 100644 --- a/pkg/cmd/roachtest/tests/nodejs_postgres.go +++ b/pkg/cmd/roachtest/tests/nodejs_postgres.go @@ -13,8 +13,6 @@ package tests import ( "context" "fmt" - "os" - "path/filepath" "strings" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" @@ -46,56 +44,23 @@ func registerNodeJSPostgres(r registry.Registry) { err = c.StartE(ctx, option.StartArgs("--secure")) require.NoError(t, err) - user := "testuser" - certsDir := "/home/ubuntu/certs" - localCertsDir, err := filepath.Abs("./certs") - require.NoError(t, err) + const user = "testuser" // certs auto-generated by roachprod start --secure err = repeatRunE(ctx, t, c, node, "create sql user", fmt.Sprintf( - `./cockroach sql --certs-dir %s -e "CREATE USER %s CREATEDB"`, - certsDir, user, + `./cockroach sql --certs-dir certs -e "CREATE USER %s CREATEDB"`, user, )) require.NoError(t, err) - err = repeatRunE(ctx, t, c, c.All(), "create user certs", - fmt.Sprintf(`./cockroach cert create-client testuser --certs-dir %s --ca-key=%s/ca.key`, - certsDir, certsDir)) - require.NoError(t, err) - err = repeatRunE(ctx, t, c, node, "create test database", - fmt.Sprintf(`./cockroach sql --certs-dir %s -e "CREATE DATABASE postgres_node_test"`, certsDir), + `./cockroach sql --certs-dir certs -e "CREATE DATABASE postgres_node_test"`, ) require.NoError(t, err) - err = os.RemoveAll(localCertsDir) - require.NoError(t, err) - - err = c.Get(ctx, t.L(), certsDir, localCertsDir) - require.NoError(t, err) - - // Certs can have at max 0600 privilege. - err = filepath.Walk(localCertsDir, func(path string, info os.FileInfo, err error) error { - // Don't change permissions for the certs directory. - if path == localCertsDir { - return nil - } - if err != nil { - return err - } - return os.Chmod(path, os.FileMode(0600)) - }) + version, err := fetchCockroachVersion(ctx, c, node[0]) require.NoError(t, err) - version, err := fetchCockroachVersion( - ctx, c, node[0], NewSecureDBConnectionParams(user, localCertsDir, 26257), - ) - require.NoError(t, err) - - err = alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], - NewSecureDBConnectionParams("root", localCertsDir, 26257), - ) + err = alterZoneConfigAndClusterSettings(ctx, version, c, node[0]) require.NoError(t, err) err = repeatRunE( @@ -160,8 +125,8 @@ func registerNodeJSPostgres(r registry.Registry) { fmt.Sprintf( `cd /mnt/data1/node-postgres/ && sudo \ PGPORT=26257 PGUSER=%s PGSSLMODE=require PGDATABASE=postgres_node_test \ -PGSSLCERT=%s/client.%s.crt PGSSLKEY=%s/client.%s.key PGSSLROOTCERT=%s/ca.crt yarn test`, - user, certsDir, user, certsDir, user, certsDir, +PGSSLCERT=$HOME/certs/client.%s.crt PGSSLKEY=$HOME/certs/client.%s.key PGSSLROOTCERT=$HOME/certs/ca.crt yarn test`, + user, user, user, ), ) rawResultsStr := string(rawResults) diff --git a/pkg/cmd/roachtest/tests/orm_helpers.go b/pkg/cmd/roachtest/tests/orm_helpers.go index fee0a1d0deb8..ed115539f990 100644 --- a/pkg/cmd/roachtest/tests/orm_helpers.go +++ b/pkg/cmd/roachtest/tests/orm_helpers.go @@ -12,7 +12,6 @@ package tests import ( "context" - gosql "database/sql" "fmt" "sort" "strings" @@ -27,27 +26,11 @@ import ( // cause thousands of table descriptors and schema change jobs to accumulate // rapidly, thereby decreasing performance. func alterZoneConfigAndClusterSettings( - ctx context.Context, - version string, - c cluster.Cluster, - nodeIdx int, - dbConnectionParams *SecureDBConnectionParams, + ctx context.Context, version string, c cluster.Cluster, nodeIdx int, ) error { - var db *gosql.DB - var err error - if dbConnectionParams != nil { - db, err = c.ConnSecure( - ctx, nodeIdx, dbConnectionParams.username, - dbConnectionParams.certsDir, dbConnectionParams.port, - ) - if err != nil { - return err - } - } else { - db, err = c.ConnE(ctx, nodeIdx) - if err != nil { - return err - } + db, err := c.ConnE(ctx, nodeIdx) + if err != nil { + return err } defer db.Close() diff --git a/pkg/cmd/roachtest/tests/pgjdbc.go b/pkg/cmd/roachtest/tests/pgjdbc.go index a92e8b1b3b93..db06137f989c 100644 --- a/pkg/cmd/roachtest/tests/pgjdbc.go +++ b/pkg/cmd/roachtest/tests/pgjdbc.go @@ -39,14 +39,12 @@ func registerPgjdbc(r registry.Registry) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/pgx.go b/pkg/cmd/roachtest/tests/pgx.go index 44fb657511d2..1a0d6e568a10 100644 --- a/pkg/cmd/roachtest/tests/pgx.go +++ b/pkg/cmd/roachtest/tests/pgx.go @@ -39,14 +39,12 @@ func registerPgx(r registry.Registry) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/pop.go b/pkg/cmd/roachtest/tests/pop.go index 688763f7eefb..75d2b8826e4a 100644 --- a/pkg/cmd/roachtest/tests/pop.go +++ b/pkg/cmd/roachtest/tests/pop.go @@ -33,11 +33,11 @@ func registerPop(r registry.Registry) { t.Status("setting up cockroach") c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0], nil); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/psycopg.go b/pkg/cmd/roachtest/tests/psycopg.go index 38e6f8383830..7fe501342085 100644 --- a/pkg/cmd/roachtest/tests/psycopg.go +++ b/pkg/cmd/roachtest/tests/psycopg.go @@ -37,14 +37,12 @@ func registerPsycopg(r registry.Registry) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/registry.go b/pkg/cmd/roachtest/tests/registry.go index 36b8e38f2ed4..3e852c6e140c 100644 --- a/pkg/cmd/roachtest/tests/registry.go +++ b/pkg/cmd/roachtest/tests/registry.go @@ -94,6 +94,7 @@ func RegisterTests(r registry.Registry) { registerScrubAllChecksTPCC(r) registerScrubIndexOnlyTPCC(r) registerSecondaryIndexesMultiVersionCluster(r) + registerSecure(r) registerSequelize(r) registerSQLAlchemy(r) registerSQLSmith(r) diff --git a/pkg/cmd/roachtest/tests/ruby_pg.go b/pkg/cmd/roachtest/tests/ruby_pg.go index 98f6b4915a87..edeff8373e60 100644 --- a/pkg/cmd/roachtest/tests/ruby_pg.go +++ b/pkg/cmd/roachtest/tests/ruby_pg.go @@ -45,12 +45,12 @@ func registerRubyPG(r registry.Registry) { } c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0], nil); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/sequelize.go b/pkg/cmd/roachtest/tests/sequelize.go index 1eed70b5aeb9..f2cacc445641 100644 --- a/pkg/cmd/roachtest/tests/sequelize.go +++ b/pkg/cmd/roachtest/tests/sequelize.go @@ -41,12 +41,12 @@ func registerSequelize(r registry.Registry) { } c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0], nil); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/smoketest_secure.go b/pkg/cmd/roachtest/tests/smoketest_secure.go new file mode 100644 index 000000000000..b2a99d0f4960 --- /dev/null +++ b/pkg/cmd/roachtest/tests/smoketest_secure.go @@ -0,0 +1,41 @@ +// Copyright 2021 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package tests + +import ( + "context" + "fmt" + + "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" + "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option" + "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" + "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" + "github.com/stretchr/testify/require" +) + +func registerSecure(r registry.Registry) { + for _, numNodes := range []int{1, 3} { + r.Add(registry.TestSpec{ + Name: fmt.Sprintf("smoketest/secure/nodes=%d", numNodes), + Tags: []string{"smoketest", "weekly"}, + Owner: registry.OwnerKV, // TODO: OwnerTestEng once the open PR that introduces it has merged + Cluster: r.MakeClusterSpec(numNodes), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + c.Put(ctx, t.Cockroach(), "./cockroach") + c.Start(ctx, option.StartArgs("--secure")) + db := c.Conn(ctx, 1) + defer db.Close() + _, err := db.QueryContext(ctx, `SELECT 1`) + require.NoError(t, err) + }, + }) + } +} diff --git a/pkg/cmd/roachtest/tests/sqlalchemy.go b/pkg/cmd/roachtest/tests/sqlalchemy.go index 34797dc17fca..1673bbb375ec 100644 --- a/pkg/cmd/roachtest/tests/sqlalchemy.go +++ b/pkg/cmd/roachtest/tests/sqlalchemy.go @@ -170,14 +170,12 @@ func runSQLAlchemy(ctx context.Context, t test.Test, c cluster.Cluster) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - version, err := fetchCockroachVersion(ctx, c, node[0], nil) + version, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, version, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, version, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/typeorm.go b/pkg/cmd/roachtest/tests/typeorm.go index 13203f7a17c4..cca198d563e3 100644 --- a/pkg/cmd/roachtest/tests/typeorm.go +++ b/pkg/cmd/roachtest/tests/typeorm.go @@ -39,14 +39,12 @@ func registerTypeORM(r registry.Registry) { c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) c.Start(ctx, c.All()) - cockroachVersion, err := fetchCockroachVersion(ctx, c, node[0], nil) + cockroachVersion, err := fetchCockroachVersion(ctx, c, node[0]) if err != nil { t.Fatal(err) } - if err := alterZoneConfigAndClusterSettings( - ctx, cockroachVersion, c, node[0], nil, - ); err != nil { + if err := alterZoneConfigAndClusterSettings(ctx, cockroachVersion, c, node[0]); err != nil { t.Fatal(err) } diff --git a/pkg/sql/logictest/testdata/logic_test/zigzag_join b/pkg/sql/logictest/testdata/logic_test/zigzag_join index 79c907918b0c..47ee97fa5363 100644 --- a/pkg/sql/logictest/testdata/logic_test/zigzag_join +++ b/pkg/sql/logictest/testdata/logic_test/zigzag_join @@ -31,7 +31,13 @@ SELECT * FROM a WHERE a = 5 AND b = 2 AND c = 'foo' ---- 5 5 2 foo -# Turn off zigzag joins and verify output. +# Turn off zigzag joins and verify output. First with a hint, then with the +# session variable. +query III rowsort +SELECT n,a,b FROM a@{NO_ZIGZAG_JOIN} WHERE a = 4 AND b = 1 +---- +4 4 1 + statement ok SET enable_zigzag_join = false diff --git a/pkg/sql/opt/bench/bench_test.go b/pkg/sql/opt/bench/bench_test.go index 9c242546b23d..cbd392dbcc9b 100644 --- a/pkg/sql/opt/bench/bench_test.go +++ b/pkg/sql/opt/bench/bench_test.go @@ -484,6 +484,7 @@ func (h *harness) runSimple(tb testing.TB, query benchQuery, phase Phase) { execMemo, nil, /* catalog */ root, + &h.semaCtx, &h.evalCtx, true, /* allowAutoCommit */ ) @@ -537,6 +538,7 @@ func (h *harness) runPrepared(tb testing.TB, phase Phase) { execMemo, nil, /* catalog */ root, + &h.semaCtx, &h.evalCtx, true, /* allowAutoCommit */ ) diff --git a/pkg/sql/opt/exec/execbuilder/builder.go b/pkg/sql/opt/exec/execbuilder/builder.go index 10d6061d2181..0ad67903c267 100644 --- a/pkg/sql/opt/exec/execbuilder/builder.go +++ b/pkg/sql/opt/exec/execbuilder/builder.go @@ -38,6 +38,7 @@ type Builder struct { catalog cat.Catalog e opt.Expr disableTelemetry bool + semaCtx *tree.SemaContext evalCtx *tree.EvalContext // subqueries accumulates information about subqueries that are part of scalar @@ -93,6 +94,10 @@ type Builder struct { // containsFullIndexScan is set to true if the statement contains a secondary // index scan. ContainsFullIndexScan bool + + // containsBoundedStalenessScan is true if the query uses bounded + // staleness and contains a scan. + containsBoundedStalenessScan bool } // New constructs an instance of the execution node builder using the @@ -111,6 +116,7 @@ func New( mem *memo.Memo, catalog cat.Catalog, e opt.Expr, + semaCtx *tree.SemaContext, evalCtx *tree.EvalContext, allowAutoCommit bool, ) *Builder { @@ -120,6 +126,7 @@ func New( mem: mem, catalog: catalog, e: e, + semaCtx: semaCtx, evalCtx: evalCtx, allowAutoCommit: allowAutoCommit, initialAllowAutoCommit: allowAutoCommit, @@ -223,6 +230,12 @@ func (b *Builder) findBuiltWithExpr(id opt.WithID) *builtWithExpr { return nil } +// boundedStaleness returns true if this query uses bounded staleness. +func (b *Builder) boundedStaleness() bool { + return b.semaCtx != nil && b.semaCtx.AsOfSystemTime != nil && + b.semaCtx.AsOfSystemTime.BoundedStaleness +} + // mdVarContainer is an IndexedVarContainer implementation used by BuildScalar - // it maps indexed vars to columns in the metadata. type mdVarContainer struct { diff --git a/pkg/sql/opt/exec/execbuilder/cascades.go b/pkg/sql/opt/exec/execbuilder/cascades.go index 691f40858df9..04fa2cdb80b2 100644 --- a/pkg/sql/opt/exec/execbuilder/cascades.go +++ b/pkg/sql/opt/exec/execbuilder/cascades.go @@ -293,7 +293,9 @@ func (cb *cascadeBuilder) planCascade( } // 5. Execbuild the optimized expression. - eb := New(execFactory, &o, factory.Memo(), cb.b.catalog, optimizedExpr, evalCtx, allowAutoCommit) + eb := New( + execFactory, &o, factory.Memo(), cb.b.catalog, optimizedExpr, semaCtx, evalCtx, allowAutoCommit, + ) if bufferRef != nil { // Set up the With binding. eb.addBuiltWithExpr(cascadeInputWithID, bufferColMap, bufferRef) diff --git a/pkg/sql/opt/exec/execbuilder/format.go b/pkg/sql/opt/exec/execbuilder/format.go index 2d209e37dbdc..cc7f68a78663 100644 --- a/pkg/sql/opt/exec/execbuilder/format.go +++ b/pkg/sql/opt/exec/execbuilder/format.go @@ -41,6 +41,7 @@ func fmtInterceptor(f *memo.ExprFmtCtx, scalar opt.ScalarExpr) string { f.Memo, nil, /* catalog */ scalar, + nil, /* semaCtx */ nil, /* evalCtx */ false, /* allowAutoCommit */ ) diff --git a/pkg/sql/opt/exec/execbuilder/relational.go b/pkg/sql/opt/exec/execbuilder/relational.go index 69a0a179c1b9..60694f6c3353 100644 --- a/pkg/sql/opt/exec/execbuilder/relational.go +++ b/pkg/sql/opt/exec/execbuilder/relational.go @@ -170,6 +170,15 @@ func (b *Builder) buildRelational(e memo.RelExpr) (execPlan, error) { "cannot execute %s in a read-only transaction", b.statementTag(e)) } + // Raise error if bounded staleness is used incorrectly. + if b.boundedStaleness() { + if _, ok := boundedStalenessAllowList[e.Op()]; !ok { + return execPlan{}, unimplemented.NewWithIssuef(67562, + "cannot use bounded staleness for %s", b.statementTag(e), + ) + } + } + // Collect usage telemetry for relational node, if appropriate. if !b.disableTelemetry { if c := opt.OpTelemetryCounters[e.Op()]; c != nil { @@ -562,6 +571,30 @@ func (b *Builder) scanParams( softLimit := int64(math.Ceil(reqProps.LimitHint)) hardLimit := scan.HardLimit.RowCount() + // If this is a bounded staleness query, check that it touches at most one + // range. + if b.boundedStaleness() { + valid := true + if b.containsBoundedStalenessScan { + // We already planned a scan, perhaps as part of a subquery. + valid = false + } else if hardLimit != 0 { + // If hardLimit is not 0, from KV's perspective, this is a multi-row scan + // with a limit. That means that even if the limit is 1, the scan can span + // multiple ranges if the first range is empty. + valid = false + } else { + maxResults, ok := b.indexConstraintMaxResults(scan, relProps) + valid = ok && maxResults == 1 + } + if !valid { + return exec.ScanParams{}, opt.ColMap{}, unimplemented.NewWithIssuef(67562, + "cannot use bounded staleness for queries that may touch more than one range or require an index join", + ) + } + b.containsBoundedStalenessScan = true + } + parallelize := false if hardLimit == 0 && softLimit == 0 { maxResults, ok := b.indexConstraintMaxResults(scan, relProps) @@ -915,7 +948,9 @@ func (b *Builder) buildApplyJoin(join memo.RelExpr) (execPlan, error) { return nil, err } - eb := New(ef, &o, f.Memo(), b.catalog, newRightSide, b.evalCtx, false /* allowAutoCommit */) + eb := New( + ef, &o, f.Memo(), b.catalog, newRightSide, b.semaCtx, b.evalCtx, false, /* allowAutoCommit */ + ) eb.disableTelemetry = true eb.withExprs = withExprs plan, err := eb.Build() @@ -2470,3 +2505,25 @@ func (b *Builder) statementTag(expr memo.RelExpr) string { return expr.Op().SyntaxTag() } } + +// boundedStalenessAllowList contains the operators that may be used with +// bounded staleness queries. +var boundedStalenessAllowList = map[opt.Operator]struct{}{ + opt.ValuesOp: {}, + opt.ScanOp: {}, + opt.PlaceholderScanOp: {}, + opt.SelectOp: {}, + opt.ProjectOp: {}, + opt.GroupByOp: {}, + opt.ScalarGroupByOp: {}, + opt.DistinctOnOp: {}, + opt.EnsureDistinctOnOp: {}, + opt.LimitOp: {}, + opt.OffsetOp: {}, + opt.SortOp: {}, + opt.OrdinalityOp: {}, + opt.Max1RowOp: {}, + opt.ProjectSetOp: {}, + opt.WindowOp: {}, + opt.ExplainOp: {}, +} diff --git a/pkg/sql/opt/exec/execbuilder/statement.go b/pkg/sql/opt/exec/execbuilder/statement.go index 343b6ec00ef4..48aa5f222ec4 100644 --- a/pkg/sql/opt/exec/execbuilder/statement.go +++ b/pkg/sql/opt/exec/execbuilder/statement.go @@ -130,7 +130,14 @@ func (b *Builder) buildExplain(explain *memo.ExplainExpr) (execPlan, error) { func(ef exec.ExplainFactory) (exec.Plan, error) { // Create a separate builder for the explain query. explainBld := New( - ef, b.optimizer, b.mem, b.catalog, explain.Input, b.evalCtx, b.initialAllowAutoCommit, + ef, + b.optimizer, + b.mem, + b.catalog, + explain.Input, + b.semaCtx, + b.evalCtx, + b.initialAllowAutoCommit, ) explainBld.disableTelemetry = true return explainBld.Build() diff --git a/pkg/sql/opt/exec/execbuilder/testdata/join b/pkg/sql/opt/exec/execbuilder/testdata/join index 16e1670de7b2..c2d57e25bca3 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/join +++ b/pkg/sql/opt/exec/execbuilder/testdata/join @@ -2033,7 +2033,25 @@ CREATE TABLE zigzag ( INDEX c_idx(c) ) -# No zigzag join should be planned if enable_zigzag_join is false. +# No zigzag join should be planned if there is a hint or enable_zigzag_join is +# false. +query T +EXPLAIN SELECT a,b,c FROM zigzag@{NO_ZIGZAG_JOIN} WHERE b = 5 AND c = 6.0 +---- +distribution: local +vectorized: true +· +• filter +│ filter: c = 6.0 +│ +└── • index join + │ table: zigzag@primary + │ + └── • scan + missing stats + table: zigzag@b_idx + spans: [/5 - /5] + statement ok SET enable_zigzag_join = false diff --git a/pkg/sql/opt/idxconstraint/index_constraints_test.go b/pkg/sql/opt/idxconstraint/index_constraints_test.go index 7a6a5eac96d4..84279b440ecd 100644 --- a/pkg/sql/opt/idxconstraint/index_constraints_test.go +++ b/pkg/sql/opt/idxconstraint/index_constraints_test.go @@ -135,7 +135,7 @@ func TestIndexConstraints(t *testing.T) { if !remainingFilter.IsTrue() { execBld := execbuilder.New( nil /* execFactory */, nil /* optimizer */, f.Memo(), nil, /* catalog */ - &remainingFilter, &evalCtx, false, /* allowAutoCommit */ + &remainingFilter, &semaCtx, &evalCtx, false, /* allowAutoCommit */ ) expr, err := execBld.BuildScalar() if err != nil { diff --git a/pkg/sql/opt/memo/expr.go b/pkg/sql/opt/memo/expr.go index 6ed981e22fcf..75b6a0efefd5 100644 --- a/pkg/sql/opt/memo/expr.go +++ b/pkg/sql/opt/memo/expr.go @@ -349,6 +349,9 @@ type ScanFlags struct { // this table. NoIndexJoin bool + // NoZigzagJoin disallows use of a zigzag join for scanning this table. + NoZigzagJoin bool + // ForceIndex forces the use of a specific index (specified in Index). // ForceIndex and NoIndexJoin cannot both be set at the same time. ForceIndex bool @@ -358,7 +361,7 @@ type ScanFlags struct { // Empty returns true if there are no flags set. func (sf *ScanFlags) Empty() bool { - return !sf.NoIndexJoin && !sf.ForceIndex + return !sf.NoIndexJoin && !sf.NoZigzagJoin && !sf.ForceIndex } // JoinFlags stores restrictions on the join execution method, derived from diff --git a/pkg/sql/opt/memo/expr_format.go b/pkg/sql/opt/memo/expr_format.go index 03a74fe987f1..07cd3628fec0 100644 --- a/pkg/sql/opt/memo/expr_format.go +++ b/pkg/sql/opt/memo/expr_format.go @@ -397,9 +397,12 @@ func (f *ExprFmtCtx) formatRelational(e RelExpr, tp treeprinter.Node) { tp.Childf("limit: %s", private.HardLimit) } if !private.Flags.Empty() { + var b strings.Builder + b.WriteString("flags:") if private.Flags.NoIndexJoin { - tp.Childf("flags: no-index-join") - } else if private.Flags.ForceIndex { + b.WriteString(" no-index-join") + } + if private.Flags.ForceIndex { idx := md.Table(private.Table).Index(private.Flags.Index) dir := "" switch private.Flags.Direction { @@ -409,8 +412,12 @@ func (f *ExprFmtCtx) formatRelational(e RelExpr, tp treeprinter.Node) { case tree.Descending: dir = ",rev" } - tp.Childf("flags: force-index=%s%s", idx.Name(), dir) + b.WriteString(fmt.Sprintf(" force-index=%s%s", idx.Name(), dir)) + } + if private.Flags.NoZigzagJoin { + b.WriteString(" no-zigzag-join") } + tp.Child(b.String()) } if private.Locking != nil { strength := "" diff --git a/pkg/sql/opt/memo/interner.go b/pkg/sql/opt/memo/interner.go index e54f7759ff5a..e47ac1f87616 100644 --- a/pkg/sql/opt/memo/interner.go +++ b/pkg/sql/opt/memo/interner.go @@ -487,6 +487,7 @@ func (h *hasher) HashScanLimit(val ScanLimit) { func (h *hasher) HashScanFlags(val ScanFlags) { h.HashBool(val.NoIndexJoin) + h.HashBool(val.NoZigzagJoin) h.HashBool(val.ForceIndex) h.HashInt(int(val.Direction)) h.HashUint64(uint64(val.Index)) diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go index 07c85b0e4be8..6812b116d31d 100644 --- a/pkg/sql/opt/optbuilder/select.go +++ b/pkg/sql/opt/optbuilder/select.go @@ -510,6 +510,7 @@ func (b *Builder) buildScan( private := memo.ScanPrivate{Table: tabID, Cols: scanColIDs} if indexFlags != nil { private.Flags.NoIndexJoin = indexFlags.NoIndexJoin + private.Flags.NoZigzagJoin = indexFlags.NoZigzagJoin if indexFlags.Index != "" || indexFlags.IndexID != 0 { idx := -1 for i := 0; i < tab.IndexCount(); i++ { @@ -538,6 +539,10 @@ func (b *Builder) buildScan( if locking.isSet() { private.Locking = locking.get() } + if b.semaCtx.AsOfSystemTime != nil && b.semaCtx.AsOfSystemTime.BoundedStaleness { + private.Flags.NoIndexJoin = true + private.Flags.NoZigzagJoin = true + } b.addCheckConstraintsForTable(tabMeta) b.addComputedColsForTable(tabMeta) @@ -1181,7 +1186,6 @@ func (b *Builder) buildFromWithLateral( // validateAsOf ensures that any AS OF SYSTEM TIME timestamp is consistent with // that of the root statement. func (b *Builder) validateAsOf(asOfClause tree.AsOfClause) { - // TODO(#67558): prohibit bounded staleness in subqueries. asOf, err := tree.EvalAsOfTimestamp( b.ctx, asOfClause, diff --git a/pkg/sql/opt/partialidx/implicator_test.go b/pkg/sql/opt/partialidx/implicator_test.go index dc56b67c20bf..58a4431a1476 100644 --- a/pkg/sql/opt/partialidx/implicator_test.go +++ b/pkg/sql/opt/partialidx/implicator_test.go @@ -113,7 +113,7 @@ func TestImplicator(t *testing.T) { } else { execBld := execbuilder.New( nil /* factory */, nil /* optimizer */, f.Memo(), nil, /* catalog */ - &remainingFilters, &evalCtx, false, /* allowAutoCommit */ + &remainingFilters, &semaCtx, &evalCtx, false, /* allowAutoCommit */ ) expr, err := execBld.BuildScalar() if err != nil { diff --git a/pkg/sql/opt/xform/select_funcs.go b/pkg/sql/opt/xform/select_funcs.go index 18943be35f49..d6205a80a01d 100644 --- a/pkg/sql/opt/xform/select_funcs.go +++ b/pkg/sql/opt/xform/select_funcs.go @@ -913,20 +913,19 @@ func (c *CustomFuncs) canMaybeConstrainNonInvertedIndex( func (c *CustomFuncs) GenerateZigzagJoins( grp memo.RelExpr, scanPrivate *memo.ScanPrivate, filters memo.FiltersExpr, ) { - tab := c.e.mem.Metadata().Table(scanPrivate.Table) - // Short circuit unless zigzag joins are explicitly enabled. - if !c.e.evalCtx.SessionData.ZigzagJoinEnabled { + if !c.e.evalCtx.SessionData.ZigzagJoinEnabled || scanPrivate.Flags.NoZigzagJoin { return } fixedCols := memo.ExtractConstColumns(filters, c.e.evalCtx) - if fixedCols.Len() < 2 { // Zigzagging requires at least 2 columns to have fixed values. return } + tab := c.e.mem.Metadata().Table(scanPrivate.Table) + // Zigzag joins aren't currently equipped to produce system columns, so // don't generate any if some system columns are requested. foundSystemCol := false @@ -1271,7 +1270,7 @@ func (c *CustomFuncs) GenerateInvertedIndexZigzagJoins( grp memo.RelExpr, scanPrivate *memo.ScanPrivate, filters memo.FiltersExpr, ) { // Short circuit unless zigzag joins are explicitly enabled. - if !c.e.evalCtx.SessionData.ZigzagJoinEnabled { + if !c.e.evalCtx.SessionData.ZigzagJoinEnabled || scanPrivate.Flags.NoZigzagJoin { return } diff --git a/pkg/sql/opt/xform/testdata/rules/select b/pkg/sql/opt/xform/testdata/rules/select index 2391ae624c9f..670aa001025f 100644 --- a/pkg/sql/opt/xform/testdata/rules/select +++ b/pkg/sql/opt/xform/testdata/rules/select @@ -5232,7 +5232,7 @@ CREATE TABLE zz_redundant ( # Simple zigzag case - where all requested columns are in the indexes being # joined. -opt +opt expect=GenerateZigzagJoins SELECT q,r FROM pqr WHERE q = 1 AND r = 2 ---- inner-join (zigzag pqr@q pqr@r) @@ -5245,7 +5245,7 @@ inner-join (zigzag pqr@q pqr@r) ├── q:2 = 1 [outer=(2), constraints=(/2: [/1 - /1]; tight), fd=()-->(2)] └── r:3 = 2 [outer=(3), constraints=(/3: [/2 - /2]; tight), fd=()-->(3)] -opt +opt expect=GenerateZigzagJoins SELECT q,r FROM pqr WHERE q = 1 AND r IS NULL ---- inner-join (zigzag pqr@q pqr@r) @@ -5258,7 +5258,7 @@ inner-join (zigzag pqr@q pqr@r) ├── q:2 = 1 [outer=(2), constraints=(/2: [/1 - /1]; tight), fd=()-->(2)] └── r:3 IS NULL [outer=(3), constraints=(/3: [/NULL - /NULL]; tight), fd=()-->(3)] -memo +memo expect=GenerateZigzagJoins SELECT q,r FROM pqr WHERE q = 1 AND r = 2 ---- memo (optimized, ~13KB, required=[presentation: q:2,r:3]) @@ -5305,7 +5305,7 @@ memo (optimized, ~13KB, required=[presentation: q:2,r:3]) └── G17: (const 2) # Case where the fixed columns are extracted from a complicated expression. -opt +opt expect=GenerateZigzagJoins SELECT q,r FROM pqr WHERE q = 1 AND ((r < 1 AND r > 1) OR (r >= 2 AND r <= 2)) ---- inner-join (zigzag pqr@q pqr@r) @@ -5320,7 +5320,7 @@ inner-join (zigzag pqr@q pqr@r) # Nested zigzag case - zigzag join needs to be wrapped in a lookup join to # satisfy required columns. -opt +opt expect=GenerateZigzagJoins SELECT q,r,s FROM pqr WHERE q = 1 AND r = 2 ---- inner-join (lookup pqr) @@ -5339,7 +5339,7 @@ inner-join (lookup pqr) │ └── r:3 = 2 [outer=(3), constraints=(/3: [/2 - /2]; tight), fd=()-->(3)] └── filters (true) -memo +memo expect=GenerateZigzagJoins SELECT q,r,s FROM pqr WHERE q = 1 AND r = 2 ---- memo (optimized, ~15KB, required=[presentation: q:2,r:3,s:4]) @@ -5391,7 +5391,7 @@ memo (optimized, ~15KB, required=[presentation: q:2,r:3,s:4]) └── G19: (const 2) # Zigzag with fixed columns of different types. -opt +opt expect=GenerateZigzagJoins SELECT q,s FROM pqr WHERE q = 1 AND s = 'foo' ---- inner-join (zigzag pqr@q pqr@s) @@ -5404,7 +5404,7 @@ inner-join (zigzag pqr@q pqr@s) ├── q:2 = 1 [outer=(2), constraints=(/2: [/1 - /1]; tight), fd=()-->(2)] └── s:4 = 'foo' [outer=(4), constraints=(/4: [/'foo' - /'foo']; tight), fd=()-->(4)] -memo +memo expect=GenerateZigzagJoins SELECT q,s FROM pqr WHERE q = 1 AND s = 'foo' ---- memo (optimized, ~11KB, required=[presentation: q:2,s:4]) @@ -5445,7 +5445,7 @@ memo (optimized, ~11KB, required=[presentation: q:2,s:4]) # Zigzag with implicit equality column in addition to primary key: # indexes on (r,s) and (t,s) should be chosen even though s is not being fixed # in the ON clause. -opt +opt expect=GenerateZigzagJoins SELECT r,t FROM pqr WHERE r = 1 AND t = 'foo' ---- inner-join (zigzag pqr@rs pqr@ts) @@ -5458,7 +5458,7 @@ inner-join (zigzag pqr@rs pqr@ts) ├── r:3 = 1 [outer=(3), constraints=(/3: [/1 - /1]; tight), fd=()-->(3)] └── t:5 = 'foo' [outer=(5), constraints=(/5: [/'foo' - /'foo']; tight), fd=()-->(5)] -memo +memo expect=GenerateZigzagJoins SELECT r,t FROM pqr WHERE r = 1 AND t = 'foo' ---- memo (optimized, ~13KB, required=[presentation: r:3,t:5]) @@ -5505,7 +5505,7 @@ memo (optimized, ~13KB, required=[presentation: r:3,t:5]) └── G17: (const 'foo') # Zigzag with choice between indexes for multiple equality predicates. -opt +opt expect=GenerateZigzagJoins SELECT p,q,r,s FROM pqr WHERE q = 1 AND r = 1 AND s = 'foo' ---- inner-join (zigzag pqr@q pqr@s) @@ -5520,6 +5520,25 @@ inner-join (zigzag pqr@q pqr@s) ├── r:3 = 1 [outer=(3), constraints=(/3: [/1 - /1]; tight), fd=()-->(3)] └── s:4 = 'foo' [outer=(4), constraints=(/4: [/'foo' - /'foo']; tight), fd=()-->(4)] +# Zigzag join should not be produced when there is a NO_ZIGZAG_JOIN hint. +opt expect-not=GenerateZigzagJoins +SELECT q,r FROM pqr@{NO_ZIGZAG_JOIN} WHERE q = 1 AND r = 2 +---- +select + ├── columns: q:2!null r:3!null + ├── fd: ()-->(2,3) + ├── index-join pqr + │ ├── columns: q:2 r:3 + │ ├── fd: ()-->(2) + │ └── scan pqr@q + │ ├── columns: p:1!null q:2!null + │ ├── constraint: /2/1: [/1 - /1] + │ ├── flags: no-zigzag-join + │ ├── key: (1) + │ └── fd: ()-->(2) + └── filters + └── r:3 = 2 [outer=(3), constraints=(/3: [/2 - /2]; tight), fd=()-->(3)] + # Tests for zigzag joins over partial indexes. exec-ddl @@ -6253,6 +6272,37 @@ index-join b ├── key: (1) └── fd: (1)-->(9) +# Zigzag join should not be produced when there is a NO_ZIGZAG_JOIN hint. +opt expect-not=GenerateInvertedIndexZigzagJoins +SELECT k FROM b@{NO_ZIGZAG_JOIN} WHERE j @> '{"a": "b", "c": "d"}' +---- +project + ├── columns: k:1!null + ├── immutable + ├── key: (1) + └── inverted-filter + ├── columns: k:1!null + ├── inverted expression: /9 + │ ├── tight: true, unique: true + │ ├── union spans: empty + │ └── INTERSECTION + │ ├── span expression + │ │ ├── tight: true, unique: true + │ │ └── union spans: ["7a\x00\x01\x12b\x00\x01", "7a\x00\x01\x12b\x00\x01"] + │ └── span expression + │ ├── tight: true, unique: true + │ └── union spans: ["7c\x00\x01\x12d\x00\x01", "7c\x00\x01\x12d\x00\x01"] + ├── key: (1) + └── scan b@j_inv_idx + ├── columns: k:1!null j_inverted_key:9!null + ├── inverted constraint: /9/1 + │ └── spans + │ ├── ["7a\x00\x01\x12b\x00\x01", "7a\x00\x01\x12b\x00\x01"] + │ └── ["7c\x00\x01\x12d\x00\x01", "7c\x00\x01\x12d\x00\x01"] + ├── flags: no-zigzag-join + ├── key: (1) + └── fd: (1)-->(9) + exec-ddl CREATE TABLE inv_zz_partial ( k INT PRIMARY KEY, diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index 3ff9162ab93f..ed033e32d03b 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -805,7 +805,7 @@ func (u *sqlSymUnion) setVar() *tree.SetVar { %token MULTIPOLYGON MULTIPOLYGONM MULTIPOLYGONZ MULTIPOLYGONZM %token NAN NAME NAMES NATURAL NEVER NEXT NO NOCANCELQUERY NOCONTROLCHANGEFEED NOCONTROLJOB -%token NOCREATEDB NOCREATELOGIN NOCREATEROLE NOLOGIN NOMODIFYCLUSTERSETTING NO_INDEX_JOIN +%token NOCREATEDB NOCREATELOGIN NOCREATEROLE NOLOGIN NOMODIFYCLUSTERSETTING NO_INDEX_JOIN NO_ZIGZAG_JOIN %token NONE NON_VOTERS NORMAL NOT NOTHING NOTNULL NOVIEWACTIVITY NOWAIT NULL NULLIF NULLS NUMERIC %token OF OFF OFFSET OID OIDS OIDVECTOR ON ONLY OPT OPTION OPTIONS OR @@ -9557,6 +9557,11 @@ index_flags_param: { $$.val = &tree.IndexFlags{NoIndexJoin: true} } +| + NO_ZIGZAG_JOIN + { + $$.val = &tree.IndexFlags{NoZigzagJoin: true} + } | IGNORE_FOREIGN_KEYS { @@ -9621,6 +9626,7 @@ opt_index_flags: // Index flags: // '{' FORCE_INDEX = [, ...] '}' // '{' NO_INDEX_JOIN [, ...] '}' +// '{' NO_ZIGZAG_JOIN [, ...] '}' // '{' IGNORE_FOREIGN_KEYS [, ...] '}' // // Join types: @@ -13061,6 +13067,7 @@ unreserved_keyword: | NO | NORMAL | NO_INDEX_JOIN +| NO_ZIGZAG_JOIN | NOCREATEDB | NOCREATELOGIN | NOCANCELQUERY diff --git a/pkg/sql/parser/testdata/select_clauses b/pkg/sql/parser/testdata/select_clauses index b71ddf1fbe70..37220ab5c80a 100644 --- a/pkg/sql/parser/testdata/select_clauses +++ b/pkg/sql/parser/testdata/select_clauses @@ -318,6 +318,14 @@ SELECT ('a') FROM t@{NO_INDEX_JOIN} -- fully parenthesized SELECT _ FROM t@{NO_INDEX_JOIN} -- literals removed SELECT 'a' FROM _@{NO_INDEX_JOIN} -- identifiers removed +parse +SELECT 'a' FROM t@{NO_ZIGZAG_JOIN} +---- +SELECT 'a' FROM t@{NO_ZIGZAG_JOIN} +SELECT ('a') FROM t@{NO_ZIGZAG_JOIN} -- fully parenthesized +SELECT _ FROM t@{NO_ZIGZAG_JOIN} -- literals removed +SELECT 'a' FROM _@{NO_ZIGZAG_JOIN} -- identifiers removed + parse SELECT 'a' FROM t@{IGNORE_FOREIGN_KEYS} ---- @@ -335,12 +343,12 @@ SELECT _ FROM t@{FORCE_INDEX=idx,ASC} -- literals removed SELECT 'a' FROM _@{FORCE_INDEX=_,ASC} -- identifiers removed parse -SELECT 'a' FROM t@{FORCE_INDEX=idx,DESC,IGNORE_FOREIGN_KEYS} +SELECT 'a' FROM t@{FORCE_INDEX=idx,DESC,IGNORE_FOREIGN_KEYS,NO_ZIGZAG_JOIN} ---- -SELECT 'a' FROM t@{FORCE_INDEX=idx,DESC,IGNORE_FOREIGN_KEYS} -SELECT ('a') FROM t@{FORCE_INDEX=idx,DESC,IGNORE_FOREIGN_KEYS} -- fully parenthesized -SELECT _ FROM t@{FORCE_INDEX=idx,DESC,IGNORE_FOREIGN_KEYS} -- literals removed -SELECT 'a' FROM _@{FORCE_INDEX=_,DESC,IGNORE_FOREIGN_KEYS} -- identifiers removed +SELECT 'a' FROM t@{FORCE_INDEX=idx,DESC,NO_ZIGZAG_JOIN,IGNORE_FOREIGN_KEYS} -- normalized! +SELECT ('a') FROM t@{FORCE_INDEX=idx,DESC,NO_ZIGZAG_JOIN,IGNORE_FOREIGN_KEYS} -- fully parenthesized +SELECT _ FROM t@{FORCE_INDEX=idx,DESC,NO_ZIGZAG_JOIN,IGNORE_FOREIGN_KEYS} -- literals removed +SELECT 'a' FROM _@{FORCE_INDEX=_,DESC,NO_ZIGZAG_JOIN,IGNORE_FOREIGN_KEYS} -- identifiers removed error SELECT a FROM foo@{FORCE_INDEX} diff --git a/pkg/sql/plan_opt.go b/pkg/sql/plan_opt.go index a132a1aa06f6..e64f43d2c0ae 100644 --- a/pkg/sql/plan_opt.go +++ b/pkg/sql/plan_opt.go @@ -210,6 +210,7 @@ func (p *planner) makeOptimizerPlan(ctx context.Context) error { &p.stmt, newDistSQLSpecExecFactory(p, planningMode), execMemo, + p.SemaCtx(), p.EvalContext(), p.autoCommit, ) @@ -244,6 +245,7 @@ func (p *planner) makeOptimizerPlan(ctx context.Context) error { &p.stmt, newDistSQLSpecExecFactory(p, distSQLLocalOnlyPlanning), execMemo, + p.SemaCtx(), p.EvalContext(), p.autoCommit, ) @@ -264,6 +266,7 @@ func (p *planner) makeOptimizerPlan(ctx context.Context) error { &p.stmt, newExecFactory(p), execMemo, + p.SemaCtx(), p.EvalContext(), p.autoCommit, ) @@ -554,6 +557,7 @@ func (opc *optPlanningCtx) runExecBuilder( stmt *Statement, f exec.Factory, mem *memo.Memo, + semaCtx *tree.SemaContext, evalCtx *tree.EvalContext, allowAutoCommit bool, ) error { @@ -563,7 +567,9 @@ func (opc *optPlanningCtx) runExecBuilder( var containsFullIndexScan bool if !planTop.instrumentation.ShouldBuildExplainPlan() { // No instrumentation. - bld := execbuilder.New(f, &opc.optimizer, mem, &opc.catalog, mem.RootExpr(), evalCtx, allowAutoCommit) + bld := execbuilder.New( + f, &opc.optimizer, mem, &opc.catalog, mem.RootExpr(), semaCtx, evalCtx, allowAutoCommit, + ) plan, err := bld.Build() if err != nil { return err @@ -576,7 +582,14 @@ func (opc *optPlanningCtx) runExecBuilder( // Create an explain factory and record the explain.Plan. explainFactory := explain.NewFactory(f) bld := execbuilder.New( - explainFactory, &opc.optimizer, mem, &opc.catalog, mem.RootExpr(), evalCtx, allowAutoCommit, + explainFactory, + &opc.optimizer, + mem, + &opc.catalog, + mem.RootExpr(), + semaCtx, + evalCtx, + allowAutoCommit, ) plan, err := bld.Build() if err != nil { diff --git a/pkg/sql/sem/tree/eval_test.go b/pkg/sql/sem/tree/eval_test.go index 6c2c5a549de6..bfc2b4631526 100644 --- a/pkg/sql/sem/tree/eval_test.go +++ b/pkg/sql/sem/tree/eval_test.go @@ -104,7 +104,7 @@ func optBuildScalar(evalCtx *tree.EvalContext, e tree.Expr) (tree.TypedExpr, err bld := execbuilder.New( nil /* factory */, &o, o.Memo(), nil /* catalog */, o.Memo().RootExpr(), - evalCtx, false, /* allowAutoCommit */ + &semaCtx, evalCtx, false, /* allowAutoCommit */ ) expr, err := bld.BuildScalar() if err != nil { diff --git a/pkg/sql/sem/tree/select.go b/pkg/sql/sem/tree/select.go index 1ae1526c471c..e6cd5d593088 100644 --- a/pkg/sql/sem/tree/select.go +++ b/pkg/sql/sem/tree/select.go @@ -268,6 +268,7 @@ type IndexID uint32 // - FORCE_INDEX= // - ASC / DESC // - NO_INDEX_JOIN +// - NO_ZIGZAG_JOIN // - IGNORE_FOREIGN_KEYS // It is used optionally after a table name in SELECT statements. type IndexFlags struct { @@ -278,6 +279,8 @@ type IndexFlags struct { Direction Direction // NoIndexJoin cannot be specified together with an index. NoIndexJoin bool + // NoZigzagJoin indicates we should not plan a zigzag join for this scan. + NoZigzagJoin bool // IgnoreForeignKeys disables optimizations based on outbound foreign key // references from this table. This is useful in particular for scrub queries // used to verify the consistency of foreign key relations. @@ -299,6 +302,9 @@ func (ih *IndexFlags) CombineWith(other *IndexFlags) error { if ih.NoIndexJoin && other.NoIndexJoin { return errors.New("NO_INDEX_JOIN specified multiple times") } + if ih.NoZigzagJoin && other.NoZigzagJoin { + return errors.New("NO_ZIGZAG_JOIN specified multiple times") + } if ih.IgnoreForeignKeys && other.IgnoreForeignKeys { return errors.New("IGNORE_FOREIGN_KEYS specified multiple times") } @@ -307,6 +313,7 @@ func (ih *IndexFlags) CombineWith(other *IndexFlags) error { } result := *ih result.NoIndexJoin = ih.NoIndexJoin || other.NoIndexJoin + result.NoZigzagJoin = ih.NoZigzagJoin || other.NoZigzagJoin result.IgnoreForeignKeys = ih.IgnoreForeignKeys || other.IgnoreForeignKeys result.IgnoreUniqueWithoutIndexKeys = ih.IgnoreUniqueWithoutIndexKeys || other.IgnoreUniqueWithoutIndexKeys @@ -348,8 +355,8 @@ func (ih *IndexFlags) Check() error { // Format implements the NodeFormatter interface. func (ih *IndexFlags) Format(ctx *FmtCtx) { ctx.WriteByte('@') - if !ih.NoIndexJoin && !ih.IgnoreForeignKeys && !ih.IgnoreUniqueWithoutIndexKeys && - ih.Direction == 0 { + if !ih.NoIndexJoin && !ih.NoZigzagJoin && !ih.IgnoreForeignKeys && + !ih.IgnoreUniqueWithoutIndexKeys && ih.Direction == 0 { if ih.Index != "" { ctx.FormatNode(&ih.Index) } else { @@ -379,6 +386,11 @@ func (ih *IndexFlags) Format(ctx *FmtCtx) { ctx.WriteString("NO_INDEX_JOIN") } + if ih.NoZigzagJoin { + sep() + ctx.WriteString("NO_ZIGZAG_JOIN") + } + if ih.IgnoreForeignKeys { sep() ctx.WriteString("IGNORE_FOREIGN_KEYS")