sql: add support for extra session vars for pg compatibility

This adds compatibility support for the following variables, with only
the default values that make sense in CockroachDB:

- `row_security`
- `synchronize_seqscans`,
- `lock_timeout`,
- `idle_in_transaction_session_timeout`

Release note: None

pkg/sql/logictest/testdata/logic_test/set

@@ -292,3 +292,15 @@ SET tracing.blah = 123
 
 statement error invalid value for parameter "ssl_renegotiation_limit"
 SET ssl_renegotiation_limit = 123
+
+subtest regression_35109_flowable
+
+statement ok
+SET DATESTYLE = ISO;
+  SET INTERVALSTYLE = POSTGRES;
+  SET extra_float_digits TO 3;
+  SET synchronize_seqscans TO off;
+  SET statement_timeout = 0;
+  SET lock_timeout = 0;
+  SET idle_in_transaction_session_timeout = 0;
+  SET row_security = off;

pkg/sql/logictest/testdata/logic_test/show_source

@@ -25,43 +25,47 @@ SELECT *
 FROM [SHOW ALL]
 WHERE variable != 'optimizer' AND variable != 'crdb_version'
 ----
-variable                           value
-application_name                   ·
-bytea_output                       hex
-client_encoding                    UTF8
-client_min_messages                notice
-database                           test
-datestyle                          ISO, MDY
-default_int_size                   8
-default_transaction_isolation      serializable
-default_transaction_read_only      off
-distsql                            off
-experimental_enable_zigzag_join    off
-experimental_force_split_at        off
-experimental_reorder_joins_limit   4
-experimental_serial_normalization  rowid
-experimental_vectorize             off
-extra_float_digits                 0
-force_savepoint_restart            off
-integer_datetimes                  on
-intervalstyle                      postgres
-max_index_keys                     32
-node_id                            1
-results_buffer_size                16384
-search_path                        public
-server_encoding                    UTF8
-server_version                     9.5.0
-server_version_num                 90500
-session_user                       root
-sql_safe_updates                   off
-standard_conforming_strings        on
-statement_timeout                  0
-timezone                           UTC
-tracing                            off
-transaction_isolation              serializable
-transaction_priority               normal
-transaction_read_only              off
-transaction_status                 NoTxn
+variable                             value
+application_name                     ·
+bytea_output                         hex
+client_encoding                      UTF8
+client_min_messages                  notice
+database                             test
+datestyle                            ISO, MDY
+default_int_size                     8
+default_transaction_isolation        serializable
+default_transaction_read_only        off
+distsql                              off
+experimental_enable_zigzag_join      off
+experimental_force_split_at          off
+experimental_reorder_joins_limit     4
+experimental_serial_normalization    rowid
+experimental_vectorize               off
+extra_float_digits                   0
+force_savepoint_restart              off
+idle_in_transaction_session_timeout  0
+integer_datetimes                    on
+intervalstyle                        postgres
+lock_timeout                         0
+max_index_keys                       32
+node_id                              1
+results_buffer_size                  16384
+row_security                         off
+search_path                          public
+server_encoding                      UTF8
+server_version                       9.5.0
+server_version_num                   90500
+session_user                         root
+sql_safe_updates                     off
+standard_conforming_strings          on
+statement_timeout                    0
+synchronize_seqscans                 on
+timezone                             UTC
+tracing                              off
+transaction_isolation                serializable
+transaction_priority                 normal
+transaction_read_only                off
+transaction_status                   NoTxn
 
 query I colnames
 SELECT * FROM [SHOW CLUSTER SETTING sql.defaults.distsql]

pkg/sql/logictest/testdata/planner_test/explain

@@ -197,7 +197,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'database'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW TIME ZONE
@@ -206,7 +206,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'timezone'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW DEFAULT_TRANSACTION_ISOLATION
@@ -215,7 +215,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'default_transaction_isolation'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW TRANSACTION ISOLATION LEVEL
@@ -224,7 +224,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'transaction_isolation'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW TRANSACTION PRIORITY
@@ -233,7 +233,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'transaction_priority'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW COLUMNS FROM foo

