Merge #151337 #151495

151337: opt: eliminate barrier expressions at the root r=mgartner a=mgartner

`BarrierExpr`s are now eliminated at the root of the expression tree.
These barriers are typically pulled up the expression tree by
normalization rules. At the root, they are useless, as their purpose is
an optimization barrier which prevents filters from being pushed into
their input. These barriers also prevented the placeholder-fast path
from applying, incurring extra overhead for queries on RLS tables.

Barrier elimination occurs in `optbuilder` when building the canonical
plan. It cannot be codified as a normalization rule because the
`Construct...` functions have no concept of a root expression when they
are invoked. The tree is built bottom-up so the root expression is only
known by callers, when all other expressions have been built. Barrier
expressions are never added nor pulled up the tree during exploration,
so removing a barrier at the root in the canonical plan should be
sufficient in eliminating a barriers at the root in all optimized plans.

Fixes #149694

Release note (performance improvement): Some types of simple queries
that query a table with RLS enabled are now more efficiently executed.


151495: sql: add goroutine IDs to text trace output in logs r=alyshanjahani-crl,arulajmani,yuzefovich a=dhartunian

when outputting a transaction trace in text format into the logs, we will now include the goroutine ID in the line with the operation like this:

```
=== operation:foo gid:123 <other tags>
```

Epic: None
Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: David Hartunian <[email protected]>

Author: 3 people

pkg/sql/opt/norm/testdata/rules/barrier

@@ -6,9 +6,9 @@ CREATE TABLE xy (x INT PRIMARY KEY, y INT);
 # EliminateRedundantBarrier
 # --------------------------------------------------
 
