Skip to content

Commit

Permalink
sql: support synchronous_commit and enable_seqscan as dummy no-op
Browse files Browse the repository at this point in the history
These vars are set by `osm2pgsql` and `ogr2ogr` respectively. These
default to the ON state, the OFF state affects performance but not
correctness.

Release note (sql change): Support the setting and getting of the
`synchronous_commit` and `enable_seqscan` variables, which do not affect
any performance characteristics. These are no-ops enabled to allow
certain tools to work.
  • Loading branch information
otan committed Jul 30, 2020
1 parent c09aeaf commit 51e9dc4
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 2 deletions.
8 changes: 8 additions & 0 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2036,6 +2036,14 @@ func (m *sessionDataMutator) SetDefaultReadOnly(val bool) {
m.data.DefaultReadOnly = val
}

func (m *sessionDataMutator) SetEnableSeqScan(val bool) {
m.data.EnableSeqScan = val
}

func (m *sessionDataMutator) SetSynchronousCommit(val bool) {
m.data.SynchronousCommit = val
}

func (m *sessionDataMutator) SetDistSQLMode(val sessiondata.DistSQLExecMode) {
m.data.DistSQLMode = val
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/set
Original file line number Diff line number Diff line change
Expand Up @@ -355,3 +355,9 @@ statement ok
SET experimental_distsql_planning = always;
SET experimental_distsql_planning = on;
SET experimental_distsql_planning = off

query T noticetrace
SET synchronous_commit = off; SET enable_seqscan = false
----
WARNING: setting session var "synchronous_commit" is a no-op
WARNING: setting session var "enable_seqscan" is a no-op
5 changes: 5 additions & 0 deletions pkg/sql/sessiondata/session_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ type SessionData struct {
// AlterColumnTypeGeneralEnabled is true if ALTER TABLE ... ALTER COLUMN ...
// TYPE x may be used for general conversions requiring online schema change/
AlterColumnTypeGeneralEnabled bool

// SynchronousCommit is a dummy setting for the synchronous_commit var.
SynchronousCommit bool
// EnableSeqScan is a dummy setting for the enable_seqscan var.
EnableSeqScan bool
}

// DataConversionConfig contains the parameters that influence
Expand Down
11 changes: 11 additions & 0 deletions pkg/sql/set_var.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ import (
"time"

"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -104,6 +107,14 @@ func unresolvedNameToStrVal(expr tree.Expr) tree.Expr {

func (n *setVarNode) startExec(params runParams) error {
var strVal string

if _, ok := DummyVars[n.name]; ok {
telemetry.Inc(sqltelemetry.DummySessionVarValueCounter(n.name))
params.p.SendClientNotice(
params.ctx,
pgnotice.NewWithSeverityf("WARNING", "setting session var %q is a no-op", n.name),
)
}
if n.typedValues != nil {
for i, v := range n.typedValues {
d, err := v.Eval(params.EvalContext())
Expand Down
7 changes: 7 additions & 0 deletions pkg/sql/sqltelemetry/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,10 @@ var ForceSavepointRestartCounter = telemetry.GetCounterOnce("sql.force_savepoint
func UnimplementedSessionVarValueCounter(varName, val string) telemetry.Counter {
return telemetry.GetCounter(fmt.Sprintf("unimplemented.sql.session_var.%s.%s", varName, val))
}

// DummySessionVarValueCounter is to be incremented every time
// a client attempts to set a compatitibility session var to a
// dummy value.
func DummySessionVarValueCounter(varName string) telemetry.Counter {
return telemetry.GetCounter(fmt.Sprintf("sql.session_var.dummy.%s", varName))
}
9 changes: 9 additions & 0 deletions pkg/sql/testdata/telemetry/set
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
feature-allowlist
sql.session_var.dummy.*
----

feature-usage
SET synchronous_commit = off; SET enable_seqscan = false
----
sql.session_var.dummy.enable_seqscan
sql.session_var.dummy.synchronous_commit
27 changes: 25 additions & 2 deletions pkg/sql/unsupported_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,29 @@

package sql

import "github.com/cockroachdb/cockroach/pkg/settings"

// DummyVars contains a list of dummy vars we do not support that
// PostgreSQL does, but are required as an easy fix to make certain
// tooling/ORMs work. These vars should not affect the correctness
// of results.
var DummyVars = map[string]sessionVar{
"enable_seqscan": makeDummyBooleanSessionVar(
"enable_seqscan",
func(m *sessionDataMutator, v bool) {
m.SetEnableSeqScan(v)
},
func(sv *settings.Values) string { return "on" },
),
"synchronous_commit": makeDummyBooleanSessionVar(
"synchronous_commit",
func(m *sessionDataMutator, v bool) {
m.SetSynchronousCommit(v)
},
func(sv *settings.Values) string { return "on" },
),
}

// UnsupportedVars contains the set of PostgreSQL session variables
// and client parameters that are not supported in CockroachDB.
// These are used to produce error messages and telemetry.
Expand Down Expand Up @@ -70,7 +93,7 @@ var UnsupportedVars = func(ss ...string) map[string]struct{} {
"enable_material",
"enable_mergejoin",
"enable_nestloop",
"enable_seqscan",
//"enable_seqscan",
"enable_sort",
"enable_tidscan",
"escape_string_warning",
Expand Down Expand Up @@ -136,7 +159,7 @@ var UnsupportedVars = func(ss ...string) map[string]struct{} {
// "standard_conforming_strings",
// "statement_timeout",
// "synchronize_seqscans",
"synchronous_commit",
// "synchronous_commit",
"tcp_keepalives_count",
"tcp_keepalives_idle",
"tcp_keepalives_interval",
Expand Down
26 changes: 26 additions & 0 deletions pkg/sql/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,29 @@ func parseBoolVar(varName, val string) (bool, error) {
return b, nil
}

// makeDummyBooleanSessionVar generates a sessionVar for a bool session setting.
// These functions allow the setting to be changed, but whose values are not used.
// They are logged to telemetry and output a notice that these are unused.
func makeDummyBooleanSessionVar(
name string, setFunc func(*sessionDataMutator, bool), sv func(_ *settings.Values) string,
) sessionVar {
return sessionVar{
GetStringVal: makePostgresBoolGetStringValFn(name),
Set: func(_ context.Context, m *sessionDataMutator, s string) error {
b, err := parseBoolVar(name, s)
if err != nil {
return err
}
setFunc(m, b)
return nil
},
Get: func(evalCtx *extendedEvalContext) string {
return formatBoolAsPostgresSetting(evalCtx.SessionData.DefaultReadOnly)
},
GlobalDefault: sv,
}
}

// varGen is the main definition array for all session variables.
// Note to maintainers: try to keep this sorted in the source code.
var varGen = map[string]sessionVar{
Expand Down Expand Up @@ -1143,6 +1166,9 @@ var varGen = map[string]sessionVar{
const compatErrMsg = "this parameter is currently recognized only for compatibility and has no effect in CockroachDB."

func init() {
for k, v := range DummyVars {
varGen[k] = v
}
// Initialize delegate.ValidVars.
for v := range varGen {
delegate.ValidVars[v] = struct{}{}
Expand Down

0 comments on commit 51e9dc4

Please sign in to comment.