Merge cockroachdb#59410 cockroachdb#59730

59410: sql: change SHOW ZONE CONFIGURATION to use FROM instead of FOR r=otan a=andyyang890

The SHOW ZONE CONFIGURATION statements were changed to use
FROM instead of FOR to standardize the use of FROM with SHOW.

Refs: cockroachdb#57638

Release note (sql change): This patch overrides the release note
added in cockroachdb#58740. The SHOW ZONE CONFIGURATION statement has been
changed to use FROM instead of FOR.

59730: backupccl: fix rare failure in reading backup file r=dt a=pbardea

BackupManifests are compresed when written to ExternalStorage. When
reading backup manifests, we check if the content type indicates that
the backup manifest is compressed (and thus needs to be decompressed).
We do this because some previous backups were not compressed, so backup
needs to be able to detect if the backup manfiest has been compressed.

However, very rarely (1 in 60000 attempts in my case), the compressed
data might be detected as "application/vnd.ms-fontobject", rather than a
gzipped file. This causes backup to not decompress the file, and thus
try to unmarshall the compressed data. The file is misdetected because
the defined sniffing algorithm in http.DetectContentType first looks at
the 35th byte to see if it matches a "magic pattern", before checking if
the data is in gzip format. And, sometimes, the 35th byte happens to
match up.

This commit updates the check so that we only check if the GZip magic
bytes header is present or not, rather than checking all possible
content types. The gzip header is not expected to conflict with the
first 6 bytes of normally generated protobuf messages that are
compressed.

Closes cockroachdb#59685.
Closes cockroachdb#54550.

Release note (bug fix): Fixes a bug where backups would fail with an
error when trying to read a backup that was written.

Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Paul Bardea <[email protected]>

docs/generated/sql/bnf/show_zone_stmt.bnf

@@ -1,14 +1,14 @@
 show_zone_stmt ::=
-	'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'RANGE' zone_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'DATABASE' database_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'TABLE' table_name 'PARTITION' partition_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'TABLE' table_name 
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'PARTITION' partition_name 'OF' 'TABLE' table_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'INDEX' table_name '@' index_name 'PARTITION' partition_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'INDEX' table_name '@' index_name 
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'INDEX' standalone_index_name 'PARTITION' partition_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'INDEX' standalone_index_name 
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'PARTITION' partition_name 'OF' 'INDEX' table_name '@' index_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'PARTITION' partition_name 'OF' 'INDEX' standalone_index_name
+	'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'RANGE' zone_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'DATABASE' database_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'TABLE' table_name 'PARTITION' partition_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'TABLE' table_name 
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'PARTITION' partition_name 'OF' 'TABLE' table_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'INDEX' table_name '@' index_name 'PARTITION' partition_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'INDEX' table_name '@' index_name 
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'INDEX' standalone_index_name 'PARTITION' partition_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'INDEX' standalone_index_name 
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'PARTITION' partition_name 'OF' 'INDEX' table_name '@' index_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'PARTITION' partition_name 'OF' 'INDEX' standalone_index_name
 	| 'SHOW' 'ZONE' 'CONFIGURATIONS'
 	| 'SHOW' 'ALL' 'ZONE' 'CONFIGURATIONS'

docs/generated/sql/bnf/stmt_block.bnf

@@ -721,12 +721,12 @@ show_users_stmt ::=
 	'SHOW' 'USERS'
 
 show_zone_stmt ::=
-	'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'RANGE' zone_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'DATABASE' database_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'TABLE' table_name opt_partition
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'PARTITION' partition_name 'OF' 'TABLE' table_name
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'INDEX' table_index_name opt_partition
-	| 'SHOW' 'ZONE' 'CONFIGURATION' for_or_from 'PARTITION' partition_name 'OF' 'INDEX' table_index_name
+	'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'RANGE' zone_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'DATABASE' database_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'TABLE' table_name opt_partition
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'PARTITION' partition_name 'OF' 'TABLE' table_name
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'INDEX' table_index_name opt_partition
+	| 'SHOW' 'ZONE' 'CONFIGURATION' from_with_implicit_for_alias 'PARTITION' partition_name 'OF' 'INDEX' table_index_name
 	| 'SHOW' 'ZONE' 'CONFIGURATIONS'
 	| 'SHOW' 'ALL' 'ZONE' 'CONFIGURATIONS'
 
@@ -1535,9 +1535,8 @@ opt_compact ::=
 	'COMPACT'
 	| 
 
-for_or_from ::=
-	'FOR'
-	| 'FROM'
+from_with_implicit_for_alias ::=
+	'FROM'
 
 zone_name ::=
 	unrestricted_name

pkg/ccl/backupccl/backup_test.go