-# TODO(sql-queries): for simplicity, we should eliminate the barrier expression
-# at the root in this case. It serves no purpose because the filters were able
-# to permeate it.
+# NOTE: Barriers at the root are eliminated by optbuilder, not a normalization
+# rule. exprnorm bypasses optbuilder, so the root barrier remains in the output
+# of this test case.
 exprnorm expect=EliminateRedundantBarrier
 (Barrier
   (Barrier

pkg/sql/opt/norm/testdata/rules/join

@@ -2055,19 +2055,17 @@ SET ROLE alice;
 norm expect=PushLeakproofProjectionsIntoPermeableBarrier
 SELECT CASE WHEN y = 20 THEN 10 ELSE 0 END FROM rls
 ----
-barrier
+project
  ├── columns: case:6
- └── project
-      ├── columns: case:6
-      ├── select
-      │    ├── columns: y:2 alice_has_access:3!null
-      │    ├── fd: ()-->(3)
-      │    ├── scan rls
-      │    │    └── columns: y:2 alice_has_access:3
-      │    └── filters
-      │         └── alice_has_access:3 [outer=(3), constraints=(/3: [/true - /true]; tight), fd=()-->(3)]
-      └── projections
-           └── CASE WHEN y:2 = 20 THEN 10 ELSE 0 END [as=case:6, outer=(2)]
+ ├── select
+ │    ├── columns: y:2 alice_has_access:3!null
+ │    ├── fd: ()-->(3)
+ │    ├── scan rls
+ │    │    └── columns: y:2 alice_has_access:3
+ │    └── filters
+ │         └── alice_has_access:3 [outer=(3), constraints=(/3: [/true - /true]; tight), fd=()-->(3)]
+ └── projections
+      └── CASE WHEN y:2 = 20 THEN 10 ELSE 0 END [as=case:6, outer=(2)]
 
 # Only some expressions in projection are leakproof
 norm expect-not=PushLeakproofProjectionsIntoPermeableBarrier
@@ -2122,23 +2120,19 @@ project
 norm expect=PushLeakproofProjectionsIntoPermeableBarrier
 SELECT x, y, CASE WHEN y = 20 THEN 10 ELSE 0 END FROM rls
 ----
-barrier
+project
  ├── columns: x:1!null y:2 case:6
  ├── key: (1)
  ├── fd: (1)-->(2), (2)-->(6)
- └── project
-      ├── columns: case:6 x:1!null y:2
-      ├── key: (1)
-      ├── fd: (1)-->(2), (2)-->(6)
-      ├── select
-      │    ├── columns: x:1!null y:2 alice_has_access:3!null
-      │    ├── key: (1)
-      │    ├── fd: ()-->(3), (1)-->(2)
-      │    ├── scan rls
-      │    │    ├── columns: x:1!null y:2 alice_has_access:3
-      │    │    ├── key: (1)
-      │    │    └── fd: (1)-->(2,3)
-      │    └── filters
-      │         └── alice_has_access:3 [outer=(3), constraints=(/3: [/true - /true]; tight), fd=()-->(3)]
-      └── projections
-           └── CASE WHEN y:2 = 20 THEN 10 ELSE 0 END [as=case:6, outer=(2)]
+ ├── select
+ │    ├── columns: x:1!null y:2 alice_has_access:3!null
+ │    ├── key: (1)
+ │    ├── fd: ()-->(3), (1)-->(2)
+ │    ├── scan rls
+ │    │    ├── columns: x:1!null y:2 alice_has_access:3
+ │    │    ├── key: (1)
+ │    │    └── fd: (1)-->(2,3)
+ │    └── filters
+ │         └── alice_has_access:3 [outer=(3), constraints=(/3: [/true - /true]; tight), fd=()-->(3)]
+ └── projections
+      └── CASE WHEN y:2 = 20 THEN 10 ELSE 0 END [as=case:6, outer=(2)]

pkg/sql/opt/norm/testdata/rules/project

@@ -2689,21 +2689,17 @@ project
 norm expect=PushLeakproofFiltersIntoPermeableBarrier
 SELECT * FROM rls WHERE y = 20;
 ----
-barrier
+select
  ├── columns: x:1!null y:2!null alice_has_access:3!null
  ├── key: (1)
  ├── fd: ()-->(2,3)
- └── select
-      ├── columns: x:1!null y:2!null alice_has_access:3!null
-      ├── key: (1)
-      ├── fd: ()-->(2,3)
-      ├── scan rls
-      │    ├── columns: x:1!null y:2 alice_has_access:3
-      │    ├── key: (1)
-      │    └── fd: (1)-->(2,3)
-      └── filters
-           ├── alice_has_access:3 [outer=(3), constraints=(/3: [/true - /true]; tight), fd=()-->(3)]
-           └── y:2 = 20 [outer=(2), constraints=(/2: [/20 - /20]; tight), fd=()-->(2)]
+ ├── scan rls
+ │    ├── columns: x:1!null y:2 alice_has_access:3
+ │    ├── key: (1)
+ │    └── fd: (1)-->(2,3)
+ └── filters
+      ├── alice_has_access:3 [outer=(3), constraints=(/3: [/true - /true]; tight), fd=()-->(3)]
+      └── y:2 = 20 [outer=(2), constraints=(/2: [/20 - /20]; tight), fd=()-->(2)]
 
 # --------------------------------------------------
 # NormalizeLikeAny

pkg/sql/opt/norm/testdata/rules/select

@@ -16,6 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/delegate"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
+	"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/norm"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/optgen/exprgen"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -299,7 +300,12 @@ func (b *Builder) buildStmtAtRoot(stmt tree.Statement, desiredTypes []*types.T)
 	// A "root" statement cannot refer to anything from an enclosing query, so
 	// we always start with an empty scope.
 	inScope := b.allocScope()
-	return b.buildStmtAtRootWithScope(stmt, desiredTypes, inScope)
+	outScope = b.buildStmtAtRootWithScope(stmt, desiredTypes, inScope)
+	if b, ok := outScope.expr.(*memo.BarrierExpr); ok {
+		// Eliminate a barrier that has been pulled up to the root of the tree.
+		outScope.expr = b.Input
+	}
+	return outScope
 }
 
 // buildStmtAtRootWithScope is similar to buildStmtAtRoot, but allows a scope to

pkg/sql/opt/optbuilder/builder.go

@@ -2110,15 +2110,13 @@ CREATE POLICY p1 ON ob USING (user_has_access);
 norm
 SELECT * FROM ob WHERE y = 20;
 ----
-barrier
+select
  ├── columns: x:1!null y:2!null user_has_access:3!null
- └── select
-      ├── columns: x:1!null y:2!null user_has_access:3!null
-      ├── scan ob
-      │    └── columns: x:1!null y:2 user_has_access:3
-      └── filters
-           ├── user_has_access:3
-           └── y:2 = 20
+ ├── scan ob
+ │    └── columns: x:1!null y:2 user_has_access:3
+ └── filters
+      ├── user_has_access:3
+      └── y:2 = 20
 
 # Barrier needed. Try to expose with a division by zero error.
 norm

pkg/sql/opt/optbuilder/testdata/row_level_security

@@ -0,0 +1,96 @@
+exec-ddl
+CREATE TABLE t (
+  a INT PRIMARY KEY,
+  b INT,
+  c INT,
+  d INT,
+  INDEX (b),
+  INDEX (d, c)
+)
+----
+
+# The fast path is conditional on having a small estimated row count. Inject
+# statistics so that we don't have to worry about this aspect in tests.
+exec-ddl
+ALTER TABLE t INJECT STATISTICS '[
+  {
+    "columns": ["a"],
+    "created_at": "2018-05-01 1:00:00.00000+00:00",
+    "row_count": 10,
+    "distinct_count": 10
+  }
+]'
+----
+
+exec-ddl
+CREATE ROLE alice
+----
+
+exec-ddl
+CREATE ROLE bob
+----
+
+exec-ddl
+CREATE POLICY select_policy_alice
+ON t
+FOR SELECT
+TO alice
+USING (true)
+----
+
+exec-ddl
+CREATE POLICY select_policy_bob
+ON t
+FOR SELECT
+TO bob
+USING (d = 0)
+----
+
+exec-ddl
+ALTER TABLE t ENABLE ROW LEVEL SECURITY
+----
+
+exec-ddl
+SET ROLE alice
+----
+
+placeholder-fast-path
+SELECT a FROM t WHERE b = $1
+----
+placeholder-scan t@t_b_idx
+ ├── columns: a:1!null
+ ├── has-placeholder
+ ├── stats: [rows=9.9]
+ ├── key: (1)
+ └── span
+      └── $1
+
+# The index on (d, c) cannot be used because d is not constrained by the query
+# nor the policy.
+placeholder-fast-path
+SELECT a FROM t WHERE c = $1
+----
+no fast path
+
+exec-ddl
+SET ROLE bob
+----
+
+# The fast path cannot be used because there is an additional filter on d that
+# is not pushed into an index constraint.
+placeholder-fast-path
+SELECT a FROM t WHERE b = $1
+----
+no fast path
+
+placeholder-fast-path
+SELECT a FROM t WHERE c = $1
+----
+placeholder-scan t@t_d_c_idx
+ ├── columns: a:1!null
+ ├── has-placeholder
+ ├── stats: [rows=9.801]
+ ├── key: (1)
+ └── span
+      ├── 0
+      └── $1

