Merge #45472 #45607 #45609 #45613

45472: storage/engine: Teeing engine fixes r=itsbilal a=itsbilal

This change introduces some misc teeing engine fixes, such as proper
cache initialization, copying of SSTs before ingestions (now that
pebble also deletes SSTs after an Ingest), and better null msg
handling in GetProto. The cache part fixes a segfault in
GetStats.

Fixes #42654 .

Release note: None.

45607: sql: fix usages of ActiveVersionOrEmpty in {create, alter}_table.go r=irfansharif a=rohany

Fixes #45519.

This PR removes an incorrect usage of ActiveVersionOrEmpty in
alter_table.go, and updates a comment in create_table.go detailing its
usage.

Release note: None

45609: sql: support star-expanding label-less tuples + numeric tuple indexing r=RaduBerinde a=knz

Requested by @RaduBerinde  / @ajwerner .

Previously, it was impossible to use `(t).*` to expand a single
tuple-typed column into multiple projections if the tuple was not
labeled to start with.

This limitation was caused by another limitation, that it was not
possible to refer to a single column in a tuple if the tuple was not
already labeled.

This patch addresses both limitations.

Release note (sql change): CockroachDB now supports expanding all
columns of a tuple using the `.*` notation, for example `SELECT (t).*
FROM (SELECT (1,'b',2.3) AS t)`. This is a CockroachDB-specific extension.

Release note (sql change): CockroachDB now supports accessing the Nth
column in a column with tuple type using the syntax `(...).@N`, for
example `SELECT (t).@2 FROM (SELECT (1,'b',2.3) AS t)`. This is a
CockroachDB-specific extension.

45613: pgwire: fix decimal decoding with trailing zeroes r=jordanlewis a=otan

Resolves #25460

In addition to the release note, I have also modified the docker runfile
to support elixir tests, and also updated the elixir tests to be run.

Release note (bug fix): In previous PRs, drivers that do not truncate
trailing zeroes for decimals in the binary format end up having
inaccuracies of up to 10^4 during the decode step. In this PR, we fix
the error by truncating the trailing zeroes as appropriate. This fixes
known incorrect decoding cases with Postgrex in Elixir.




Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>
Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>

docs/generated/sql/bnf/stmt_block.bnf

@@ -1546,6 +1546,7 @@ d_expr ::=
 	| 'PLACEHOLDER'
 	| '(' a_expr ')' '.' '*'
 	| '(' a_expr ')' '.' unrestricted_name
+	| '(' a_expr ')' '.' '@' 'ICONST'
 	| '(' a_expr ')'
 	| func_expr
 	| select_with_parens

pkg/acceptance/adapter_test.go

@@ -55,6 +55,15 @@ func TestDockerJava(t *testing.T) {
 	testDockerFail(ctx, t, "java", []string{"sh", "-c", "cd /mnt/data/java && mvn -o foobar"})
 }
 
