From dadd46e9ee8cae7e1e009ba4f418cbd272cbcc0e Mon Sep 17 00:00:00 2001
From: Rafi Shamim <rafi@cockroachlabs.com>
Date: Sun, 6 Oct 2024 04:02:20 +0000
Subject: [PATCH] sql/copy: use the proper interface for internal queries

Sicne these internal queries are executed in order to run a
user-initiated command, it should use the planner's internal executor
rather than making a new one from the InternalDB.

This doesn't matter on this branch apart from code hygiene, but in 24.1,
this could cause test-only assertions to fail. Specifically, the check
that makes sure internally generated queries use SERIALIZABLE would
incorrectly catch these queries executed on behalf of COPY.

This also removes an outdated comment on the isql.Rows interface. This
comment is no longer true ever since 0316935492e26a58741b4c3d0e924c007bd8c541
was merged.

Release note: None
---
 pkg/sql/copy/BUILD.bazel                     |  1 +
 pkg/sql/copy/copy_test.go                    |  2 ++
 pkg/sql/copy/testdata/copy_to_read_committed | 27 ++++++++++++++++++++
 pkg/sql/copy_to.go                           |  4 +--
 pkg/sql/isql/isql_db.go                      |  3 ---
 5 files changed, 32 insertions(+), 5 deletions(-)
 create mode 100644 pkg/sql/copy/testdata/copy_to_read_committed

diff --git a/pkg/sql/copy/BUILD.bazel b/pkg/sql/copy/BUILD.bazel
index a587b7e314dc..6aa12567a4dd 100644
--- a/pkg/sql/copy/BUILD.bazel
+++ b/pkg/sql/copy/BUILD.bazel
@@ -13,6 +13,7 @@ go_test(
     shard_count = 4,
     deps = [
         "//pkg/base",
+        "//pkg/ccl",
         "//pkg/cli/clisqlclient",
         "//pkg/kv",
         "//pkg/kv/kvclient/kvcoord",
diff --git a/pkg/sql/copy/copy_test.go b/pkg/sql/copy/copy_test.go
index 9fd5f1ba09d3..fc4af2c93509 100644
--- a/pkg/sql/copy/copy_test.go
+++ b/pkg/sql/copy/copy_test.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/cockroachdb/apd/v3"
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/ccl"
 	"github.com/cockroachdb/cockroach/pkg/cli/clisqlclient"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
@@ -89,6 +90,7 @@ const csvData = `%d|155190|7706|1|17|21168.23|0.04|0.02|N|O|1996-03-13|1996-02-1
 
 func TestDataDriven(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer ccl.TestingEnableEnterprise()() // allow usage of READ COMMITTED
 	ctx := context.Background()
 
 	doTest := func(t *testing.T, d *datadriven.TestData, conn clisqlclient.Conn) string {
diff --git a/pkg/sql/copy/testdata/copy_to_read_committed b/pkg/sql/copy/testdata/copy_to_read_committed
new file mode 100644
index 000000000000..98715239c753
--- /dev/null
+++ b/pkg/sql/copy/testdata/copy_to_read_committed
@@ -0,0 +1,27 @@
+# Verify that the default transaction isolation level is applied to the
+# internal queries issued by the COPY command.
+exec-ddl
+SET default_transaction_isolation = 'read committed'
+----
+
+copy-to
+COPY (SELECT current_setting('transaction_isolation')) TO STDOUT
+----
+read committed
+
+copy-to
+COPY (SELECT 1, 2, 3) TO STDOUT
+----
+1	2	3
+
+copy-to
+COPY (
+  SELECT repeat('a' || i::STRING, 5), repeat('b' || i::STRING, 5)
+  FROM ROWS FROM (generate_series(1, 5)) AS i
+) TO STDOUT;
+----
+a1a1a1a1a1	b1b1b1b1b1
+a2a2a2a2a2	b2b2b2b2b2
+a3a3a3a3a3	b3b3b3b3b3
+a4a4a4a4a4	b4b4b4b4b4
+a5a5a5a5a5	b5b5b5b5b5
diff --git a/pkg/sql/copy_to.go b/pkg/sql/copy_to.go
index 8bfd0d6ad4f8..a088e20bc257 100644
--- a/pkg/sql/copy_to.go
+++ b/pkg/sql/copy_to.go
@@ -186,10 +186,10 @@ func runCopyTo(
 		}).String()
 	}
 
-	it, err := p.execCfg.InternalDB.NewInternalExecutor(p.SessionData()).QueryIteratorEx(
+	it, err := p.InternalSQLTxn().QueryIteratorEx(
 		ctx,
 		cmd.Stmt.String(),
-		txn,
+		p.Txn(),
 		sessiondata.NoSessionDataOverride,
 		q,
 	)
diff --git a/pkg/sql/isql/isql_db.go b/pkg/sql/isql/isql_db.go
index 554938036b96..701f2e33fc37 100644
--- a/pkg/sql/isql/isql_db.go
+++ b/pkg/sql/isql/isql_db.go
@@ -240,9 +240,6 @@ type Rows interface {
 	// Types returns the types of the columns returned by this iterator. The
 	// returned array is guaranteed to correspond 1:1 with the tree.Datums rows
 	// returned by Cur().
-	//
-	// WARNING: this method is safe to call anytime *after* the first call to
-	// Next() (including after Close() was called).
 	Types() colinfo.ResultColumns
 
 	// HasResults returns true if there are results to the query, false otherwise.