Merge cockroachdb#86639 cockroachdb#87151 cockroachdb#87175

86639: sql/schemachanger: add back TXN_DROPPED, stop using OFFLINE r=postamar a=ajwerner The first two commits enable synthetic descriptors to be used during transaction execution to influence the virtual tables. We added the `OFFLINE` and performed write operations during drops to move the descriptor into that state in order to deal with the fact that synthetically dropped descriptors would not appear so in virtual tables. Fortunately, this shortcoming has been rectified. We now revive the TXN_DROPPED state. This proves to be important because the `OFFLINE` state is not backwards compatible, and the `TXN_DROPPED` state is. Ideally we'd find a way to move the descriptors straight to `DROPPED` in the pre-commit case for all the cases we today support, but doing that would require an overhaul of the dependency rules which, at this point in the cycle, feels risky. Fixes cockroachdb#86626 Release justification: Fixes a release-blocking backwards incompatibility. Release note: None 87151: sql: plumb isCopy down to event log r=rafiss a=otan Previously, we relied on the `txn` being filled to log `COPY` statements. This is however inaccurate - `txn` may be nil if `COPY` is the first statement encountered. This meant that the first COPY encountered would not be logged. Instead, plumb down `isCopy` and use `timeutil.Now()` if it is a COPY related statement. Release justification: logging related critical fix Release note: None 87175: sql: fix COPY internal error in optimizer r=cucaroach a=cucaroach Statistics builder code to evaluate distinct count had a typo and would crash if there were more rows than columns. Fix bug and rewrite to be a little clearer. Fixes: cockroachdb#87011 Release justification: low-risk high priority fix to new funcationality Release note: None Co-authored-by: Andrew Werner <[email protected]> Co-authored-by: Oliver Tan <[email protected]> Co-authored-by: Tommy Reilly <[email protected]>
itsbilal · Sep 1, 2022 · 9911916 · 9911916
4 parents d18ecbb + affdfee + c20ba1b + b431524
commit 9911916
Show file tree

Hide file tree

Showing 275 changed files with 9,870 additions and 4,489 deletions.
diff --git a/pkg/bench/rttanalysis/testdata/benchmark_expectations b/pkg/bench/rttanalysis/testdata/benchmark_expectations
@@ -31,22 +31,22 @@ exp,benchmark
 16,CreateRole/create_role_with_2_options
 19,CreateRole/create_role_with_3_options
 14,CreateRole/create_role_with_no_options
-15,DropDatabase/drop_database_0_tables
-17,DropDatabase/drop_database_1_table
-19,DropDatabase/drop_database_2_tables
-21,DropDatabase/drop_database_3_tables
+13,DropDatabase/drop_database_0_tables
+15,DropDatabase/drop_database_1_table
+17,DropDatabase/drop_database_2_tables
+19,DropDatabase/drop_database_3_tables
 19,DropRole/drop_1_role
 27,DropRole/drop_2_roles
 35,DropRole/drop_3_roles
-17,DropSequence/drop_1_sequence
-19,DropSequence/drop_2_sequences
-21,DropSequence/drop_3_sequences
-18,DropTable/drop_1_table
-21,DropTable/drop_2_tables
-24,DropTable/drop_3_tables
-19,DropView/drop_1_view
-20,DropView/drop_2_views
-20,DropView/drop_3_views
+15,DropSequence/drop_1_sequence
+17,DropSequence/drop_2_sequences
+19,DropSequence/drop_3_sequences
+16,DropTable/drop_1_table
+19,DropTable/drop_2_tables
+22,DropTable/drop_3_tables
+17,DropView/drop_1_view
+18,DropView/drop_2_views
+18,DropView/drop_3_views
 13,Grant/grant_all_on_1_table
 14,Grant/grant_all_on_2_tables
 14,Grant/grant_all_on_3_tables

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors-declarative b/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors-declarative
@@ -24,34 +24,34 @@ DROP DATABASE dd CASCADE;
 ----
 job paused at pausepoint
 
-# At this point, we have a descriptor entry for `dd` in an OFFLINE state.
+# At this point, we have a descriptor entry for `dd` in a dropped state.
 query-sql
 WITH tbls AS (
 	SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor
 )
 SELECT orig->'database'->'name', orig->'database'->'state' FROM tbls WHERE id = 107;
 ----
