Merge #104859 #107920 #108923

104859: pkg/server: initialize tenant version setting in settings watcher r=knz a=healthy-pod

This code change removes the early version setting initialization
because it is not needed.

Initialization now happens in the settings watcher during the initial
scan when the tenant logical version is read from the settings table.

The setting is only initialized if it's empty / not pre-initialized
because some tests pre-initialize it.

Release note: None
Epic: CRDB-26691

107920: plpgsql: add support for sql exec and INTO statements r=DrewKimball a=DrewKimball

#### plpgsql: add parser support for sql exec statements

This patch adds support in the parser for directly executing SQL statements
in a PLpgSQL routine. Note that postgres disallows this syntax for statements
that return rows, but we allow it because execution can handle row-returning
statements and it allows running statements like `IMPORT`, which returns rows.

SQL statements with an `INTO` clause are also supported; `INTO` allows for
the result of a SQL statement to be assigned to one or more PLpgSQL
variables. It cannot be used with a statement that does not return rows.

Example:
```
CREATE FUNCTION f() RETURNS INT AS $$
  DECLARE
    i INT;
  BEGIN
    SET testing_optimizer_disable_rule_probability = 0;
    INSERT INTO xy VALUES (1, 2);
    SELECT 1 + 1;
    SELECT 100 INTO i;
    SELECT max(x) INTO i FROM xy;
    INSERT INTO xy VALUES (10, 10) RETURNING x INTO i;
    RETURN i;
  END
$$ LANGUAGE PLpgSQL;
```

Informs #105252

Release note: None

#### plpgsql: raise end-of-function error during runtime

This commit moves the end-of-function error from compile-time to
run-time, using a RAISE statement to throw the error. This more
closely mirrors postgres behavior, where it's possible to define
a function that is not guaranteed to terminate. Example:
```
CREATE OR REPLACE FUNCTION f(a INT, b INT) RETURNS INT AS $$
  BEGIN
    IF a < b THEN
      RETURN a;
    END IF;
  END
$$ LANGUAGE PLpgSQL;

root@localhost:26257/defaultdb> SELECT f(1, 2);
  f
-----
  1
(1 row)

Time: 25ms total (execution 24ms / network 0ms)

root@localhost:26257/defaultdb> SELECT f(2, 1);
ERROR: control reached end of function without RETURN
SQLSTATE: 2F005
```

This commit also refactors some of the PLpgSQL routine-building logic
to simplify building multiple body statements.

Epic: CRDB-799

Release note: None

#### plpgsql: add support for sql exec and INTO statements

This patch adds execution support for executing SQL statements within
PLpgSQL routines, both for the case when the result is discarded and when
`INTO` syntax is used to assign the result to a set of target variables.
The former case is handled by building the SQL statement into one of the
first body statements of a routine; this captures the result-discarding
behavior, but ensures any side effects still occur. The latter case is
handled by aliasing the output columns of the SQL statement to the target
variable names; this is done in the last body statement of the routine,
since the result of the query is needed.

Examples:
```
CREATE OR REPLACE FUNCTION f() RETURNS INT AS $$
  BEGIN
    INSERT INTO xy VALUES (1000, 1000);
    DELETE FROM xy WHERE x = 1000;
    RETURN 0;
  END
$$ LANGUAGE PLpgSQL;

CREATE OR REPLACE FUNCTION f() RETURNS INT AS $$
  DECLARE
    i INT;
  BEGIN
    SELECT x INTO i FROM xy ORDER BY x DESC;
    RETURN i;
  END
$$ LANGUAGE PLpgSQL;
```

Fixes #105252

Release note (sql change): Added support for executing SQL statements
directly within PLpgSQL routines. Note that this currently only applies
to the subset of statements that can be executed within SQL UDFs, so
`CREATE TABLE` is not supported, for example. INTO syntax is also
supported, e.g. `SELECT * INTO a, b FROM xy;`.

108923: compose/compare: Add PG compat session variables r=chrisseto a=chrisseto

Previously, `TestComposeCompare` would fail due to differences in how floats were cast to strings and the default behavior of NULL ordering. This commit sets PG compatibility session variables for the CockroachDB instances used in this test.

This commit only fixes failures when:
1. A float is being cast to a string (`SELECT 1234567::FLOAT8::STRING`)
2. A `SELECT` contains an `ORDER BY` _without_ specifying how `NULLS` should be handled.

There are still other known and unknown failures.

Epic: None
Informs: #99181

Co-authored-by: healthy-pod <ahmad@cockroachlabs.com>
Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
Co-authored-by: Chris Seto <chriskseto@gmail.com>

Author: 4 people

pkg/cli/mt_start_sql.go