@@ -17,7 +17,6 @@ import (
 	"io"
 	"io/ioutil"
 	"math/rand"
-	"net/http"
 	"net/url"
 	"os"
 	"path"
@@ -335,8 +334,7 @@ func TestBackupRestoreStatementResult(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		fileType := http.DetectContentType(backupManifestBytes)
-		require.Equal(t, ZipType, fileType)
+		require.True(t, isGZipped(backupManifestBytes))
 	})
 
 	sqlDB.Exec(t, "CREATE DATABASE data2")
@@ -468,8 +466,7 @@ func TestBackupRestorePartitioned(t *testing.T) {
 					if err != nil {
 						t.Fatal(err)
 					}
-					fileType := http.DetectContentType(backupPartitionBytes)
-					require.Equal(t, ZipType, fileType)
+					require.True(t, isGZipped(backupPartitionBytes))
 				}
 			}
 		}
@@ -1586,8 +1583,7 @@ func TestBackupRestoreResume(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		fileType := http.DetectContentType(backupManifestBytes)
-		if fileType == ZipType {
+		if isGZipped(backupManifestBytes) {
 			backupManifestBytes, err = decompressData(backupManifestBytes)
 			require.NoError(t, err)
 		}
@@ -3946,8 +3942,7 @@ func TestBackupRestoreChecksum(t *testing.T) {
 		if err != nil {
 			t.Fatalf("%+v", err)
 		}
-		fileType := http.DetectContentType(backupManifestBytes)
-		if fileType == ZipType {
+		if isGZipped(backupManifestBytes) {
 			backupManifestBytes, err = decompressData(backupManifestBytes)
 			require.NoError(t, err)
 		}

pkg/ccl/backupccl/manifest_handling.go

@@ -16,7 +16,6 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io/ioutil"
-	"net/http"
 	"net/url"
 	"path"
 	"sort"
@@ -72,14 +71,26 @@ const (
 const (
 	// BackupFormatDescriptorTrackingVersion added tracking of complete DBs.
 	BackupFormatDescriptorTrackingVersion uint32 = 1
-	// ZipType is the format of a GZipped compressed file.
-	ZipType = "application/x-gzip"
 
 	dateBasedIncFolderName  = "/20060102/150405.00"
 	dateBasedIntoFolderName = "/2006/01/02-150405.00"
 	latestFileName          = "LATEST"
 )
 
+// isGZipped detects whether the given bytes represent GZipped data. This check
+// is used rather than a standard implementation such as http.DetectContentType
+// since some zipped data may be mis-identified by that method. We've seen
+// gzipped data incorrectly identified as "application/vnd.ms-fontobject". The
+// magic bytes are from the MIME sniffing algorithm http.DetectContentType is
+// based which can be found at https://mimesniff.spec.whatwg.org/.
+//
+// This method is only used to detect if protobufs are GZipped, and there are no
+// conflicts between the starting bytes of a protobuf and these magic bytes.
+func isGZipped(dat []byte) bool {
+	gzipPrefix := []byte("\x1F\x8B\x08")
+	return bytes.HasPrefix(dat, gzipPrefix)
+}
+
 // BackupFileDescriptors is an alias on which to implement sort's interface.
 type BackupFileDescriptors []BackupManifest_File
 
@@ -223,8 +234,7 @@ func readBackupManifest(
 		}
 	}
 
-	fileType := http.DetectContentType(descBytes)
-	if fileType == ZipType {
+	if isGZipped(descBytes) {
 		descBytes, err = decompressData(descBytes)
 		if err != nil {
 			return BackupManifest{}, errors.Wrap(
@@ -290,8 +300,7 @@ func readBackupPartitionDescriptor(
 		}
 	}
 
-	fileType := http.DetectContentType(descBytes)
-	if fileType == ZipType {
+	if isGZipped(descBytes) {
 		descBytes, err = decompressData(descBytes)
 		if err != nil {
 			return BackupPartitionDescriptor{}, errors.Wrap(

pkg/sql/parser/parse_test.go

@@ -671,17 +671,17 @@ func TestParse(t *testing.T) {
 		{`SHOW EXPERIMENTAL_FINGERPRINTS FROM TABLE d.t`},
 		{`SHOW ZONE CONFIGURATIONS`},
 		{`EXPLAIN SHOW ZONE CONFIGURATIONS`},
-		{`SHOW ZONE CONFIGURATION FOR RANGE default`},
-		{`SHOW ZONE CONFIGURATION FOR RANGE meta`},
-		{`SHOW ZONE CONFIGURATION FOR DATABASE db`},
-		{`SHOW ZONE CONFIGURATION FOR TABLE db.t`},
-		{`SHOW ZONE CONFIGURATION FOR TABLE db.schema.t`},
-		{`SHOW ZONE CONFIGURATION FOR PARTITION p OF TABLE db.t`},
-		{`SHOW ZONE CONFIGURATION FOR TABLE t`},
-		{`SHOW ZONE CONFIGURATION FOR PARTITION p OF TABLE t`},
-		{`SHOW ZONE CONFIGURATION FOR INDEX db.t@i`},
-		{`SHOW ZONE CONFIGURATION FOR INDEX t@i`},
-		{`SHOW ZONE CONFIGURATION FOR INDEX i`},
+		{`SHOW ZONE CONFIGURATION FROM RANGE default`},
+		{`SHOW ZONE CONFIGURATION FROM RANGE meta`},
+		{`SHOW ZONE CONFIGURATION FROM DATABASE db`},
+		{`SHOW ZONE CONFIGURATION FROM TABLE db.t`},
+		{`SHOW ZONE CONFIGURATION FROM TABLE db.schema.t`},
+		{`SHOW ZONE CONFIGURATION FROM PARTITION p OF TABLE db.t`},
+		{`SHOW ZONE CONFIGURATION FROM TABLE t`},
+		{`SHOW ZONE CONFIGURATION FROM PARTITION p OF TABLE t`},
+		{`SHOW ZONE CONFIGURATION FROM INDEX db.t@i`},
+		{`SHOW ZONE CONFIGURATION FROM INDEX t@i`},
+		{`SHOW ZONE CONFIGURATION FROM INDEX i`},
 
 		// Tables are the default, but can also be specified with
 		// GRANT x ON TABLE y. However, the stringer does not output TABLE.
@@ -2263,24 +2263,24 @@ $function$`,
 
 		{`SHOW ALL ZONE CONFIGURATIONS`, `SHOW ZONE CONFIGURATIONS`},
 
-		{`SHOW ZONE CONFIGURATION FOR TABLE t PARTITION foo`,
-			`SHOW ZONE CONFIGURATION FOR PARTITION foo OF TABLE t`},
-		{`SHOW ZONE CONFIGURATION FOR INDEX t@idx PARTITION foo`,
-			`SHOW ZONE CONFIGURATION FOR PARTITION foo OF INDEX t@idx`},
-
-		{`SHOW ZONE CONFIGURATION FROM RANGE foo`, `SHOW ZONE CONFIGURATION FOR RANGE foo`},
-		{`SHOW ZONE CONFIGURATION FROM DATABASE foo`, `SHOW ZONE CONFIGURATION FOR DATABASE foo`},
-		{`SHOW ZONE CONFIGURATION FROM TABLE foo`, `SHOW ZONE CONFIGURATION FOR TABLE foo`},
-		{`SHOW ZONE CONFIGURATION FROM TABLE foo PARTITION bar`,
-			`SHOW ZONE CONFIGURATION FOR PARTITION bar OF TABLE foo`},
-		{`SHOW ZONE CONFIGURATION FROM PARTITION foo OF TABLE bar`,
-			`SHOW ZONE CONFIGURATION FOR PARTITION foo OF TABLE bar`},
-		{`SHOW ZONE CONFIGURATION FROM INDEX foo`,
-			`SHOW ZONE CONFIGURATION FOR INDEX foo`},
-		{`SHOW ZONE CONFIGURATION FROM INDEX foo PARTITION bar`,
-			`SHOW ZONE CONFIGURATION FOR PARTITION bar OF INDEX foo`},
-		{`SHOW ZONE CONFIGURATION FROM PARTITION foo OF INDEX bar`,
-			`SHOW ZONE CONFIGURATION FOR PARTITION foo OF INDEX bar`},
+		{`SHOW ZONE CONFIGURATION FROM TABLE t PARTITION foo`,
+			`SHOW ZONE CONFIGURATION FROM PARTITION foo OF TABLE t`},
+		{`SHOW ZONE CONFIGURATION FROM INDEX t@idx PARTITION foo`,
+			`SHOW ZONE CONFIGURATION FROM PARTITION foo OF INDEX t@idx`},
+
+		{`SHOW ZONE CONFIGURATION FOR RANGE foo`, `SHOW ZONE CONFIGURATION FROM RANGE foo`},
+		{`SHOW ZONE CONFIGURATION FOR DATABASE foo`, `SHOW ZONE CONFIGURATION FROM DATABASE foo`},
+		{`SHOW ZONE CONFIGURATION FOR TABLE foo`, `SHOW ZONE CONFIGURATION FROM TABLE foo`},
+		{`SHOW ZONE CONFIGURATION FOR TABLE foo PARTITION bar`,
+			`SHOW ZONE CONFIGURATION FROM PARTITION bar OF TABLE foo`},
+		{`SHOW ZONE CONFIGURATION FOR PARTITION foo OF TABLE bar`,
+			`SHOW ZONE CONFIGURATION FROM PARTITION foo OF TABLE bar`},
+		{`SHOW ZONE CONFIGURATION FOR INDEX foo`,
+			`SHOW ZONE CONFIGURATION FROM INDEX foo`},
+		{`SHOW ZONE CONFIGURATION FOR INDEX foo PARTITION bar`,
+			`SHOW ZONE CONFIGURATION FROM PARTITION bar OF INDEX foo`},
+		{`SHOW ZONE CONFIGURATION FOR PARTITION foo OF INDEX bar`,
+			`SHOW ZONE CONFIGURATION FROM PARTITION foo OF INDEX bar`},
 
 		{`BEGIN`,
 			`BEGIN TRANSACTION`},

pkg/sql/parser/sql.y

@@ -5097,38 +5097,38 @@ show_roles_stmt:
 | SHOW ROLES error // SHOW HELP: SHOW ROLES
 
 show_zone_stmt:
-  SHOW ZONE CONFIGURATION for_or_from RANGE zone_name
+  SHOW ZONE CONFIGURATION from_with_implicit_for_alias RANGE zone_name
   {
     $$.val = &tree.ShowZoneConfig{ZoneSpecifier: tree.ZoneSpecifier{NamedZone: tree.UnrestrictedName($6)}}
   }
-| SHOW ZONE CONFIGURATION for_or_from DATABASE database_name
+| SHOW ZONE CONFIGURATION from_with_implicit_for_alias DATABASE database_name
   {
     $$.val = &tree.ShowZoneConfig{ZoneSpecifier: tree.ZoneSpecifier{Database: tree.Name($6)}}
   }
-| SHOW ZONE CONFIGURATION for_or_from TABLE table_name opt_partition
+| SHOW ZONE CONFIGURATION from_with_implicit_for_alias TABLE table_name opt_partition
   {
     name := $6.unresolvedObjectName().ToTableName()
     $$.val = &tree.ShowZoneConfig{ZoneSpecifier: tree.ZoneSpecifier{
         TableOrIndex: tree.TableIndexName{Table: name},
         Partition: tree.Name($7),
     }}
   }
-| SHOW ZONE CONFIGURATION for_or_from PARTITION partition_name OF TABLE table_name
+| SHOW ZONE CONFIGURATION from_with_implicit_for_alias PARTITION partition_name OF TABLE table_name
   {
     name := $9.unresolvedObjectName().ToTableName()
     $$.val = &tree.ShowZoneConfig{ZoneSpecifier: tree.ZoneSpecifier{
       TableOrIndex: tree.TableIndexName{Table: name},
       Partition: tree.Name($6),
     }}
   }
-| SHOW ZONE CONFIGURATION for_or_from INDEX table_index_name opt_partition
+| SHOW ZONE CONFIGURATION from_with_implicit_for_alias INDEX table_index_name opt_partition
   {
     $$.val = &tree.ShowZoneConfig{ZoneSpecifier: tree.ZoneSpecifier{
       TableOrIndex: $6.tableIndexName(),
       Partition: tree.Name($7),
     }}
   }
-| SHOW ZONE CONFIGURATION for_or_from PARTITION partition_name OF INDEX table_index_name
+| SHOW ZONE CONFIGURATION from_with_implicit_for_alias PARTITION partition_name OF INDEX table_index_name
   {
     $$.val = &tree.ShowZoneConfig{ZoneSpecifier: tree.ZoneSpecifier{
       TableOrIndex: $9.tableIndexName(),
@@ -5144,9 +5144,9 @@ show_zone_stmt:
     $$.val = &tree.ShowZoneConfig{}
   }
 
-for_or_from:
-  FOR
-| FROM
+from_with_implicit_for_alias:
+  FROM
+| FOR { /* SKIP DOC */ }
 
 // %Help: SHOW RANGE - show range information for a row
 // %Category: Misc

pkg/sql/sem/tree/zone.go

@@ -95,7 +95,7 @@ func (node *ShowZoneConfig) Format(ctx *FmtCtx) {
 	if node.ZoneSpecifier == (ZoneSpecifier{}) {
 		ctx.WriteString("SHOW ZONE CONFIGURATIONS")
 	} else {
-		ctx.WriteString("SHOW ZONE CONFIGURATION FOR ")
+		ctx.WriteString("SHOW ZONE CONFIGURATION FROM ")
 		ctx.FormatNode(&node.ZoneSpecifier)
 	}
 }