Merge #43200 #43242

43200: roachtest: improve error reporting from monitor r=andreimatei a=andreimatei

Knowing that errors (in particular, test failures) come from a monitor
is useful. Before this patch, you'd have to look at the succinct stack
trace printed on failures to figure that out.

Release note: None

43242: opt: don't attempt to negate Overlaps operator r=justinj a=justinj

Fixes #43228.

Previously, we would attempt to fold expressions of the form
```
NOT(a && b)
```
which did not work, because the && operator has no negated form.
This commit adds Overlaps to the blacklist to not negate.

I thought about doing something more general (blacklist of non-negatable
ops?) to avoid this happening in the future but I think new operators
are uncommon enough that it's not worth the overhead.

Release note (bug fix): No longer fail on an expression of the form
NOT(a && b).

Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
Co-authored-by: Justin Jaffray <justin@cockroachlabs.com>

pkg/cmd/roachtest/cluster.go

@@ -40,8 +40,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
+	"github.com/cockroachdb/errors"
 	_ "github.com/lib/pq"
-	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -2151,7 +2151,7 @@ func (m *monitor) WaitE() error {
 		return errors.New("already failed")
 	}
 
-	return m.wait(roachprod, "monitor", m.nodes)
+	return errors.Wrap(m.wait(roachprod, "monitor", m.nodes), "monitor failure")
 }
 
 func (m *monitor) Wait() {
@@ -2204,9 +2204,13 @@ func (m *monitor) wait(args ...string) error {
 			m.cancel()
 			wg.Done()
 		}()
-		setErr(m.g.Wait())
+		setErr(errors.Wrap(m.g.Wait(), "monitor task failed"))
 	}()
 
+	setMonitorCmdErr := func(err error) {
+		setErr(errors.Wrap(err, "monitor command failure"))
+	}
+
 	// 2. The second goroutine forks/execs the monitoring command.
 	pipeR, pipeW := io.Pipe()
 	wg.Add(1)
@@ -2221,7 +2225,7 @@ func (m *monitor) wait(args ...string) error {
 
 		monL, err := m.l.ChildLogger(`MONITOR`)
 		if err != nil {
-			setErr(err)
+			setMonitorCmdErr(err)
 			return
 		}
 		defer monL.close()
@@ -2233,7 +2237,7 @@ func (m *monitor) wait(args ...string) error {
 			if err != context.Canceled && !strings.Contains(err.Error(), "killed") {
 				// The expected reason for an error is that the monitor was killed due
 				// to the context being canceled. Any other error is an actual error.
-				setErr(err)
+				setMonitorCmdErr(err)
 				return
 			}
 		}

pkg/cmd/roachtest/cluster_test.go

@@ -20,7 +20,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/testutils"
-	"github.com/pkg/errors"
+	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -148,7 +148,7 @@ func TestClusterMonitor(t *testing.T) {
 
 		// If wait terminates, context gets canceled.
 		err := m.wait(`true`)
-		if err != context.Canceled {
+		if !errors.Is(err, context.Canceled) {
 			t.Errorf(`expected context canceled, got: %+v`, err)
 		}
 	})

pkg/sql/opt/norm/rules/bool.opt

@@ -132,7 +132,7 @@ $input
 # by the Not operator. For example, Eq maps to Ne, and Gt maps to Le. All
 # comparisons can be negated except for the JSON comparisons.
 [NegateComparison, Normalize]
-(Not $input:(Comparison $left:* $right:*) & ^(Contains|JsonExists|JsonSomeExists|JsonAllExists))
+(Not $input:(Comparison $left:* $right:*) & ^(Contains|JsonExists|JsonSomeExists|JsonAllExists|Overlaps))
 =>
 (NegateComparison (OpName $input) $left $right)
 

pkg/sql/opt/norm/testdata/rules/bool

@@ -398,7 +398,8 @@ SELECT * FROM a WHERE
   NOT('[1, 2]' @> j) AND NOT(j <@ '[3, 4]') AND
   NOT(j ? 'foo') AND
   NOT(j ?| ARRAY['foo']) AND
-  NOT(j ?& ARRAY['foo'])
+  NOT(j ?& ARRAY['foo']) AND
+  NOT(ARRAY[i] && ARRAY[1])
 ----
 select
  ├── columns: k:1(int!null) i:2(int) f:3(float) s:4(string) j:5(jsonb)
@@ -413,7 +414,8 @@ select
       ├── NOT ('[3, 4]' @> j) [type=bool, outer=(5)]
       ├── NOT (j ? 'foo') [type=bool, outer=(5)]
       ├── NOT (j ?| ARRAY['foo']) [type=bool, outer=(5)]
-      └── NOT (j ?& ARRAY['foo']) [type=bool, outer=(5)]
+      ├── NOT (j ?& ARRAY['foo']) [type=bool, outer=(5)]
+      └── NOT (ARRAY[i] && ARRAY[1]) [type=bool, outer=(2)]
 
 # --------------------------------------------------
 # EliminateNot