Merge #148134 #148168 #148248 #148329

148134: sqlccl: remove ignored internal error in ExplainGist that's been fixed r=yuzefovich a=yuzefovich

See: #133129.
Epic: None

Release note: None

148168: workloadccl: remove stale comment around importing fixtures r=yuzefovich a=yuzefovich

This commit deletes now-stale comment that was added in 09abc26. That change made a switch from using IMPORT TABLE to IMPORT INTO for importing fixtures as a temporary workaround. However, since then we completely deprecated IMPORT TABLE syntax, so the only way to import the data now is to create the tables separately, which makes the comment confusing.

Epic: None
Release note: None

148248: parser: remove IMPORT TABLE and IMPORT non-INTO support r=yuzefovich a=yuzefovich

**parser: remove IMPORT TABLE and IMPORT non-INTO support**

PGDUMP and MYSQLDUMP code is now being removed, so remove the parser support.

Release note (sql change): IMPORT TABLE as well PGDUMP and MYSQLDUMP
formats of IMPORT are now fully removed. These have been deprecated
since 23.2.

**tree: remove unused Into and Bundle fields of Import**

Also update one spot under assumption that `tree.Import.Table` is now
always non-nil.

Epic: None

148329: execbuilder: fix a silly bug around finding the most recent full stat r=yuzefovich a=yuzefovich

This commit fixes a bug that was added in 3bc0992. In short, in a complicated logical expression we had `v1 && v2 || v3` when we wanted `v1 && (v2 || v3)`. This allowed us to hit an index of bounds when evaluating `v3`, which in this concrete case could mean that we iterated over all stats and didn't find any full ones.

There are two other places with similar conditionals, and they have the correct usage of parenthesis, so I didn't change the structure overall. I did find that we want to also skip the merged stat, depending on the session variable.

Also the bug seems quite difficult to hit, so I omitted the release note.

Fixes: #148316.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

docs/generated/sql/bnf/import_dump.bnf

@@ -1,9 +1,5 @@
 import_stmt ::=
-	'IMPORT' import_format file_location 'WITH' kv_option_list
-	| 'IMPORT' import_format file_location 
-	| 'IMPORT' 'TABLE' table_name 'FROM' import_format file_location 'WITH' kv_option_list
-	| 'IMPORT' 'TABLE' table_name 'FROM' import_format file_location 
-	| 'IMPORT' 'INTO' table_name '(' insert_column_list ')' import_format 'DATA' '(' file_location ( ( ',' file_location ) )* ')' 'WITH' kv_option_list
+	'IMPORT' 'INTO' table_name '(' insert_column_list ')' import_format 'DATA' '(' file_location ( ( ',' file_location ) )* ')' 'WITH' kv_option_list
 	| 'IMPORT' 'INTO' table_name '(' insert_column_list ')' import_format 'DATA' '(' file_location ( ( ',' file_location ) )* ')' 
 	| 'IMPORT' 'INTO' table_name import_format 'DATA' '(' file_location ( ( ',' file_location ) )* ')' 'WITH' kv_option_list
 	| 'IMPORT' 'INTO' table_name import_format 'DATA' '(' file_location ( ( ',' file_location ) )* ')' 

docs/generated/sql/bnf/stmt_block.bnf

@@ -238,9 +238,7 @@ explain_stmt ::=
 	| 'EXPLAIN' 'ANALYSE' '(' explain_option_list ')' explainable_stmt
 
 import_stmt ::=
-	'IMPORT' import_format string_or_placeholder opt_with_options
-	| 'IMPORT' 'TABLE' table_name 'FROM' import_format string_or_placeholder opt_with_options
-	| 'IMPORT' 'INTO' table_name '(' insert_column_list ')' import_format 'DATA' '(' string_or_placeholder_list ')' opt_with_options
+	'IMPORT' 'INTO' table_name '(' insert_column_list ')' import_format 'DATA' '(' string_or_placeholder_list ')' opt_with_options
 	| 'IMPORT' 'INTO' table_name import_format 'DATA' '(' string_or_placeholder_list ')' opt_with_options
 
 insert_stmt ::=
@@ -719,24 +717,20 @@ explainable_stmt ::=
 explain_option_list ::=
 	( explain_option_name ) ( ( ',' explain_option_name ) )*
 
+insert_column_list ::=
+	( insert_column_item ) ( ( ',' insert_column_item ) )*
+
 import_format ::=
 	name
 
-string_or_placeholder ::=
-	non_reserved_word_or_sconst
-	| 'PLACEHOLDER'
+string_or_placeholder_list ::=
+	( string_or_placeholder ) ( ( ',' string_or_placeholder ) )*
 
 opt_with_options ::=
 	'WITH' kv_option_list
 	| 'WITH' 'OPTIONS' '(' kv_option_list ')'
 	| 
 
