sql: support SET LOCAL and txn-scoped session variable changes

[knz]

https://www.postgresql.org/docs/10/sql-set.html

The effects of SET LOCAL last only till the end of the current transaction, whether committed or not. A special case is SET followed by SET LOCAL within a single transaction: the SET LOCAL value will be seen until the end of the transaction, but afterwards (if the transaction is committed) the SET value will take effect.

The effects of SET or SET LOCAL are also canceled by rolling back to a savepoint that is earlier than the command.

This ticket also includes adding support for the set_config(setting_name, val, is_local) builtin: https://www.postgresql.org/docs/13/functions-admin.html

Epic CRDB-2507

[coco98]

Hi folks! Keen on having this supported. If possible, would love to know when this when this might land (roughly ofcourse).

[knz]

What about you start by explaining why this is important to you:

• why do you need this?
• what is the use case?
• what other options have you explored? what didn't work?

[coco98]

Sure! I'm one of the creators of Hasura and was exploring what it would take to support cockroach. :)

Our access control layer uses SET LOCAL for passing session variables (derived from the HTTP request) as context to some triggers (defined on "updatable" views in postgres).

An option on our end is to not use SET LOCAL, but that would make it harder to some of the underlying Postgres magic.

We can go into more detail if relevant to this issue.

[coco98]

Also, just wanted to add a note to the team that having the error message while connecting with a postgres client to point to the right issue on github is amazingly helpful. Thanks!

[knz]

Thank you for your answer and your kind words!

1. So I think it's unreasonable to focus on this one feature before you've made a larger-scale inventory of all the postgres features that your app/framework uses. Enabling tracing in postgres to create a SQL trace would be the first step, then go through that trace in CockroachDB to see what's supported and what isn't

2. CockroachDB does not support triggers nor updatable views. It would be interesting to explore the "larger picture" and see what the framework is trying to achieve.