pkg/sql/opt/xform/testdata/placeholder-fast-path/rls

@@ -145,10 +145,14 @@ func parseLine(s string) (traceLine, error) {
 	if match == nil {
 		return traceLine{}, errors.Newf("line doesn't match: %s", s)
 	}
+	text := match[3]
+	// Strip goroutine IDs from the text for test consistency
+	gidRe := regexp.MustCompile(` gid:\d+`)
+	text = string(gidRe.ReplaceAll([]byte(text), nil))
 	return traceLine{
 		timeSinceTraceStart: match[1],
 		timeSincePrev:       match[2],
-		text:                match[3],
+		text:                text,
 	}, nil
 }
 

pkg/util/tracing/span_test.go

@@ -196,16 +196,22 @@ func checkRecordingWithRedact(t T, rec tracingpb.Recording, expected string, red
 		// 	 After  |  [operation]
 		re = regexp.MustCompile(`: .*]`)
 		rec = string(re.ReplaceAll([]byte(rec), []byte("]")))
-		// 5. Change all tabs to four spaces.
+		// 5. Strip out goroutine IDs from operation lines.
+		//
+		// 	 Before |  "=== operation:foo gid:123 _verbose:‹1›"
+		// 	 After  |  "=== operation:foo _verbose:‹1›"
+		re = regexp.MustCompile(` gid:\d+`)
+		rec = string(re.ReplaceAll([]byte(rec), nil))
+		// 6. Change all tabs to four spaces.
 		rec = strings.ReplaceAll(rec, "\t", "    ")
-		// 6. Compute the outermost indentation.
+		// 7. Compute the outermost indentation.
 		indent := strings.Repeat(" ", len(rec)-len(strings.TrimLeft(rec, " ")))
-		// 7. Outdent each line by that amount.
+		// 8. Outdent each line by that amount.
 		var lines []string
 		for _, line := range strings.Split(rec, "\n") {
 			lines = append(lines, strings.TrimPrefix(line, indent))
 		}
-		// 8. Stitch everything together.
+		// 9. Stitch everything together.
 		return strings.Join(lines, "\n")
 	}
 

pkg/util/tracing/test_utils.go

@@ -259,6 +259,10 @@ func (r Recording) visitSpan(sp RecordedSpan, depth int) []traceLogData {
 	sb.SafeString("=== operation:")
 	sb.SafeString(redact.SafeString(sp.Operation))
 
+	if sp.GoroutineID != 0 {
+		sb.Printf(" gid:%d", sp.GoroutineID)
+	}
+
 	for _, tg := range sp.TagGroups {
 		var prefix redact.RedactableString
 		if tg.Name != AnonymousTagGroupName {