From 81bfa8cee00d606d92f6b3f8d43000912651abec Mon Sep 17 00:00:00 2001
From: Andrew Werner <ajwerner@cockroachlabs.com>
Date: Fri, 21 Oct 2022 00:24:56 +0000
Subject: [PATCH] sql/catalog: break descpb->parser dep

There was no need for this. Saw it as I was looking at something else.

Release note: None
---
 pkg/BUILD.bazel                     |  1 +
 pkg/sql/catalog/descpb/BUILD.bazel  |  7 ++++++-
 pkg/sql/catalog/descpb/column.go    | 15 ---------------
 pkg/sql/catalog/table_elements.go   |  6 +++++-
 pkg/sql/catalog/tabledesc/column.go | 14 ++++++++++++++
 5 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
index b09f34e912cd..cedc727dea5a 100644
--- a/pkg/BUILD.bazel
+++ b/pkg/BUILD.bazel
@@ -274,6 +274,7 @@ ALL_TESTS = [
     "//pkg/sql/catalog/catprivilege:catprivilege_test",
     "//pkg/sql/catalog/colinfo:colinfo_test",
     "//pkg/sql/catalog/dbdesc:dbdesc_test",
+    "//pkg/sql/catalog/descpb:descpb_disallowed_imports_test",
     "//pkg/sql/catalog/descpb:descpb_test",
     "//pkg/sql/catalog/descs:descs_test",
     "//pkg/sql/catalog/funcdesc:funcdesc_test",
diff --git a/pkg/sql/catalog/descpb/BUILD.bazel b/pkg/sql/catalog/descpb/BUILD.bazel
index f37988f71274..6664e3657a52 100644
--- a/pkg/sql/catalog/descpb/BUILD.bazel
+++ b/pkg/sql/catalog/descpb/BUILD.bazel
@@ -3,6 +3,7 @@ load("@rules_proto//proto:defs.bzl", "proto_library")
 load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
 load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 load("//build:STRINGER.bzl", "stringer")
+load("//pkg/testutils/buildutil:buildutil.bzl", "disallowed_imports_test")
 
 go_library(
     name = "descpb",
@@ -23,7 +24,6 @@ go_library(
     deps = [
         "//pkg/keys",
         "//pkg/sql/catalog/catpb",
-        "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/protoreflect",
@@ -95,4 +95,9 @@ stringer(
     typ = "FormatVersion",
 )
 
+disallowed_imports_test(
+    "descpb",
+    disallowed_list = ["//pkg/sql/parser"],
+)
+
 get_x_data(name = "get_x_data")
diff --git a/pkg/sql/catalog/descpb/column.go b/pkg/sql/catalog/descpb/column.go
index 3b2f4166efdc..d20d660a504c 100644
--- a/pkg/sql/catalog/descpb/column.go
+++ b/pkg/sql/catalog/descpb/column.go
@@ -11,27 +11,12 @@
 package descpb
 
 import (
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
-	"github.com/cockroachdb/errors"
 )
 
-// HasNullDefault checks that the column descriptor has a default of NULL.
-func (desc *ColumnDescriptor) HasNullDefault() bool {
-	if !desc.HasDefault() {
-		return false
-	}
-	defaultExpr, err := parser.ParseExpr(*desc.DefaultExpr)
-	if err != nil {
-		panic(errors.NewAssertionErrorWithWrappedErrf(err,
-			"failed to parse default expression %s", *desc.DefaultExpr))
-	}
-	return defaultExpr == tree.DNull
-}
-
 // HasDefault returns true if the column has a default value.
 func (desc *ColumnDescriptor) HasDefault() bool {
 	return desc.DefaultExpr != nil
diff --git a/pkg/sql/catalog/table_elements.go b/pkg/sql/catalog/table_elements.go
index 3badbc683640..c04cdcf4a8e9 100644
--- a/pkg/sql/catalog/table_elements.go
+++ b/pkg/sql/catalog/table_elements.go
@@ -292,6 +292,10 @@ type Column interface {
 	// HasDefault returns true iff the column has a default expression set.
 	HasDefault() bool
 
+	// HasNullDefault returns true if the column has a default expression and
+	// that expression is NULL.
+	HasNullDefault() bool
+
 	// GetDefaultExpr returns the column default expression if it exists,
 	// empty string otherwise.
 	GetDefaultExpr() string
@@ -808,7 +812,7 @@ func ColumnNeedsBackfill(col Column) bool {
 	//  - computed columns
 	//  - non-nullable columns (note: if a non-nullable column doesn't have a
 	//    default value, the backfill will fail unless the table is empty).
-	if col.ColumnDesc().HasNullDefault() {
+	if col.HasNullDefault() {
 		return false
 	}
 	return col.HasDefault() || !col.IsNullable() || col.IsComputed()
diff --git a/pkg/sql/catalog/tabledesc/column.go b/pkg/sql/catalog/tabledesc/column.go
index 055bad5d97c5..5c0aed5111b2 100644
--- a/pkg/sql/catalog/tabledesc/column.go
+++ b/pkg/sql/catalog/tabledesc/column.go
@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr"
+	"github.com/cockroachdb/cockroach/pkg/sql/parser"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -103,6 +104,19 @@ func (w column) HasDefault() bool {
 	return w.desc.HasDefault()
 }
 
+// HasNullDefault checks that the column descriptor has a default of NULL.
+func (w column) HasNullDefault() bool {
+	if !w.HasDefault() {
+		return false
+	}
+	// We ignore the error because what are we going to do with it? It means
+	// that the default expressions is not parsable. Somebody with a context
+	// who needs to use it will be in a better place to log it. If it is not
+	// parsable, it is not NULL.
+	defaultExpr, _ := parser.ParseExpr(w.GetDefaultExpr())
+	return defaultExpr == tree.DNull
+}
+
 // GetDefaultExpr returns the column default expression if it exists,
 // empty string otherwise.
 func (w column) GetDefaultExpr() string {