pkg/sql/opt/exec/execbuilder/testdata/explain

@@ -188,7 +188,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'database'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW TIME ZONE
@@ -197,7 +197,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'timezone'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW DEFAULT_TRANSACTION_ISOLATION
@@ -206,7 +206,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'default_transaction_isolation'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW TRANSACTION ISOLATION LEVEL
@@ -215,7 +215,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'transaction_isolation'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW TRANSACTION PRIORITY
@@ -224,7 +224,7 @@ render            ·       ·
  └── filter       ·       ·
       │           filter  variable = 'transaction_priority'
       └── values  ·       ·
-·                 size    3 columns, 41 rows
+·                 size    3 columns, 45 rows
 
 query TTT
 EXPLAIN SHOW COLUMNS FROM foo

pkg/sql/unsupported_vars.go

@@ -92,7 +92,7 @@ var UnsupportedVars = func(ss ...string) map[string]struct{} {
 	"geqo_threshold",
 	"gin_fuzzy_search_limit",
 	"gin_pending_list_limit",
-	"idle_in_transaction_session_timeout",
+	// "idle_in_transaction_session_timeout",
 	"ignore_checksum_failure",
 	"join_collapse_limit",
 	"lc_messages",
@@ -101,7 +101,7 @@ var UnsupportedVars = func(ss ...string) map[string]struct{} {
 	"lc_time",
 	"lo_compat_privileges",
 	"local_preload_libraries",
-	"lock_timeout",
+	// "lock_timeout",
 	"log_btree_build_stats",
 	"log_duration",
 	"log_error_verbosity",
@@ -130,7 +130,7 @@ var UnsupportedVars = func(ss ...string) map[string]struct{} {
 	"random_page_cost",
 	"replacement_sort_tuples",
 	"role",
-	"row_security",
+	// "row_security",
 	// "search_path",
 	"seed",
 	"seq_page_cost",
@@ -140,7 +140,7 @@ var UnsupportedVars = func(ss ...string) map[string]struct{} {
 	// "ssl_renegotiation_limit",
 	// "standard_conforming_strings",
 	// "statement_timeout",
-	"synchronize_seqscans",
+	//	"synchronize_seqscans",
 	"synchronous_commit",
 	"tcp_keepalives_count",
 	"tcp_keepalives_idle",

pkg/sql/vars.go

@@ -469,6 +469,13 @@ var varGen = map[string]sessionVar{
 	// See https://www.postgresql.org/docs/10/static/runtime-config-client.html#GUC-INTERVALSTYLE
 	`intervalstyle`: makeCompatStringVar(`IntervalStyle`, "postgres"),
 
+	// See https://www.postgresql.org/docs/10/static/runtime-config-client.html#GUC-LOC-TIMEOUT
+	`lock_timeout`: makeCompatIntVar(`lock_timeout`, 0),
+
+	// See https://www.postgresql.org/docs/10/static/runtime-config-client.html#GUC-IDLE-IN-TRANSACTION-SESSION-TIMEOUT
+	// See also issue #5924.
+	`idle_in_transaction_session_timeout`: makeCompatIntVar(`idle_in_transaction_session_timeout`, 0),
+
 	// See https://www.postgresql.org/docs/10/static/runtime-config-preset.html#GUC-MAX-INDEX-KEYS
 	`max_index_keys`: makeReadOnlyVar("32"),
 
@@ -588,7 +595,22 @@ var varGen = map[string]sessionVar{
 
 	// Supported for PG compatibility only.
 	// See https://www.postgresql.org/docs/10/static/runtime-config-compatible.html#GUC-STANDARD-CONFORMING-STRINGS
-	`standard_conforming_strings`: makeCompatStringVar(`standard_conforming_strings`, "on"),
+	`standard_conforming_strings`: makeCompatBoolVar(`standard_conforming_strings`, true, false /* anyAllowed */),
+
+	// See https://www.postgresql.org/docs/10/static/runtime-config-compatible.html#GUC-SYNCHRONIZE-SEQSCANS
+	// The default in pg is "on" but the behavior in CockroachDB is "off". As this does not affect
+	// results received by clients, we accept both values.
+	`synchronize_seqscans`: makeCompatBoolVar(`synchronize_seqscans`, true, true /* anyAllowed */),
+
+	// See https://www.postgresql.org/docs/10/static/runtime-config-client.html#GUC-ROW-SECURITY
+	// The default in pg is "on" but row security is not supported in CockroachDB.
+	// We blindly accept both values because as long as there are now row security policies defined,
+	// either value produces the same query results in PostgreSQL. That is, as long as CockroachDB
+	// does not support row security, accepting either "on" and "off" but ignoring the result
+	// is postgres-compatible.
+	// If/when CockroachDB is extended to support row security, the default and allowed values
+	// should be modified accordingly.
+	`row_security`: makeCompatBoolVar(`row_security`, false, true /* anyAllowed */),
 
 	`statement_timeout`: {
 		GetStringVal: stmtTimeoutVarGetStringVal,
@@ -722,7 +744,47 @@ func makeReadOnlyVar(value string) sessionVar {
 	}
 }
 
-func globalFalse(_ *settings.Values) string { return formatBoolAsPostgresSetting(false) }
+func displayPgBool(val bool) func(_ *settings.Values) string {
+	strVal := formatBoolAsPostgresSetting(val)
+	return func(_ *settings.Values) string { return strVal }
+}
+
+var globalFalse = displayPgBool(false)
+
+func makeCompatBoolVar(varName string, displayValue, anyValAllowed bool) sessionVar {
+	displayValStr := formatBoolAsPostgresSetting(displayValue)
+	return sessionVar{
+		Get: func(_ *extendedEvalContext) string { return displayValStr },
+		Set: func(_ context.Context, m *sessionDataMutator, s string) error {
+			b, err := parsePostgresBool(s)
+			if err != nil {
+				return pgerror.NewErrorf(pgerror.CodeInvalidParameterValueError, "%v", err)
+			}
+			if anyValAllowed || b == displayValue {
+				return nil
+			}
+			telemetry.Count(fmt.Sprintf("unimplemented.sql.session_var.%s.%s", varName, s))
+			allowedVals := []string{displayValStr}
+			if anyValAllowed {
+				allowedVals = append(allowedVals, formatBoolAsPostgresSetting(!displayValue))
+			}
+			return newVarValueError(varName, s, allowedVals...).SetDetailf(
+				"this parameter is currently recognized only for compatibility and has no effect in CockroachDB.")
+		},
+		GlobalDefault: func(sv *settings.Values) string { return displayValStr },
+	}
+}
+
+func makeCompatIntVar(varName string, displayValue int, extraAllowed ...int) sessionVar {
+	displayValueStr := strconv.Itoa(displayValue)
+	extraAllowedStr := make([]string, len(extraAllowed))
+	for i, v := range extraAllowed {
+		extraAllowedStr[i] = strconv.Itoa(v)
+	}
+	varObj := makeCompatStringVar(varName, displayValueStr, extraAllowedStr...)
+	varObj.GetStringVal = makeIntGetStringValFn(varName)
+	return varObj
+}
 
 func makeCompatStringVar(varName, displayValue string, extraAllowed ...string) sessionVar {
 	allowedVals := append(extraAllowed, strings.ToLower(displayValue))
@@ -737,6 +799,7 @@ func makeCompatStringVar(varName, displayValue string, extraAllowed ...string) s
 					return nil
 				}
 			}
+			telemetry.Count(fmt.Sprintf("unimplemented.sql.session_var.%s.%s", varName, s))
 			return newVarValueError(varName, s, allowedVals...).SetDetailf(
 				"this parameter is currently recognized only for compatibility and has no effect in CockroachDB.")
 		},