+func TestDockerElixir(t *testing.T) {
+	s := log.Scope(t)
+	defer s.Close(t)
+
+	ctx := context.Background()
+	testDockerSuccess(ctx, t, "elixir", []string{"sh", "-c", "cd /mnt/data/elixir/test_crdb && mix local.hex --force && mix deps.get && psql -c 'CREATE DATABASE IF NOT EXISTS testdb' && mix test"})
+	testDockerFail(ctx, t, "elixir", []string{"sh", "-c", "cd /mnt/data/elixir/test_crdb && mix local.hex --force && mix deps.get && mix thisshouldfail"})
+}
+
 func TestDockerNodeJS(t *testing.T) {
 	s := log.Scope(t)
 	defer s.Close(t)

pkg/acceptance/testdata/Dockerfile

@@ -1,4 +1,4 @@
-FROM ubuntu:artful-20170916
+FROM ubuntu:18.04
 
 # This Dockerfile bundles several language runtimes and test frameworks into one
 # container for our acceptance tests.
@@ -49,12 +49,18 @@ RUN apt-get install --yes --no-install-recommends openjdk-8-jdk \
 RUN curl -fsSL https://dl.yarnpkg.com/debian/pubkey.gpg > /etc/apt/trusted.gpg.d/yarn.asc \
  && echo "deb https://dl.yarnpkg.com/debian/ stable main" > /etc/apt/sources.list.d/yarn.list \
  && curl -fsSL https://packages.microsoft.com/keys/microsoft.asc > /etc/apt/trusted.gpg.d/microsoft.asc \
- && echo "deb https://packages.microsoft.com/repos/microsoft-ubuntu-zesty-prod zesty main" > /etc/apt/sources.list.d/microsoft.list \
+ && echo "deb https://packages.microsoft.com/repos/microsoft-ubuntu-bionic-prod/ bionic main" > /etc/apt/sources.list.d/microsoft.list \
+ && apt-get install --yes --no-install-recommends gnupg \
+ && curl https://packages.erlang-solutions.com/erlang-solutions_2.0_all.deb > erlang-solutions_2.0_all.deb && dpkg -i erlang-solutions_2.0_all.deb \
  && apt-get update \
  && apt-get install --yes --no-install-recommends \
-    dotnet-sdk-2.0.0 \
+    dotnet-sdk-2.1 \
+    dotnet-runtime-2.1 \
     expect \
+    elixir \
+    esl-erlang \
     libc6-dev \
+    libcurl4 \
     libpq-dev \
     libpqtypes-dev \
     make \

pkg/acceptance/testdata/elixir/test_crdb/.formatter.exs

@@ -0,0 +1,4 @@
+# Used by "mix format"
+[
+  inputs: ["{mix,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}"]
+]

pkg/acceptance/testdata/elixir/test_crdb/.gitignore

@@ -0,0 +1,24 @@
+# The directory Mix will write compiled artifacts to.
+/_build/
+
+# If you run "mix test --cover", coverage assets end up here.
+/cover/
+
+# The directory Mix downloads your dependencies sources to.
+/deps/
+
+# Where third-party dependencies like ExDoc output generated docs.
+/doc/
+
+# Ignore .fetch files in case you like to edit your project deps locally.
+/.fetch
+
+# If the VM crashes, it generates a dump, let's ignore it too.
+erl_crash.dump
+
+# Also ignore archive artifacts (built via "mix archive.build").
+*.ez
+
+# Ignore package tarball (built via "mix hex.build").
+debug-*.tar
+

pkg/acceptance/testdata/elixir/test_crdb/mix.exs

@@ -0,0 +1,22 @@
+defmodule Cockroach.MixProject do
+  use Mix.Project
+
+  def project do
+    [
+      app: :debug,
+      version: "0.1.0",
+      start_permanent: Mix.env() == :prod,
+      deps: deps()
+    ]
+  end
+
+  def application do
+    [
+      extra_applications: [:logger]
+    ]
+  end
+
+  defp deps do
+    [{:postgrex, "~> 0.13", hex: :postgrex_cdb, override: true}]
+  end
+end

pkg/acceptance/testdata/elixir/test_crdb/mix.lock

@@ -0,0 +1,6 @@
+%{
+  "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], [], "hexpm", "4a0850c9be22a43af9920a71ab17c051f5f7d45c209e40269a1938832510e4d9"},
+  "db_connection": {:hex, :db_connection, "1.1.3", "89b30ca1ef0a3b469b1c779579590688561d586694a3ce8792985d4d7e575a61", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm", "5f0a16a58312a610d5eb0b07506280c65f5137868ad479045f2a2dc4ced80550"},
+  "decimal": {:hex, :decimal, "1.8.1", "a4ef3f5f3428bdbc0d35374029ffcf4ede8533536fa79896dd450168d9acdf3c", [:mix], [], "hexpm", "3cb154b00225ac687f6cbd4acc4b7960027c757a5152b369923ead9ddbca7aec"},
+  "postgrex": {:hex, :postgrex_cdb, "0.13.5", "83f818ea1b959d176c694bb60c36f42080c36a7413f0d3b05d58ef2093fe17f5", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm", "1a01ba25472ad33bafbac6f042fde2dbab93e23bdaa49ffa3722926165c1052f"},
+}

pkg/acceptance/testdata/elixir/test_crdb/test/decimal_test.exs

@@ -0,0 +1,63 @@
+defmodule Cockroach.TestDecimal do
+  use ExUnit.Case
+
+  test "handles decimal correctly" do
+    ssl_opts = [certfile: System.get_env("PGSSLCERT"), keyfile: System.get_env("PGSSLKEY")]
+    {port, _} = Integer.parse(System.get_env("PGPORT") || "26257")
+    {start_ok, pid} = Postgrex.start_link(
+      hostname: System.get_env("PGHOST") || "localhost",
+      username: System.get_env("PGUSER") || "root",
+      password: "",
+      database: "testdb",
+      port: port,
+      ssl: (System.get_env("PGSSLCERT") != nil && true) || false,
+      ssl_opts: ssl_opts
+    )
+    assert start_ok
+    for dec <- [
+      Decimal.new("0"),
+      Decimal.new("0.0"),
+      Decimal.new("0.00"),
+      Decimal.new("0.000"),
+      Decimal.new("0.0000"),
+      Decimal.new("0.00000"),
+      Decimal.new("0.00001"),
+      Decimal.new("0.000012"),
+      Decimal.new("0.0000012"),
+      Decimal.new("0.00000012"),
+      Decimal.new("0.00000012"),
+      Decimal.new(".00000012"),
+      Decimal.new("1"),
+      Decimal.new("1.0"),
+      Decimal.new("1.000"),
+      Decimal.new("1.0000"),
+      Decimal.new("1.00000"),
+      Decimal.new("1.000001"),
+      Decimal.new("12345"),
+      Decimal.new("12345.0"),
+      Decimal.new("12345.000"),
+      Decimal.new("12345.0000"),
+      Decimal.new("12345.00000"),
+      Decimal.new("12345.000001"),
+      Decimal.new("12340"),
+      Decimal.new("123400"),
+      Decimal.new("1234000"),
+      Decimal.new("12340000"),
+      Decimal.new("12340.00"),
+      Decimal.new("123400.00"),
+      Decimal.new("1234000.00"),
+      Decimal.new("12340000.00"),
+      Decimal.new("1.2345e-10"),
+      Decimal.new("1.23450e-10"),
+      Decimal.new("1.234500e-10"),
+      Decimal.new("1.2345000e-10"),
+      Decimal.new("1.23450000e-10"),
+      Decimal.new("1.234500000e-10"),
+    ] do
+      {result_ok, result} = Postgrex.query(pid, "SELECT CAST($1 AS DECIMAL)", [dec])
+      assert result_ok
+      [[ret]] = result.rows
+      assert ret == dec
+    end
+  end
+end

pkg/acceptance/testdata/elixir/test_crdb/test/test_helper.exs

@@ -0,0 +1 @@
+ExUnit.start()

pkg/acceptance/util_docker.go

@@ -58,7 +58,7 @@ func testDockerSuccess(ctx context.Context, t *testing.T, name string, cmd []str
 const (
 	// Iterating against a locally built version of the docker image can be done
 	// by changing acceptanceImage to the hash of the container.
-	acceptanceImage = "docker.io/cockroachdb/acceptance:20180416-090309"
+	acceptanceImage = "docker.io/cockroachdb/acceptance:20200303-091324"
 )
 
 func testDocker(

pkg/sql/alter_table.go

@@ -347,8 +347,7 @@ func (n *alterTableNode) startExec(params runParams) error {
 
 		case *tree.AlterTableAlterPrimaryKey:
 			// Make sure that all nodes in the cluster are able to perform primary key changes before proceeding.
-			version := params.p.ExecCfg().Settings.Version.ActiveVersionOrEmpty(params.ctx)
-			if !version.IsActive(clusterversion.VersionPrimaryKeyChanges) {
+			if !params.p.ExecCfg().Settings.Version.IsActive(params.ctx, clusterversion.VersionPrimaryKeyChanges) {
 				return pgerror.Newf(pgcode.FeatureNotSupported,
 					"all nodes are not the correct version for primary key changes")
 			}
@@ -359,7 +358,7 @@ func (n *alterTableNode) startExec(params runParams) error {
 			}
 
 			if t.Sharded != nil {
-				if !version.IsActive(clusterversion.VersionHashShardedIndexes) {
+				if !params.p.ExecCfg().Settings.Version.IsActive(params.ctx, clusterversion.VersionHashShardedIndexes) {
 					return invalidClusterForShardedIndexError
 				}
 				if !params.p.EvalContext().SessionData.HashShardedIndexesEnabled {

pkg/sql/create_table.go

@@ -1181,14 +1181,12 @@ func MakeTableDesc(
 	// If all nodes in the cluster know how to handle secondary indexes with column families,
 	// write the new version into new index descriptors.
 	indexEncodingVersion := sqlbase.BaseIndexFormatVersion
-	// We can't use st.Version.IsActive because this method is sometimes called
-	// before the version has been initialized, leading to a panic. There are also
-	// cases where this function is called in tests where st is nil.
-	if st != nil {
-		if version := st.Version.ActiveVersionOrEmpty(ctx); version != (clusterversion.ClusterVersion{}) &&
-			version.IsActive(clusterversion.VersionSecondaryIndexColumnFamilies) {
-			indexEncodingVersion = sqlbase.SecondaryIndexFamilyFormatVersion
-		}
+	// We can't use st.Version.IsActive because this method is used during
+	// server setup before the cluster version has been initialized.
+	version := st.Version.ActiveVersionOrEmpty(ctx)
+	if version != (clusterversion.ClusterVersion{}) &&
+		version.IsActive(clusterversion.VersionSecondaryIndexColumnFamilies) {
+		indexEncodingVersion = sqlbase.SecondaryIndexFamilyFormatVersion
 	}
 
 	for i, def := range n.Defs {
@@ -1210,7 +1208,7 @@ func MakeTableDesc(
 				if st == nil {
 					return desc, invalidClusterForShardedIndexError
 				}
-				if version := st.Version.ActiveVersionOrEmpty(ctx); version == (clusterversion.ClusterVersion{}) ||
+				if version == (clusterversion.ClusterVersion{}) ||
 					!version.IsActive(clusterversion.VersionHashShardedIndexes) {
 					return desc, invalidClusterForShardedIndexError
 				}

pkg/sql/execinfra/server_config.go

@@ -60,7 +60,7 @@ import (
 //
 // ATTENTION: When updating these fields, add to version_history.txt explaining
 // what changed.
-const Version execinfrapb.DistSQLVersion = 25
+const Version execinfrapb.DistSQLVersion = 26
 
 // MinAcceptedVersion is the oldest version that the server is
 // compatible with; see above.

pkg/sql/logictest/testdata/logic_test/srfs

@@ -1083,8 +1083,12 @@ unnest
 (1,2)
 (3,4)
 
-query error pq: type tuple{int, int} is not composite
+query II colnames
 SELECT (unnest(ARRAY[(1,2),(3,4)])).*
+----
+?column?  ?column?
+1         1
+3         3
 
 query T colnames
 SELECT * FROM unnest(ARRAY[(1,2),(3,4)])

pkg/sql/logictest/testdata/logic_test/tuple

@@ -825,8 +825,17 @@ SELECT ((unnest(ARRAY[1,2], ARRAY[1,2]))).unnest;
 query error pq: type tuple{int, int, int} is not composite
 SELECT ((1,2,3)).x FROM tb
 
-query error pq: type tuple{int, int, int} is not composite
+query I colnames
+SELECT ((1,2,3)).@2 FROM tb
+----
+?column?
+2
+
+query III colnames
 SELECT ((1,2,3)).* FROM tb
+----
+?column?  ?column?  ?column?
+1         2         3
 
 # Accessing all the columns
 

pkg/sql/opt/exec/execbuilder/scalar.go

@@ -348,6 +348,7 @@ func (b *Builder) buildColumnAccess(
 	}
 	childTyp := colAccess.Input.DataType()
 	colIdx := int(colAccess.Idx)
+	// Find a label if there is one. It's OK if there isn't.
 	lbl := ""
 	if childTyp.TupleLabels() != nil {
 		lbl = childTyp.TupleLabels()[colIdx]

pkg/sql/opt/norm/fold_constants.go

@@ -313,8 +313,7 @@ func (c *CustomFuncs) FoldColumnAccess(input opt.ScalarExpr, idx memo.TupleOrdin
 	if memo.CanExtractConstDatum(input) {
 		datum := memo.ExtractConstDatum(input)
 
-		colName := input.DataType().TupleLabels()[idx]
-		texpr := tree.NewTypedColumnAccessExpr(datum, colName, int(idx))
+		texpr := tree.NewTypedColumnAccessExpr(datum, "" /* by-index access */, int(idx))
 		result, err := texpr.Eval(c.f.evalCtx)
 		if err == nil {
 			return c.f.ConstructConstVal(result, texpr.ResolvedType())

pkg/sql/opt/optbuilder/testdata/srfs

@@ -583,11 +583,30 @@ project
  └── projections
       └── (((x:1, n:2) AS x, n)).x [as=x:3]
 
+build
+SELECT (information_schema._pg_expandarray(ARRAY['c', 'b', 'a'])).@2
+----
+project
+ ├── columns: "?column?":3
+ ├── project-set
+ │    ├── columns: x:1 n:2
+ │    ├── values
+ │    │    └── ()
+ │    └── zip
+ │         └── information_schema._pg_expandarray(ARRAY['c','b','a'])
+ └── projections
+      └── (((x:1, n:2) AS x, n)).n [as="?column?":3]
+
 build
 SELECT (information_schema._pg_expandarray(ARRAY['c', 'b', 'a'])).other
 ----
 error (42804): could not identify column "other" in tuple{string AS x, int AS n}
 
+build
+SELECT (information_schema._pg_expandarray(ARRAY['c', 'b', 'a'])).@4
+----
+error (42601): tuple column 4 does not exist
+
 build
 SELECT temp.n from information_schema._pg_expandarray(ARRAY['c','b','a']) AS temp;
 ----

pkg/sql/opt/optbuilder/util.go

@@ -62,7 +62,7 @@ func (b *Builder) expandStar(
 	case *tree.TupleStar:
 		texpr := inScope.resolveType(t.Expr, types.Any)
 		typ := texpr.ResolvedType()
-		if typ.Family() != types.TupleFamily || typ.TupleLabels() == nil {
+		if typ.Family() != types.TupleFamily {
 			panic(tree.NewTypeIsNotCompositeError(typ))
 		}
 
@@ -88,14 +88,27 @@ func (b *Builder) expandStar(
 		tTuple, isTuple := texpr.(*tree.Tuple)
 
 		aliases = typ.TupleLabels()
+		for i := len(aliases); i < len(typ.TupleContents()); i++ {
+			// Add aliases for all the non-named columns in the tuple.
+			aliases = append(aliases, "?column?")
+		}
 		exprs = make([]tree.TypedExpr, len(typ.TupleContents()))
 		for i := range typ.TupleContents() {
 			if isTuple {
 				// De-tuplify: ((a,b,c)).* -> a, b, c
 				exprs[i] = tTuple.Exprs[i].(tree.TypedExpr)
 			} else {
-				// Can't de-tuplify: (Expr).* -> (Expr).a, (Expr).b, (Expr).c
-				exprs[i] = tree.NewTypedColumnAccessExpr(texpr, typ.TupleLabels()[i], i)
+				// Can't de-tuplify:
+				// either (Expr).* -> (Expr).a, (Expr).b, (Expr).c if there are enough labels,
+				// or (Expr).* -> (Expr).@1, (Expr).@2, (Expr).@3 if labels are missing.
+				//
+				// We keep the labels if available so that the column name
+				// generation still produces column label "x" for, e.g. (E).x.
+				colName := ""
+				if i < len(typ.TupleContents()) {
+					colName = aliases[i]
+				}
+				exprs[i] = tree.NewTypedColumnAccessExpr(texpr, colName, i)
 			}
 		}
 

pkg/sql/parser/parse_test.go

@@ -665,6 +665,7 @@ func TestParse(t *testing.T) {
 		{`SELECT (() AS a)`},
 		{`SELECT ((() AS a)).a`},
 		{`SELECT ((() AS a)).*`},
+		{`SELECT ((() AS a)).@1`},
 		{`SELECT (TABLE a)`},
 		{`SELECT 0x1`},
 		{`SELECT 'Deutsch' COLLATE de`},
@@ -1057,10 +1058,12 @@ func TestParse(t *testing.T) {
 		{`DELETE FROM [123 AS t]@idx`},
 
 		{`SELECT (1 + 2).*`},
+		{`SELECT (1 + 2).@1`},
 		{`SELECT (1 + 2).col`},
 		{`SELECT (abc.def).col`},
 		{`SELECT (i.keys).col`},
 		{`SELECT (i.keys).*`},
+		{`SELECT (i.keys).@1`},
 		{`SELECT (ARRAY['a', 'b', 'c']).name`},
 
 		{`SELECT 1 FOR UPDATE`},

pkg/sql/parser/sql.y

@@ -8372,6 +8372,12 @@ d_expr:
   {
     $$.val = &tree.ColumnAccessExpr{Expr: $2.expr(), ColName: $5 }
   }
+| '(' a_expr ')' '.' '@' ICONST
+  {
+    idx, err := $6.numVal().AsInt32()
+    if err != nil || idx <= 0 { return setErr(sqllex, err) }
+    $$.val = &tree.ColumnAccessExpr{Expr: $2.expr(), ByIndex: true, ColIndex: int(idx-1)}
+  }
 | '(' a_expr ')'
   {
     $$.val = &tree.ParenExpr{Expr: $2.expr()}