@@ -14,7 +14,6 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/server/serverctl"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
@@ -60,23 +59,7 @@ func runStartSQL(cmd *cobra.Command, args []string) error {
 	const serverType redact.SafeString = "SQL server"
 
 	initConfig := func(ctx context.Context) error {
-		if err := serverCfg.InitSQLServer(ctx); err != nil {
-			return err
-		}
-
-		// We need a value in the version setting prior to the update
-		// coming from the system.settings table. This value must be valid
-		// and compatible with the state of the tenant's keyspace.
-		//
-		// Since we don't know at which binary version the tenant
-		// keyspace was initialized, we must be conservative and
-		// assume it was created a long time ago; and that we may
-		// have to run all known migrations since then. So initialize
-		// the version setting to the minimum supported version.
-		st := serverCfg.BaseConfig.Settings
-		return clusterversion.Initialize(
-			ctx, st.Version.BinaryMinSupportedVersion(), &st.SV,
-		)
+		return serverCfg.InitSQLServer(ctx)
 	}
 
 	newServerFn := func(ctx context.Context, serverCfg server.Config, stopper *stop.Stopper) (serverctl.ServerStartupInterface, error) {

pkg/compose/compare/compare/compare_test.go

@@ -65,6 +65,8 @@ func TestCompare(t *testing.T) {
 			addr: "postgresql://root@cockroach1:26257/postgres?sslmode=disable",
 			init: []string{
 				"SET CLUSTER SETTING cluster.organization = 'Cockroach Labs - Production Testing'",
+				"SET extra_float_digits = 0",   // For Postgres Compat when casting floats to strings.
+				"SET null_ordered_last = true", // For Postgres Compat, see https://www.cockroachlabs.com/docs/stable/order-by#parameters
 				fmt.Sprintf("SET CLUSTER SETTING enterprise.license = '%s'", license),
 				"drop database if exists postgres",
 				"create database postgres",
@@ -74,6 +76,8 @@ func TestCompare(t *testing.T) {
 			addr: "postgresql://root@cockroach2:26257/postgres?sslmode=disable",
 			init: []string{
 				"SET CLUSTER SETTING cluster.organization = 'Cockroach Labs - Production Testing'",
+				"SET extra_float_digits = 0",   // For Postgres Compat when casting floats to strings.
+				"SET null_ordered_last = true", // For Postgres Compat https://www.cockroachlabs.com/docs/stable/order-by#parameters
 				fmt.Sprintf("SET CLUSTER SETTING enterprise.license = '%s'", license),
 				"drop database if exists postgres",
 				"create database postgres",

pkg/server/settingswatcher/settings_watcher.go

@@ -339,15 +339,26 @@ func (s *SettingsWatcher) setLocked(
 	// tenant, or because the new version <= old version).
 	if key == versionSettingKey && !s.codec.ForSystemTenant() {
 		var newVersion clusterversion.ClusterVersion
-		oldVersion := s.settings.Version.ActiveVersion(ctx)
+		oldVersion := s.settings.Version.ActiveVersionOrEmpty(ctx)
 		if err := protoutil.Unmarshal([]byte(val.Value), &newVersion); err != nil {
 			log.Warningf(ctx, "failed to set cluster version: %s", err.Error())
 		} else if newVersion.LessEq(oldVersion.Version) {
 			// Nothing to do.
-		} else if err := s.settings.Version.SetActiveVersion(ctx, newVersion); err != nil {
-			log.Warningf(ctx, "failed to set cluster version: %s", err.Error())
-		} else if newVersion != oldVersion {
-			log.Infof(ctx, "set cluster version from %v to: %v", oldVersion, newVersion)
+		} else {
+			// Check if cluster version setting is initialized. If it is empty then it is not
+			// initialized.
+			if oldVersion.Version.Equal(roachpb.Version{}) {
+				// Cluster version setting not initialized.
+				if err := clusterversion.Initialize(ctx, newVersion.Version, &s.settings.SV); err != nil {
+					log.Fatalf(ctx, "failed to initialize cluster version setting: %s", err.Error())
+					return
+				}
+			}
+			if err := s.settings.Version.SetActiveVersion(ctx, newVersion); err != nil {
+				log.Warningf(ctx, "failed to set cluster version: %s", err.Error())
+			} else {
+				log.Infof(ctx, "set cluster version from %v to: %v", oldVersion, newVersion)
+			}
 		}
 		return
 	}

pkg/server/testserver.go

@@ -1406,18 +1406,6 @@ func (ts *testServer) StartTenant(
 	if st == nil {
 		st = cluster.MakeTestingClusterSettings()
 	}
-	// Verify that the settings object that was passed in has
-	// initialized the version setting. This is pretty much necessary
-	// for secondary tenants. See the comments at the beginning of
-	// `runStartSQL()` in cli/mt_start_sql.go and
-	// `makeSharedProcessTenantServerConfig()` in
-	// server_controller_new_server.go.
-	//
-	// The version is initialized in MakeTestingClusterSettings(). This
-	// assertion is there to prevent inadvertent changes to
-	// MakeTestingClusterSettings() and as a guardrail for tests that
-	// pass a custom params.Settings.
-	clusterversion.AssertInitialized(ctx, &st.SV)
 
 	// Needed for backward-compat on crdb_internal.ranges{_no_leases}.
 	// Remove in v23.2.