Skip to content

Commit

Permalink
sql/copy: use the proper interface for internal queries
Browse files Browse the repository at this point in the history
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 0316935
was merged.

Release note: None
  • Loading branch information
rafiss committed Oct 6, 2024
1 parent 63dd348 commit dadd46e
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 5 deletions.
1 change: 1 addition & 0 deletions pkg/sql/copy/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ go_test(
shard_count = 4,
deps = [
"//pkg/base",
"//pkg/ccl",
"//pkg/cli/clisqlclient",
"//pkg/kv",
"//pkg/kv/kvclient/kvcoord",
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/copy/copy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/copy/testdata/copy_to_read_committed
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions pkg/sql/copy_to.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/isql/isql_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit dadd46e

Please sign in to comment.