-insert_column_list ::=
-	( insert_column_item ) ( ( ',' insert_column_item ) )*
-
-string_or_placeholder_list ::=
-	( string_or_placeholder ) ( ( ',' string_or_placeholder ) )*
-
 insert_target ::=
 	table_name_opt_idx
 	| table_name_opt_idx 'AS' table_alias_name
@@ -775,6 +769,10 @@ reset_session_stmt ::=
 reset_csetting_stmt ::=
 	'RESET' 'CLUSTER' 'SETTING' var_name
 
+string_or_placeholder ::=
+	non_reserved_word_or_sconst
+	| 'PLACEHOLDER'
+
 opt_with_restore_options ::=
 	'WITH' restore_options_list
 	| 'WITH' 'OPTIONS' '(' restore_options_list ')'
@@ -2032,16 +2030,12 @@ drop_policy_stmt ::=
 explain_option_name ::=
 	non_reserved_word
 
-non_reserved_word_or_sconst ::=
-	non_reserved_word
-	| 'SCONST'
+insert_column_item ::=
+	column_name
 
 kv_option_list ::=
 	( kv_option ) ( ( ',' kv_option ) )*
 
-insert_column_item ::=
-	column_name
-
 session_var ::=
 	'identifier'
 	| 'identifier' session_var_parts
@@ -2059,6 +2053,10 @@ var_name ::=
 	name
 	| name attrs
 
+non_reserved_word_or_sconst ::=
+	non_reserved_word
+	| 'SCONST'
+
 restore_options_list ::=
 	( restore_options ) ( ( ',' restore_options ) )*
 
@@ -2925,15 +2923,15 @@ non_reserved_word ::=
 	| col_name_keyword
 	| type_func_name_keyword
 
+column_name ::=
+	name
+
 kv_option ::=
 	name '=' string_or_placeholder
 	| name
 	| 'SCONST' '=' string_or_placeholder
 	| 'SCONST'
 
-column_name ::=
-	name
-
 session_var_parts ::=
 	( '.' 'identifier' ) ( ( '.' 'identifier' ) )*
 

pkg/ccl/testccl/sqlccl/explain_test.go

@@ -191,7 +191,6 @@ func TestExplainGist(t *testing.T) {
 				// Ignore all errors except the internal ones.
 				for _, knownErr := range []string{
 					"expected equivalence dependants to be its closure", // #119045
-					"not in index", // #133129
 				} {
 					if strings.Contains(err.Error(), knownErr) {
 						// Don't fail the test on a set of known errors.

pkg/ccl/workloadccl/fixture.go

@@ -374,17 +374,7 @@ func ImportFixture(
 		defer enableFn()
 	}
 
-	pathPrefix := csvServer
-	if pathPrefix == `` {
-		pathPrefix = `workload://`
-	}
-
-	// Pre-create tables. It's required that we pre-create the tables before we
-	// parallelize the IMPORT because for multi-region setups, the create table
-	// will end up modifying the crdb_internal_region type (to install back
-	// references). If create table is done in parallel with IMPORT, some IMPORT
-	// jobs may fail because the type is being modified concurrently with the
-	// IMPORT. Removing the need to pre-create is being tracked with #70987.
+	// Prepare the tables for ingestion via IMPORT INTO.
 	const maxTableBatchSize = 5000
 	currentTable := 0
 	for currentTable < len(tables) {
@@ -404,6 +394,11 @@ func ImportFixture(
 		currentTable += maxTableBatchSize
 	}
 
+	pathPrefix := csvServer
+	if pathPrefix == `` {
+		pathPrefix = `workload://`
+	}
+
 	// Default to unbounded unless a flag exists for it.
 	concurrencyLimit := math.MaxInt
 	if flagser, ok := gen.(workload.Flagser); ok {

pkg/internal/sqlsmith/bulkio.go

@@ -258,7 +258,6 @@ func makeImport(s *Smither) (tree.Statement, bool) {
 
 	return &tree.Import{
 		Table:      tree.NewUnqualifiedTableName(tab),
-		Into:       true,
 		FileFormat: "CSV",
 		Files:      files,
 		Options: tree.KVOptions{

pkg/jobs/jobspb/jobs.proto

@@ -714,6 +714,7 @@ message ImportDetails {
   bool schemas_published = 24;
   bool tables_published = 13;
 
+  // TODO(yuzefovich): remove this.
   bool parse_bundle_schema = 14;
 
   // DefaultIntSize is the integer type that a "naked" int will be resolved

pkg/sql/importer/BUILD.bazel

@@ -38,7 +38,6 @@ go_library(
         "//pkg/jobs/joberror",
         "//pkg/jobs/jobspb",
         "//pkg/jobs/jobsprofiler",
-        "//pkg/keys",
         "//pkg/kv",
         "//pkg/kv/kvpb",
         "//pkg/kv/kvserver/kvserverbase",