Merge #73762 #73819 #73832

73762: opt: eliminate assignment casts with identical source and target types r=mgartner a=mgartner

#### sql: remove type modifiers from placeholder types

The width and precision of placeholder values are not known during
PREPARE, therefore we remove any type modifiers from placeholder types
during type checking so that a value of any width will fit within the
palceholder type.

Note that this change is similar to the changes made in #70722 to
placeholder type checking that were later reverted in #72793. In #70722
the type OIDs of placeholders could be altered, e.g., a placeholder
originally with type `INT2` would be converted to an `INT`. In this
commit, type OIDs of placeholders are not changing, only type modifiers
are.

This commit should allow some logic added in #72793 to be simplified or
removed entirely.

Release note: None

#### opt: eliminate assignment casts with identical source and target types

The `EliminateAssignmentCast` rule has been combined with the
`EliminateCast` rule. Now an assignment cast is eliminated if the source
and target type are identical. This now possible thanks to some changes
to type resolution, including:

  1. Placeholder types are resolved with unspecified type modifiers.
     This ensures that assignment casts won't be eliminated if the a
     placeholder value does not conform to the target type's modifiers.

  2. When constant integer `NumVal`s are resolved as a typed-value,
     they are validated to ensure they fit within their type's width.
     There may be more types we need to perform similar validation for,
     such as floats (see #73743).

  3. Columns in values expressions with values that have non-identical
     types but the same type OID will be typed with type modifiers. For
     example, if a values expression has a CHAR(1) value and a CHAR(3)
     value in the same column, the column will be typed as a CHAR
     without an explicit width.

  4. Type modifiers are now correctly omitted from array content types
     when arrays contain constants.

Fixes #73450

Release note (bug fix): A bug has been fixed that caused incorrect
evaluation of placeholder values in EXECUTE statements. The bug
presented when the PREPARE statement cast a placeholder value, for
example `PREPARE s AS SELECT $1::INT2`. If the assigned value for `$1`
exceeded the maximum width value of the cast target type, the result
value of the cast could be incorrect. This bug has been present since
version 19.1 or earlier.


73819: bazel: upgrade `rules_foreign_cc` to 0.7 r=rail a=rickystewart

Also add `-fcommon` to compile flags for `krb5`.

Closes #71306.

Release note: None

73832: sql/opt/exec: output index/expiry in EXPLAIN SPLIT/RELOCATE statements r=RaduBerinde a=knz

Release note (sql change): The output of `EXPLAIN ALTER INDEX/TABLE
... RELOCATE/SPLIT` now includes the target table/index name and, for
the SPLIT AT variants, the expiry timestamp.

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>

.bazelrc

@@ -1,6 +1,6 @@
 try-import %workspace%/.bazelrc.user
 
-build --symlink_prefix=_bazel/ --ui_event_filters=-DEBUG --define gotags=bazel,gss --experimental_proto_descriptor_sets_include_source_info --incompatible_strict_action_env
+build --symlink_prefix=_bazel/ --ui_event_filters=-DEBUG --define gotags=bazel,gss --experimental_proto_descriptor_sets_include_source_info --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution
 test --config=test
 build:with_ui --define cockroach_with_ui=y
 build:test --define crdb_test=y
@@ -20,26 +20,19 @@ test:ci --test_output=errors
 test:ci --test_tmpdir=/artifacts/tmp
 
 build:cross --stamp
-build:cross --host_crosstool_top=@toolchain_cross_x86_64-unknown-linux-gnu//:suite
 build:cross --define cockroach_cross=y
 
-# cross-compilation configurations. Add e.g. --config=crosslinux to turn these on
-# TODO(ricky): Having to specify both the `platform` and the `crosstool_top` is
-# weird, but I think `rules_foreign_cc` doesn't play too nicely with `--platforms`?
+# cross-compilation configurations. Add e.g. --config=crosslinux to turn these on.
 build:crosslinux --platforms=//build/toolchains:cross_linux
-build:crosslinux --crosstool_top=@toolchain_cross_x86_64-unknown-linux-gnu//:suite
 build:crosslinux '--workspace_status_command=./build/bazelutil/stamp.sh x86_64-pc-linux-gnu'
 build:crosslinux --config=cross
 build:crosswindows --platforms=//build/toolchains:cross_windows
-build:crosswindows --crosstool_top=@toolchain_cross_x86_64-w64-mingw32//:suite
 build:crosswindows '--workspace_status_command=./build/bazelutil/stamp.sh x86_64-w64-mingw32'
 build:crosswindows --config=cross
 build:crossmacos --platforms=//build/toolchains:cross_macos
-build:crossmacos --crosstool_top=@toolchain_cross_x86_64-apple-darwin19//:suite
 build:crossmacos '--workspace_status_command=./build/bazelutil/stamp.sh x86_64-apple-darwin19'
 build:crossmacos --config=cross
 build:crosslinuxarm --platforms=//build/toolchains:cross_linux_arm
-build:crosslinuxarm --crosstool_top=@toolchain_cross_aarch64-unknown-linux-gnu//:suite
 build:crosslinuxarm '--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu'
 build:crosslinuxarm --config=cross
 

WORKSPACE

@@ -338,10 +338,11 @@ c_deps()
 # aforementioned PRs.
 http_archive(
     name = "rules_foreign_cc",
-    sha256 = "45d48c71f1b1eadc33a5ad15bbeca8ce42f9ef5ab5d7ee519fc4991e6a9e93d2",
-    strip_prefix = "cockroachdb-rules_foreign_cc-f1cff45",
+    sha256 = "272ac2cde4efd316c8d7c0140dee411c89da104466701ac179286ef5a89c7b58",
+    strip_prefix = "cockroachdb-rules_foreign_cc-6f7f1b1",
     urls = [
-        "https://storage.googleapis.com/public-bazel-artifacts/bazel/cockroachdb-rules_foreign_cc-master20210730-1-gf1cff45.zip",
+        # As of commit 6f7f1b1c6f911db5706c2fcbb3d5669d95974a34 (release 0.7.0 plus a couple patches)
+        "https://storage.googleapis.com/public-bazel-artifacts/bazel/cockroachdb-rules_foreign_cc-6f7f1b1.tar.gz",
     ],
 )
 

c-deps/BUILD.bazel

@@ -6,10 +6,6 @@ load("@rules_foreign_cc//foreign_cc:configure.bzl", "configure_make")
 configure_make(
     name = "libjemalloc",
     autoconf = True,
-    configure_env_vars = select({
-        "//build/toolchains:dev": {"AR": ""},
-        "//conditions:default": {},
-    }),
     configure_in_place = True,
     configure_options = [
         "--enable-prof",
@@ -19,20 +15,20 @@ configure_make(
         "@io_bazel_rules_go//go/platform:linux_arm64": ["--host=aarch64-unknown-linux-gnu"],
         "//conditions:default": [],
     }),
+    env = select({
+        "//build/toolchains:dev": {"AR": ""},
+        "//conditions:default": {},
+    }),
     lib_source = "@jemalloc//:all",
-    make_commands = [
-        "make build_lib_static",
-        "mkdir -p libjemalloc/lib",
-    ] + select({
-        "@io_bazel_rules_go//go/platform:windows": ["cp lib/jemalloc.lib libjemalloc/lib"],
-        "//conditions:default": ["cp lib/libjemalloc.a libjemalloc/lib"],
-    }) + [
-        "cp -r include libjemalloc",
-    ],
     out_static_libs = select({
         "@io_bazel_rules_go//go/platform:windows": ["jemalloc.lib"],
         "//conditions:default": ["libjemalloc.a"],
     }),
+    targets = [
+        "build_lib_static",
+        "install_lib",
+        "install_include",
+    ],
     visibility = ["//visibility:public"],
 )
 
@@ -58,9 +54,7 @@ cmake(
             "CMAKE_BUILD_TYPE": "Release",
         },
     }),