-"dd" "OFFLINE"
+"dd" "DROP"
 
-# A database backup should fail since we are explicitly targeting an offline
+# A database backup should fail since we are explicitly targeting a dropped
 # object.
 exec-sql
 BACKUP DATABASE dd INTO 'nodelocal://0/dropped-database';
 ----
-pq: failed to resolve targets specified in the BACKUP stmt: database "dd" does not exist
+pq: failed to resolve targets specified in the BACKUP stmt: database "dd" does not exist, or invalid RESTORE timestamp: supplied backups do not cover requested time
 
 # A cluster backup should succeed.
 exec-sql
 BACKUP INTO 'nodelocal://0/cluster/dropped-database';
 ----
 
-# The offline descriptors should end up in the cluster backup.
+# The dropped descriptors should not end up in the cluster backup.
 query-sql
 SELECT count(*)
   FROM [SHOW BACKUP LATEST IN 'nodelocal://0/cluster/dropped-database']
   WHERE object_name IN ('dd', 'foo', 's');
 ----
-3
+0
 
 subtest end
 
@@ -99,18 +99,18 @@ WITH tbls AS (
 )
 SELECT orig->'schema'->'name', orig->'schema'->'state' FROM tbls WHERE id = 112;
 ----
-"s" "OFFLINE"
+"s" "DROP"
 
 query-sql
 WITH tbls AS (
 	SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor
 )
 SELECT orig->'type'->'name', orig->'type'->'state' FROM tbls WHERE id = 110 OR id = 111;
 ----
-"typ" "OFFLINE"
-"_typ" "OFFLINE"
+"typ" "DROP"
+"_typ" "DROP"
 
-# A database backup should succeed and should include the offline schema,
+# A database backup should succeed and should not include the dropped schema,
 # type, and table.
 exec-sql
 BACKUP DATABASE d2 INTO 'nodelocal://0/dropped-schema-in-database';
@@ -121,9 +121,9 @@ SELECT count(*)
   FROM [SHOW BACKUP LATEST IN 'nodelocal://0/dropped-schema-in-database']
   WHERE object_name IN ('s', 't', 'typ', '_typ');
 ----
-4
+0
 
-# A cluster backup should succeed but should include the offline schema,
+# A cluster backup should succeed but should not include the dropped schema,
 # type, and table.
 exec-sql
 BACKUP INTO 'nodelocal://0/cluster/dropped-schema-in-database';
@@ -134,7 +134,7 @@ SELECT count(*)
   FROM [SHOW BACKUP LATEST IN 'nodelocal://0/cluster/dropped-schema-in-database']
   WHERE object_name IN ('s', 't', 'typ', '_typ');
 ----
-4
+0
 
 # Restore the backups to check they are valid.
 exec-sql
@@ -145,12 +145,11 @@ exec-sql
 USE d3;
 ----
 
-# We expect to see the offline schema 's'.
+# We expect to not see the dropped schema 's'.
 query-sql
 SELECT schema_name FROM [SHOW SCHEMAS];
 ----
 public
-s
 crdb_internal
 information_schema
 pg_catalog
@@ -161,7 +160,6 @@ query-sql
 SELECT schema_name, table_name FROM [SHOW TABLES];
 ----
 public t2
-s t
 
 exec-sql
 RESTORE DATABASE d2 FROM LATEST IN 'nodelocal://0/cluster/dropped-schema-in-database' WITH new_db_name ='d4';
@@ -175,7 +173,6 @@ query-sql
 SELECT schema_name FROM [SHOW SCHEMAS];
 ----
 public
-s
 crdb_internal
 information_schema
 pg_catalog
@@ -185,6 +182,5 @@ query-sql
 SELECT schema_name, table_name FROM [SHOW TABLES];
 ----
 public t2
-s t
 
 subtest end
diff --git a/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index b/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index
@@ -125,14 +125,6 @@ upsert descriptor #104
   +    targets: <redacted>
      families:
      - columnIds:
-  ...
-     formatVersion: 3
-     id: 104
-  -  modificationTime: {}
-  +  modificationTime:
-  +    wallTime: "1640995200000000001"
-     mutations:
-     - direction: ADD
   ...
 create job #1 (non-cancelable: false): "CREATE INDEX id1 ON defaultdb.public.t1 (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1))"
   descriptor IDs: [104]