sql: ensure SHOW CREATE TABLE matches pg_class.reloptions

Release note: None
cockroachdb · Feb 23, 2022 · 8b356c4 · 8b356c4
1 parent 56756f8
commit 8b356c4
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 42 deletions.
diff --git a/pkg/sql/catalog/descriptor.go b/pkg/sql/catalog/descriptor.go
@@ -670,6 +670,8 @@ type TableDescriptor interface {
 	// GetExcludeDataFromBackup returns true if the table's row data is configured
 	// to be excluded during backup.
 	GetExcludeDataFromBackup() bool
+	// GetStorageParams returns a list of storage parameters for the table.
+	GetStorageParams(spaceBetweenEqual bool) []string
 }
 
 // TypeDescriptor will eventually be called typedesc.Descriptor.

diff --git a/pkg/sql/catalog/tabledesc/structured.go b/pkg/sql/catalog/tabledesc/structured.go
@@ -2523,6 +2523,45 @@ func (desc *wrapper) GetExcludeDataFromBackup() bool {
 	return desc.ExcludeDataFromBackup
 }
 
+// GetStorageParams implements the TableDescriptor interface.
+func (desc *wrapper) GetStorageParams(spaceBetweenEqual bool) []string {
+	var storageParams []string
+	var spacing string
+	if spaceBetweenEqual {
+		spacing = ` `
+	}
+	appendStorageParam := func(key, value string) {
+		storageParams = append(storageParams, key+spacing+`=`+spacing+value)
+	}
+	if ttl := desc.GetRowLevelTTL(); ttl != nil {
+		appendStorageParam(`ttl`, `'on'`)
+		appendStorageParam(`ttl_automatic_column`, `'on'`)
+		appendStorageParam(`ttl_expire_after`, string(ttl.DurationExpr))
+		if bs := ttl.SelectBatchSize; bs != 0 {
+			appendStorageParam(`ttl_select_batch_size`, fmt.Sprintf(`%d`, bs))
+		}
+		if bs := ttl.DeleteBatchSize; bs != 0 {
+			appendStorageParam(`ttl_delete_batch_size`, fmt.Sprintf(`%d`, bs))
+		}
+		if cron := ttl.DeletionCron; cron != "" {
+			appendStorageParam(`ttl_job_cron`, fmt.Sprintf(`'%s'`, cron))
+		}
+		if rc := ttl.RangeConcurrency; rc != 0 {
+			appendStorageParam(`ttl_range_concurrency`, fmt.Sprintf(`%d`, rc))
+		}
+		if rl := ttl.DeleteRateLimit; rl != 0 {
+			appendStorageParam(`ttl_delete_rate_limit`, fmt.Sprintf(`%d`, rl))
+		}
+		if pause := ttl.Pause; pause {
+			appendStorageParam(`ttl_pause`, fmt.Sprintf(`%t`, pause))
+		}
+	}
+	if exclude := desc.GetExcludeDataFromBackup(); exclude {
+		appendStorageParam(`exclude_data_from_backup`, `true`)
+	}
+	return storageParams
+}
+
 // GetMultiRegionEnumDependency returns true if the given table has an "implicit"
 // dependency on the multi-region enum. An implicit dependency exists for
 // REGIONAL BY TABLE table's which are homed in an explicit region

diff --git a/pkg/sql/logictest/testdata/logic_test/row_level_ttl b/pkg/sql/logictest/testdata/logic_test/row_level_ttl
@@ -59,7 +59,7 @@ ALTER TABLE tbl DROP COLUMN crdb_internal_expiration
 query T
 SELECT reloptions FROM pg_class WHERE relname = 'tbl'
 ----
-{ttl_expire_after='00:10:00':::INTERVAL}
+{ttl='on',ttl_automatic_column='on',ttl_expire_after='00:10:00':::INTERVAL}
 
 query I
 SELECT count(1) FROM [SHOW SCHEDULES]