3. CockroachDB does not support WITH RECURSIVE (recursive common table expressions, see sql: WITH RECURSIVE (recursive common table expressions) #21085) which are commonly used for graph databases.

Given points 2 and 3, maybe SET LOCAL is not the main concern here. Maybe a better strategy would be to create a dedicated CockroachDB back-end for Hasura.

Let us know what you think!

[coco98]

Thanks for your prompt response and for the breakdown. Will get back on this thread with any relevant thoughts!

[nadilas]

@coco98 @knz has there been any consensus on whether cockroachdb will support hasura or not?

[knz]

@nadilas this is not the right place to discuss hasura support. This issue is specifically about the SET LOCAL statement in SQL.

(You should post a comment on the hasura repository, not here.)

[rafiss]

@nadilas you may be interested in following this issue: hasura/graphql-engine#678

[chriscasano]

FWIW - A customer and myself investigated using PostgREST with CockroachDB. We both ran into this issue today:

{"hint":"You have attempted to use a feature that is not yet implemented.\nSee: https://github.com/cockroachdb/cockroach/issues/32562","details":"source SQL:\nset local schema ''\n ^","code":"0A000","message":"at or near "schema": syntax error: unimplemented: this syntax"}
Attempting to connect to the database...

[aeneasr]

Just wanted to chime in that we are currently hitting this. The problem is that we are trying to apply migrations for several deployments, which are scoped by the search path. The migrations are written in plain SQL without prefixes/search path (e.g. CREATE TABLE foo .... The idea is to run each migration in its own search path as we have no way of customizing the SQL statements/migrations.

Without SET LOCAL, we have to open a new connection with an explicit session-scoped search path, instead of doing:

begin transaction;
set local search_path to scope_1, public;
create table "asdf" ("bar" varchar (14) not null);
commit;
begin transaction;
set local search_path to scope_2, public;
create table "asdf" ("bar" varchar (14) not null);
commit;
begin transaction;
set local search_path to scope_3, public;
create table "asdf" ("bar" varchar (14) not null);
commit;

This causes a lot of overhead and strain on system resources as we need to check if the db connection is healthy, allocate a new pool, block connections on crdb nodes, and so on.

Having SET LOCAL support - especially for the search path - would be tremendously helpful in reducing crdb system load and overall latencies.

[rafiss]

These docs are a useful reference on how Postgres implements this: https://github.com/postgres/postgres/blob/master/src/backend/utils/misc/README

In the "Saving/Restoring GUC Variable Values" section:

Prior values of configuration variables must be remembered in order to deal
with several special cases: RESET (a/k/a SET TO DEFAULT), rollback of SET
on transaction abort, rollback of SET LOCAL at transaction end (either
commit or abort), and save/restore around a function that has a SET option.
RESET is defined as selecting the value that would be effective had there
never been any SET commands in the current session.

To handle these cases we must keep track of many distinct values for each
variable.  The primary values are:

* actual variable contents	always the current effective value

* reset_val			the value to use for RESET

(Each GUC entry also has a boot_val which is the wired-in default value.
This is assigned to the reset_val and the actual variable during
InitializeGUCOptions().  The boot_val is also consulted to restore the
correct reset_val if SIGHUP processing discovers that a variable formerly
specified in postgresql.conf is no longer set there.)

In addition to the primary values, there is a stack of former effective
values that might need to be restored in future.  Stacking and unstacking
is controlled by the GUC "nest level", which is zero when outside any
transaction, one at top transaction level, and incremented for each
open subtransaction or function call with a SET option.  A stack entry
is made whenever a GUC variable is first modified at a given nesting level.
(Note: the reset_val need not be stacked because it is only changed by
non-transactional operations.)

A stack entry has a state, a prior value of the GUC variable, a remembered
source of that prior value, and depending on the state may also have a
"masked" value.  The masked value is needed when SET followed by SET LOCAL
occur at the same nest level: the SET's value is masked but must be
remembered to restore after transaction commit.

During initialization we set the actual value and reset_val based on
whichever non-interactive source has the highest priority.  They will
have the same value.

The possible transactional operations on a GUC value are:

Entry to a function with a SET option:

	Push a stack entry with the prior variable value and state SAVE,
	then set the variable.

Plain SET command:

	If no stack entry of current level:
		Push new stack entry w/prior value and state SET
	else if stack entry's state is SAVE, SET, or LOCAL:
		change stack state to SET, don't change saved value
		(here we are forgetting effects of prior set action)
	else (entry must have state SET+LOCAL):
		discard its masked value, change state to SET
		(here we are forgetting effects of prior SET and SET LOCAL)
	Now set new value.

SET LOCAL command:

	If no stack entry of current level:
		Push new stack entry w/prior value and state LOCAL
	else if stack entry's state is SAVE or LOCAL or SET+LOCAL:
		no change to stack entry
		(in SAVE case, SET LOCAL will be forgotten at func exit)
	else (entry must have state SET):
		put current active into its masked slot, set state SET+LOCAL
	Now set new value.

We may not be able to implement this exactly the same way, since Plain SET commands in CRDB do not respect transaction boundaries [1], but the basic approach might be the one we take.

[1]: What I mean by "respecting transaction boundaries" is this behavior in Postgres:

rafiss@127:postgres> show application_name;
+--------------------+
| application_name   |
|--------------------|
| pgcli              |
+--------------------+
SHOW

rafiss@127:postgres> begin;
BEGIN

rafiss@127:postgres> set application_name = 'a';
SET

rafiss@127:postgres> rollback;
ROLLBACK

rafiss@127:postgres> show application_name;
+--------------------+
| application_name   |
|--------------------|
| pgcli              |
+--------------------+
SHOW

Whereas in CRDB:

root@:26257/defaultdb> show application_name;
  application_name
--------------------
  $ cockroach sql
(1 row)

root@:26257/defaultdb> begin;
BEGIN

root@:26257/defaultdb  OPEN> set application_name = 'a';
SET

root@:26257/defaultdb  OPEN> rollback;
ROLLBACK

root@:26257/defaultdb> show application_name;
  application_name
--------------------
  a
(1 row)

[mgartner]

We recently came across a good use-case for a txn-scoped session settings: turning off distsql for a particular query. Because SET distsql=off does not revert to its previous value in an explicit txn ROLLBACK, the setting could "leak" to subsequent queries if the transaction fails:

[email protected]:26257/defaultdb> SHOW distsql;
  distsql
-----------
  auto
(1 row)

[email protected]:26257/defaultdb> BEGIN;
BEGIN

[email protected]:26257/defaultdb  OPEN> SET distsql=off;
SET

[email protected]:26257/defaultdb  OPEN> SELECT * FROM foo;
ERROR: relation "foo" does not exist
SQLSTATE: 42P01

[email protected]:26257/? ERROR> ROLLBACK;
ROLLBACK

[email protected]:26257/defaultdb> SHOW distsql;
  distsql
-----------
  off

[knz]

We may not be able to implement this exactly the same way, since Plain SET commands in CRDB do not respect transaction boundaries

NB that since we implemented nested transactions, the conn executor now has a stack-like data structure to remember the name of nested transactions. We could thus piggy back this new mechanism to store the txn-scoped session var customizations.

[rafiss]

#69396 is the issue for tracking SET not being transactional.