Skip to content

Commit

Permalink
sql: don't disable rewrites for internal SQL
Browse files Browse the repository at this point in the history
Casting an OID to REGNAMESPACE runs this SQL using the internal executor:
```
SELECT pg_namespace.oid, nspname FROM pg_catalog.pg_namespace WHERE oid = $1
```
When the `GenerateConstrainedScans` rule is disabled, this returns no
rows and so doesn't apply the cast. The same SQL, not run via an
internal executor, but also with `GenerateConstrainedScans` disabled,
behaves correctly. Disabling `GenerateConstrainedScans` all the time
prevents the cluster from starting.

Internal SQL is sensitive to disabled optimizer rewrite rules, and it
could cause server instability to allow rules to ever be disabled for
internal SQL. Also, the `testing_optimizer_disable_rule_probability` session
setting is mainly meant to perturb query plans of foreground SQL in
randomized tests, and enabling it for internal SQL could make these
tests more noisy and create harder to debug issues.

Therefore the fix is to set the `testing_optimizer_disable_rule_probability`
setting to zero for internal SQL statements.

Fixes cockroachdb#98322

Release note: None
  • Loading branch information
Mark Sirek committed Mar 21, 2023
1 parent b791abd commit 546da49
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/sql/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,12 @@ func applyInternalExecutorSessionExceptions(sd *sessiondata.SessionData) {
// DisableBuffering is not supported by the InternalExecutor
// which uses streamingCommandResults.
sd.LocalOnlySessionData.AvoidBuffering = false
// Rewrite rules should not be disabled for internal SQL. This is mainly a
// flag for testing foreground SQL. Disrupting the behavior of internal SQL,
// which may rely on rewrite rules for proper behavior, could potentially make
// the cluster unstable and may make randomized tests more noisy with
// difficult to debug behavior.
sd.LocalOnlySessionData.TestingOptimizerDisableRuleProbability = 0
}

// applyOverrides overrides the respective fields from sd for all the fields set on o.
Expand Down
13 changes: 13 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/namespace
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,16 @@ ALTER INDEX a_pk3 RENAME TO a_pk4
# But qualification resolves the problem.
statement ok
ALTER INDEX public.a_pk3 RENAME TO a_pk4

statement ok
SET testing_optimizer_disable_rule_probability=1.0

# A cast which relies on an internal executor should not flake with rewrite
# rules disabled.
query T
select regnamespace(101)::REGNAMESPACE;
----
public

statement ok
RESET testing_optimizer_disable_rule_probability

0 comments on commit 546da49

Please sign in to comment.