-    # As of this writing (2021-05-05), foreign_cc
-    # only knows about windows, darwin and linux.
-    cmake_options = ["-GUnix Makefiles"],
+    generate_args = ["-GUnix Makefiles"],
     lib_source = "@proj//:all",
     out_static_libs = ["libproj.a"],
     visibility = ["//visibility:public"],
@@ -88,7 +82,6 @@ cmake(
             "CMAKE_CXX_FLAGS": "-fPIC",
         },
     }),
-    cmake_options = ["-GUnix Makefiles"],
     data = select({
         "//build/toolchains:is_cross_macos": [
             "@toolchain_cross_x86_64-apple-darwin19//:bin/x86_64-apple-darwin19-install_name_tool",
@@ -103,39 +96,11 @@ cmake(
         },
         "//conditions:default": {},
     }),
+    generate_args = ["-GUnix Makefiles"],
     lib_source = "@geos//:all",
-    make_commands = [
-        "mkdir -p libgeos/lib",
-        "make --no-print-directory geos_c",
-    ] + select({
-        "//build/toolchains:is_cross_macos": [
-            "cp -L lib/libgeos.dylib libgeos/lib",
-            "cp -L lib/libgeos_c.dylib libgeos/lib",
-            "PREFIX=$($OTOOL -D lib/libgeos_c.dylib | tail -n1 | rev | cut -d/ -f2- | rev)",
-            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos.3.8.1.dylib libgeos/lib/libgeos.dylib",
-            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos_c.1.dylib libgeos/lib/libgeos_c.dylib",
-            "$CMAKE_INSTALL_NAME_TOOL -change $PREFIX/libgeos.3.8.1.dylib @rpath/libgeos.3.8.1.dylib libgeos/lib/libgeos_c.dylib",
-        ],
-        "@io_bazel_rules_go//go/platform:darwin": [
-            "cp -L lib/libgeos.dylib libgeos/lib",
-            "cp -L lib/libgeos_c.dylib libgeos/lib",
-        ],
-        "@io_bazel_rules_go//go/platform:windows": [
-            # NOTE: Windows ends up in bin/ on the BUILD_TMPDIR.
-            "cp -L bin/libgeos.dll libgeos/lib",
-            "cp -L bin/libgeos_c.dll libgeos/lib",
-        ],
-        "//build/toolchains:is_cross_linux": [
-            "cp -L lib/libgeos.so libgeos/lib",
-            "cp -L lib/libgeos_c.so libgeos/lib",
-            "patchelf --set-rpath /usr/local/lib/cockroach/ libgeos/lib/libgeos_c.so",
-            "patchelf --set-soname libgeos.so libgeos/lib/libgeos.so",
-            "patchelf --replace-needed libgeos.so.3.8.1 libgeos.so libgeos/lib/libgeos_c.so",
-        ],
-        "//conditions:default": [
-            "cp -L lib/libgeos.so libgeos/lib",
-            "cp -L lib/libgeos_c.so libgeos/lib",
-        ],
+    out_lib_dir = select({
+        "@io_bazel_rules_go//go/platform:windows": "bin",
+        "//conditions:default": "lib",
     }),
     out_shared_libs = select({
         "@io_bazel_rules_go//go/platform:darwin": [
@@ -151,6 +116,25 @@ cmake(
             "libgeos.so",
         ],
     }),
+    postfix_script = "mkdir -p libgeos/lib\n" + select({
+        "//build/toolchains:is_cross_macos": (
+            "cp -L lib/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos.dylib\n" +
+            "PREFIX=$($OTOOL -D $INSTALLDIR/lib/libgeos_c.dylib | tail -n1 | rev | cut -d/ -f2- | rev)\n" +
+            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos.dylib\n" +
+            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos_c.1.dylib $INSTALLDIR/lib/libgeos_c.dylib\n" +
+            "$CMAKE_INSTALL_NAME_TOOL -change $PREFIX/libgeos.3.8.1.dylib @rpath/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos_c.dylib\n"
+        ),
+        "@io_bazel_rules_go//go/platform:darwin": "cp -L lib/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos.dylib",
+        "@io_bazel_rules_go//go/platform:windows": "",
+        "//build/toolchains:is_cross_linux": (
+            "cp -L lib/libgeos.so.3.8.1 $INSTALLDIR/lib/libgeos.so\n" +
+            "patchelf --set-rpath /usr/local/lib/cockroach/ $INSTALLDIR/lib/libgeos_c.so\n" +
+            "patchelf --set-soname libgeos.so $INSTALLDIR/lib/libgeos.so\n" +
+            "patchelf --replace-needed libgeos.so.3.8.1 libgeos.so $INSTALLDIR/lib/libgeos_c.so\n"
+        ),
+        "//conditions:default": "cp -L lib/libgeos.so.3.8.1 $INSTALLDIR/lib/libgeos.so",
+    }),
+    targets = ["geos_c"],
     visibility = ["//visibility:public"],
 )
 
@@ -165,29 +149,28 @@ configure_make(
         "--enable-static",
         "--disable-shared",
     ],
+    # We specify -fcommon to get around duplicate definition errors in recent gcc.
+    copts = ["-fcommon"],
     data = [":autom4te"],
     env = {
         "AUTOM4TE": "$(execpath :autom4te)",
     },
     lib_source = "@krb5//:all",
-    make_commands = [
-        "make",
-        "mkdir -p libkrb5/lib",
-        "cp lib/libcom_err.a libkrb5/lib",
-        "cp lib/libgssapi_krb5.a libkrb5/lib",
-        "cp lib/libkrb5.a libkrb5/lib",
-        "cp lib/libkrb5support.a libkrb5/lib",
-        "cp lib/libk5crypto.a libkrb5/lib",
-        "mkdir -p libkrb5/include/gssapi",
-        "cp include/gssapi/gssapi.h libkrb5/include/gssapi",
-    ],
     out_static_libs = [
         "libgssapi_krb5.a",
         "libkrb5.a",
         "libkrb5support.a",
         "libk5crypto.a",
         "libcom_err.a",
     ],
+    postfix_script = ("""mkdir -p libkrb5/lib
+cp lib/libcom_err.a libkrb5/lib
+cp lib/libgssapi_krb5.a libkrb5/lib
+cp lib/libkrb5.a libkrb5/lib
+cp lib/libkrb5support.a libkrb5/lib
+cp lib/libk5crypto.a libkrb5/lib
+mkdir -p libkrb5/include/gssapi
+cp include/gssapi/gssapi.h libkrb5/include/gssapi"""),
     visibility = ["//visibility:public"],
 )
 

pkg/sql/information_schema.go

@@ -36,6 +36,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/vtable"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil/pgdate"
 	"github.com/cockroachdb/errors"
+	"github.com/lib/pq/oid"
 	"golang.org/x/text/collate"
 )
 
@@ -617,6 +618,11 @@ https://www.postgresql.org/docs/9.5/infoschema-enabled-roles.html`,
 // string, or if the string's length is not bounded.
 func characterMaximumLength(colType *types.T) tree.Datum {
 	return dIntFnOrNull(func() (int32, bool) {
+		// "char" columns have a width of 1, but should report a NULL maximum
+		// character length.
+		if colType.Oid() == oid.T_char {
+			return 0, false
+		}
 		switch colType.Family() {
 		case types.StringFamily, types.CollatedStringFamily, types.BitFamily:
 			if colType.Width() > 0 {
@@ -633,6 +639,11 @@ func characterMaximumLength(colType *types.T) tree.Datum {
 // string's length is not bounded.
 func characterOctetLength(colType *types.T) tree.Datum {
 	return dIntFnOrNull(func() (int32, bool) {
+		// "char" columns have a width of 1, but should report a NULL octet
+		// length.
+		if colType.Oid() == oid.T_char {
+			return 0, false
+		}
 		switch colType.Family() {
 		case types.StringFamily, types.CollatedStringFamily:
 			if colType.Width() > 0 {