Merge #118924 #118930

118924: toolchains: make macOS toolchain remote-friendly r=rail a=rickystewart

Some bits and pieces were missing here that prevent builds from succeeding remotely.

Epic: CRDB-8308
Release note: None

118930: sql: guard against non-public mutation columns in CREATE STATISTICS r=rafiss a=michae2

Defensively guard against non-public mutation columns when building statistics requests for CREATE STATISTICS / ANALYZE.

Fixes: #118537

Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>

Author: 3 people

build/toolchains/darwin/BUILD.tmpl

@@ -33,7 +33,9 @@ filegroup(
     srcs = [
         "bin/%{target}-apple-darwin21.2-cc",
         "bin/%{target}-apple-darwin21.2-c++",
-    ],
+    ] + glob(
+        ["SDK/**"], exclude=["SDK/MacOSX12.1.sdk/usr/share/**"]
+    ) + glob(["lib/**"]),
 )
 
 filegroup(
@@ -47,6 +49,7 @@ filegroup(
     name = "linker_files",
     srcs = [
         "bin/%{target}-apple-darwin21.2-cc",
+        "bin/%{target}-apple-darwin21.2-ld",
         "bin/xcrun",
     ],
 )

build/toolchains/darwin/cc_toolchain_config.bzl.tmpl

@@ -112,7 +112,6 @@ def _impl(ctx):
                         flags = [
                             "-g1",
                             "-Wall",
-                            "-I%{repo_path}/SDK/MacOSX12.1.sdk/usr/include/c++/v1",
                         ],
                     ),
                 ]),
@@ -122,7 +121,7 @@ def _impl(ctx):
 
     linker_flags = [
         "-lstdc++",
-        "-L%{repo_path}/lib",
+        "-Lexternal/%{repo_name}/lib",
     ]
 
     default_linker_flags = feature(
@@ -138,7 +137,7 @@ def _impl(ctx):
             env_set(
                 actions = all_link_actions,
                 env_entries = [
-                    env_entry(key = "LD_LIBRARY_PATH", value = "%{repo_path}/lib"),
+                    env_entry(key = "LD_LIBRARY_PATH", value = "external/%{repo_name}/lib"),
                 ],
             ),
 	]
@@ -204,7 +203,7 @@ def _impl(ctx):
             "%sysroot%/usr/include",
             "/usr/lib/llvm-10/lib/clang/10.0.0/include",
         ],
-        builtin_sysroot = "%{repo_path}/SDK/MacOSX12.1.sdk",
+        builtin_sysroot = "external/%{repo_name}/SDK/MacOSX12.1.sdk",
     )
 
 cc_toolchain_config = rule(

build/toolchains/darwin/toolchain.bzl

@@ -11,8 +11,6 @@ def _impl(rctx):
         stripPrefix = "x-tools/x86_64-apple-darwin21.2/",
     )
 
-    repo_path = str(rctx.path(""))
-
     rctx.template(
         "BUILD",
         Label("@com_github_cockroachdb_cockroach//build:toolchains/darwin/BUILD.tmpl"),
@@ -27,7 +25,7 @@ def _impl(rctx):
         Label("@com_github_cockroachdb_cockroach//build:toolchains/darwin/cc_toolchain_config.bzl.tmpl"),
         substitutions = {
             "%{host}": rctx.attr.host,
-            "%{repo_path}": repo_path,
+            "%{repo_name}": rctx.name,
             "%{target}": rctx.attr.target,
         },
         executable = False,

pkg/sql/create_stats.go

@@ -420,6 +420,12 @@ func createStatsDefaultColumns(
 			return err
 		}
 
+		// There shouldn't be any non-public columns, but defensively skip over them
+		// if there are.
+		if !col.Public() {
+			return nil
+		}
+
 		// Skip unsupported virtual computed columns.
 		if isUnsupportedVirtual(col) {
 			return nil
@@ -466,9 +472,30 @@ func createStatsDefaultColumns(
 			continue
 		}
 
-		colIDs := make([]descpb.ColumnID, i+1)
+		colIDs := make([]descpb.ColumnID, 0, i+1)
 		for j := 0; j <= i; j++ {
-			colIDs[j] = desc.GetPrimaryIndex().GetKeyColumnID(j)
+			col, err := catalog.MustFindColumnByID(desc, desc.GetPrimaryIndex().GetKeyColumnID(j))
+			if err != nil {
+				return nil, err
+			}
+
+			// There shouldn't be any non-public columns, but defensively skip over
+			// them if there are.
+			if !col.Public() {
+				continue
+			}
+
+			// Skip unsupported virtual computed columns.
+			if isUnsupportedVirtual(col) {
+				continue
+			}
+			colIDs = append(colIDs, col.GetID())
+		}
+
+		// Do not attempt to create multi-column stats with < 2 columns. This can
+		// happen when an index contains only virtual computed columns.
+		if len(colIDs) < 2 {
+			continue
 		}
 
 		// Remember the requested stats so we don't request duplicates.
@@ -503,16 +530,23 @@ func createStatsDefaultColumns(
 				if err != nil {
 					return nil, err
 				}
+
+				// There shouldn't be any non-public columns, but defensively skip them
+				// if there are.
+				if !col.Public() {
+					continue
+				}
+
 				// Skip unsupported virtual computed columns.
 				if isUnsupportedVirtual(col) {
 					continue
 				}
 				colIDs = append(colIDs, col.GetID())
 			}
 
-			// Do not attempt to create multi-column stats with no columns. This
-			// can happen when an index contains only virtual computed columns.
-			if len(colIDs) == 0 {
+			// Do not attempt to create multi-column stats with < 2 columns. This can
+			// happen when an index contains only virtual computed columns.
+			if len(colIDs) < 2 {
 				continue
 			}
 

pkg/sql/logictest/testdata/logic_test/distsql_stats

@@ -2670,3 +2670,38 @@ upper_bound            range_rows  distinct_range_rows  equal_rows
 
 statement ok
 RESET enable_create_stats_using_extremes
+
+# Regression test for #118537. Do not create stats on non-public mutation
+# columns.
+statement ok
+CREATE TABLE t118537 (
+  a INT,
+  PRIMARY KEY (a) USING HASH WITH (bucket_count = 3)
+)
+
+statement ok
+INSERT INTO t118537 SELECT generate_series(0, 9)
+
+statement ok
+SET CLUSTER SETTING jobs.debug.pausepoints = 'newschemachanger.before.exec'
+
+skipif config local-legacy-schema-changer
+statement error job \d+ was paused before it completed with reason: pause point "newschemachanger.before.exec" hit
+ALTER TABLE t118537 ALTER PRIMARY KEY USING COLUMNS (a) USING HASH
+
+statement ok
+CREATE STATISTICS mutation FROM t118537
+
+query TTIB colnames
+SELECT statistics_name, column_names, row_count, histogram_id IS NOT NULL AS has_histogram
+FROM [SHOW STATISTICS FOR TABLE t118537]
+ORDER BY statistics_name, column_names::STRING
+----
+statistics_name  column_names  row_count  has_histogram
+mutation         {a}           10         true
+
+statement ok
+SET CLUSTER SETTING jobs.debug.pausepoints = ''
+
+statement ok
+RESUME JOB (SELECT job_id FROM crdb_internal.jobs WHERE description LIKE 'ALTER TABLE %t118537 ALTER PRIMARY KEY USING COLUMNS (a) USING HASH' AND status = 'paused' FETCH FIRST 1 ROWS ONLY)