@@ -266,11 +266,11 @@ query T
 SELECT create_statement FROM [SHOW CREATE TABLE tbl]
 ----
 CREATE TABLE public.tbl (
-                                                                                                                              id INT8 NOT NULL,
-                                                                                                                              text STRING NULL,
-                                                                                                                              crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
-                                                                                                                              CONSTRAINT tbl_pkey PRIMARY KEY (id ASC),
-                                                                                                                              FAMILY fam_0_id_text_crdb_internal_expiration (id, text, crdb_internal_expiration)
+                                                                                                                     id INT8 NOT NULL,
+                                                                                                                     text STRING NULL,
+                                                                                                                     crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
+                                                                                                                     CONSTRAINT tbl_pkey PRIMARY KEY (id ASC),
+                                                                                                                     FAMILY fam_0_id_text_crdb_internal_expiration (id, text, crdb_internal_expiration)
 ) WITH (ttl = 'on', ttl_automatic_column = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@daily')
 
 let $table_id
@@ -348,6 +348,11 @@ CREATE TABLE tbl (
   FAMILY (id, text)
 ) WITH (ttl_expire_after = '10 minutes', ttl_select_batch_size = 50, ttl_range_concurrency = 2, ttl_delete_rate_limit = 100, ttl_pause = true)
 
+query T
+SELECT reloptions FROM pg_class WHERE relname = 'tbl'
+----
+{ttl='on',ttl_automatic_column='on',ttl_expire_after='00:10:00':::INTERVAL,ttl_select_batch_size=50,ttl_range_concurrency=2,ttl_delete_rate_limit=100,ttl_pause=true}
+
 query T
 SELECT create_statement FROM [SHOW CREATE TABLE tbl]
 ----

diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go
@@ -624,10 +624,12 @@ https://www.postgresql.org/docs/9.5/catalog-pg-class.html`,
 			relPersistence = relPersistenceTemporary
 		}
 		var relOptions tree.Datum = tree.DNull
-		if ttl := table.GetRowLevelTTL(); ttl != nil {
+		if storageParams := table.GetStorageParams(false /* spaceBetweenEqual */); len(storageParams) > 0 {
 			relOptionsArr := tree.NewDArray(types.String)
-			if err := relOptionsArr.Append(tree.NewDString(fmt.Sprintf("ttl_expire_after=%s", ttl.DurationExpr))); err != nil {
-				return err
+			for _, storageParam := range storageParams {
+				if err := relOptionsArr.Append(tree.NewDString(storageParam)); err != nil {
+					return err
+				}
 			}
 			relOptions = relOptionsArr
 		}

diff --git a/pkg/sql/show_create.go b/pkg/sql/show_create.go
@@ -13,7 +13,6 @@ package sql
 import (
 	"bytes"
 	"context"
-	"fmt"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -177,38 +176,7 @@ func ShowCreateTable(
 		return "", err
 	}
 
-	var storageParams []string
-	if ttl := desc.GetRowLevelTTL(); ttl != nil {
-		storageParams = append(
-			storageParams,
-			`ttl = 'on'`,
-			`ttl_automatic_column = 'on'`,
-			fmt.Sprintf(`ttl_expire_after = %s`, ttl.DurationExpr),
-		)
-		if bs := ttl.SelectBatchSize; bs != 0 {
-			storageParams = append(storageParams, fmt.Sprintf(`ttl_select_batch_size = %d`, bs))
-		}
-		if bs := ttl.DeleteBatchSize; bs != 0 {
-			storageParams = append(storageParams, fmt.Sprintf(`ttl_delete_batch_size = %d`, bs))
-		}
-		if cron := ttl.DeletionCron; cron != "" {
-			storageParams = append(storageParams, fmt.Sprintf(`ttl_job_cron = '%s'`, cron))
-		}
-		if rc := ttl.RangeConcurrency; rc != 0 {
-			storageParams = append(storageParams, fmt.Sprintf(`ttl_range_concurrency = %d`, rc))
-		}
-		if rc := ttl.DeleteRateLimit; rc != 0 {
-			storageParams = append(storageParams, fmt.Sprintf(`ttl_delete_rate_limit = %d`, rc))
-		}
-		if pause := ttl.Pause; pause {
-			storageParams = append(storageParams, fmt.Sprintf(`ttl_pause = %t`, pause))
-		}
-	}
-	if exclude := desc.GetExcludeDataFromBackup(); exclude {
-		storageParams = append(storageParams, `exclude_data_from_backup = true`)
-	}
-
-	if len(storageParams) > 0 {
+	if storageParams := desc.GetStorageParams(true /* spaceBetweenEqual */); len(storageParams) > 0 {
 		f.Buffer.WriteString(` WITH (`)
 		f.Buffer.WriteString(strings.Join(storageParams, ", "))
 		f.Buffer.WriteString(`)`)