Skip to content

Commit

Permalink
Merge cockroachdb#88196
Browse files Browse the repository at this point in the history 
88196: sql,copy: fix test infrastructure and update pgconn r=cucaroach a=cucaroach

sql/copy: update jackc/pgconn to fix copy bug

Fixes: cockroachdb#88801

Release note: None

sql,copy: fix test infrastructure

Previously we hacked around lack of copy protocol support in logictest
by doing an in memory hack that bypassed the client connection entirely.
Now that the client supports use pgx to support copy use that instead.

Also move copy test stuff to its own package.

Fixes: cockroachdb#84619

Release note: None

Co-authored-by: Tommy Reilly <[email protected]>
  • Loading branch information
@cucaroach
craig[bot] and cucaroach committed Oct 13, 2022
2 parents 4c2c7da + 2113dbc commit 05f6c6e
Show file tree
Hide file tree
Showing 24 changed files with 459 additions and 673 deletions.
12 changes: 6 additions & 6 deletions DEPS.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4489,10 +4489,10 @@ def go_deps():
name = "com_github_jackc_pgconn",
build_file_proto_mode = "disable_global",
importpath = "github.com/jackc/pgconn",
sha256 = "48d34064a1facff7766713d9224502e7376a5d90c1506f99a37c57bfceaf9636",
strip_prefix = "github.com/jackc/pgconn@v1.12.1",
sha256 = "4d9bf1309f5cdbd589d60a485fb5d5d7333edf9652c2dd47b7dd31b12dda887e",
strip_prefix = "github.com/jackc/pgconn@v1.13.1-0.20221001150415-49cbf4659151",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgconn/com_github_jackc_pgconn-v1.12.1.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgconn/com_github_jackc_pgconn-v1.13.1-0.20221001150415-49cbf4659151.zip",
],
)
go_repository(
Expand Down Expand Up @@ -4539,10 +4539,10 @@ def go_deps():
name = "com_github_jackc_pgproto3_v2",
build_file_proto_mode = "disable_global",
importpath = "github.com/jackc/pgproto3/v2",
sha256 = "6b702c372e13520636243d3be58922968f0630b67e23ba77326ef6ee4cada463",
strip_prefix = "github.com/jackc/pgproto3/[email protected].0",
sha256 = "57884e299825af31fd01268659f1e671883b73b708a51230da14d6f8ee0e4e36",
strip_prefix = "github.com/jackc/pgproto3/[email protected].1",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgproto3/v2/com_github_jackc_pgproto3_v2-v2.3.0.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgproto3/v2/com_github_jackc_pgproto3_v2-v2.3.1.zip",
],
)
go_repository(
Expand Down
4 changes: 2 additions & 2 deletions build/bazelutil/distdir_files.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,12 +487,12 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/j-keck/arping/com_github_j_keck_arping-v0.0.0-20160618110441-2cf9dc699c56.zip": "6001c94a8c4eed55718f627346cb685cce67369ca5c29ae059f58f7abd8bd8a7",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/chunkreader/com_github_jackc_chunkreader-v1.0.0.zip": "e204c917e2652ffe047f5c8b031192757321f568654e3df8408bf04178df1408",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/chunkreader/v2/com_github_jackc_chunkreader_v2-v2.0.1.zip": "6e3f4b7d9647f31061f6446ae10de71fc1407e64f84cd0949afac0cd231e8dd2",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgconn/com_github_jackc_pgconn-v1.12.1.zip": "48d34064a1facff7766713d9224502e7376a5d90c1506f99a37c57bfceaf9636",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgconn/com_github_jackc_pgconn-v1.13.1-0.20221001150415-49cbf4659151.zip": "4d9bf1309f5cdbd589d60a485fb5d5d7333edf9652c2dd47b7dd31b12dda887e",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgio/com_github_jackc_pgio-v1.0.0.zip": "1a83c03d53f6a40339364cafcbbabb44238203c79ca0c9b98bf582d0df0e0468",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgmock/com_github_jackc_pgmock-v0.0.0-20210724152146-4ad1a8207f65.zip": "0fffd0a7a67dbdfafa04297e51028c6d2d08cd6691f3b6d78d7ae6502d3d4cf2",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgpassfile/com_github_jackc_pgpassfile-v1.0.0.zip": "1cc79fb0b80f54b568afd3f4648dd1c349f746ad7c379df8d7f9e0eb1cac938b",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgproto3/com_github_jackc_pgproto3-v1.1.0.zip": "e3766bee50ed74e49a067b2c4797a2c69015cf104bf3f3624cd483a9e940b4ee",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgproto3/v2/com_github_jackc_pgproto3_v2-v2.3.0.zip": "6b702c372e13520636243d3be58922968f0630b67e23ba77326ef6ee4cada463",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgproto3/v2/com_github_jackc_pgproto3_v2-v2.3.1.zip": "57884e299825af31fd01268659f1e671883b73b708a51230da14d6f8ee0e4e36",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgservicefile/com_github_jackc_pgservicefile-v0.0.0-20200714003250-2b9c44734f2b.zip": "8422a25b9d2b0be05c66ee1ccfdbaab144ce98f1ac678bc647064c560d4cd6e2",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgtype/com_github_jackc_pgtype-v1.11.0.zip": "6a257b81c0bd386d6241219a14ebd41d574a02aeaeb3942670c06441b864dcad",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/jackc/pgx/v4/com_github_jackc_pgx_v4-v4.16.1.zip": "c3a169a68ff0e56f9f81eee4de4d2fd2a5ec7f4d6be159159325f4863c80bd10",
Expand Down
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ require (
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/guptarohit/asciigraph v0.5.5
github.com/irfansharif/recorder v0.0.0-20211218081646-a21b46510fd6
github.com/jackc/pgconn v1.12.1
github.com/jackc/pgproto3/v2 v2.3.0
github.com/jackc/pgconn v1.13.1-0.20221001150415-49cbf4659151
github.com/jackc/pgproto3/v2 v2.3.1
github.com/jackc/pgtype v1.11.0
github.com/jackc/pgx/v4 v4.16.1
github.com/jaegertracing/jaeger v1.18.1
Expand Down
7 changes: 5 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1316,8 +1316,9 @@ github.com/jackc/pgconn v1.5.1-0.20200601181101-fa742c524853/go.mod h1:QeD3lBfpT
github.com/jackc/pgconn v1.8.0/go.mod h1:1C2Pb36bGIP9QHGBYCjnyhqu7Rv3sGshaQUvmfGIB/o=
github.com/jackc/pgconn v1.9.0/go.mod h1:YctiPyvzfU11JFxoXokUOOKQXQmDMoJL9vJzHH8/2JY=
github.com/jackc/pgconn v1.9.1-0.20210724152538-d89c8390a530/go.mod h1:4z2w8XhRbP1hYxkpTuBjTS3ne3J48K83+u0zoyvg2pI=
github.com/jackc/pgconn v1.12.1 h1:rsDFzIpRk7xT4B8FufgpCCeyjdNpKyghZeSefViE5W8=
github.com/jackc/pgconn v1.12.1/go.mod h1:ZkhRC59Llhrq3oSfrikvwQ5NaxYExr6twkdkMLaKono=
github.com/jackc/pgconn v1.13.1-0.20221001150415-49cbf4659151 h1:bPKk32KlGLQ/SWtOc3onTzZ/km0BIiwwYBqt+8YlKwU=
github.com/jackc/pgconn v1.13.1-0.20221001150415-49cbf4659151/go.mod h1:AnowpAqO4CMIIJNZl2VJp+KrkAZciAkhEl0W0JIobpI=
github.com/jackc/pgio v1.0.0 h1:g12B9UwVnzGhueNavwioyEEpAmqMe1E/BN9ES+8ovkE=
github.com/jackc/pgio v1.0.0/go.mod h1:oP+2QK2wFfUWgr+gxjoBH9KGBb31Eio69xUb0w5bYf8=
github.com/jackc/pgmock v0.0.0-20190831213851-13a1b77aafa2/go.mod h1:fGZlG77KXmcq05nJLRkk0+p82V8B8Dw8KN2/V9c/OAE=
Expand All @@ -1334,8 +1335,9 @@ github.com/jackc/pgproto3/v2 v2.0.0-rc3.0.20190831210041-4c03ce451f29/go.mod h1:
github.com/jackc/pgproto3/v2 v2.0.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
github.com/jackc/pgproto3/v2 v2.0.6/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
github.com/jackc/pgproto3/v2 v2.1.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
github.com/jackc/pgproto3/v2 v2.3.0 h1:brH0pCGBDkBW07HWlN/oSBXrmo3WB0UvZd1pIuDcL8Y=
github.com/jackc/pgproto3/v2 v2.3.0/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
github.com/jackc/pgproto3/v2 v2.3.1 h1:nwj7qwf0S+Q7ISFfBndqeLwSwxs+4DPsbRFjECT1Y4Y=
github.com/jackc/pgproto3/v2 v2.3.1/go.mod h1:WfJCnwN3HIg9Ish/j3sgWXnAfK8A9Y0bwXYU5xKaEdA=
github.com/jackc/pgservicefile v0.0.0-20200307190119-3430c5407db8/go.mod h1:vsD4gTJCa9TptPL8sPkXrLZ+hDuNrZCnj29CQpr4X1E=
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b h1:C8S2+VttkHFdOOCXJe+YGfa4vHYwlt4Zx+IVXQ97jYg=
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b/go.mod h1:vsD4gTJCa9TptPL8sPkXrLZ+hDuNrZCnj29CQpr4X1E=
Expand Down Expand Up @@ -2361,6 +2363,7 @@ golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5y
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.0.0-20220210151621-f4118a5b28e2/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220427172511-eb4f295cb31f/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220817201139-bc19a97f63c8 h1:GIAS/yBem/gq2MUqgNIzUHW7cJMmx3TGZOrnyYaNQ6c=
golang.org/x/crypto v0.0.0-20220817201139-bc19a97f63c8/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand Down
3 changes: 3 additions & 0 deletions pkg/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ ALL_TESTS = [
"//pkg/sql/contention/txnidcache:txnidcache_test",
"//pkg/sql/contention:contention_test",
"//pkg/sql/contentionpb:contentionpb_test",
"//pkg/sql/copy:copy_test",
"//pkg/sql/covering:covering_test",
"//pkg/sql/decodeusername:decodeusername_test",
"//pkg/sql/delegate:delegate_test",
Expand Down Expand Up @@ -1466,6 +1467,7 @@ GO_TARGETS = [
"//pkg/sql/contention:contention_test",
"//pkg/sql/contentionpb:contentionpb",
"//pkg/sql/contentionpb:contentionpb_test",
"//pkg/sql/copy:copy_test",
"//pkg/sql/covering:covering",
"//pkg/sql/covering:covering_test",
"//pkg/sql/decodeusername:decodeusername",
Expand Down Expand Up @@ -2643,6 +2645,7 @@ GET_X_DATA_TARGETS = [
"//pkg/sql/contention/contentionutils:get_x_data",
"//pkg/sql/contention/txnidcache:get_x_data",
"//pkg/sql/contentionpb:get_x_data",
"//pkg/sql/copy:get_x_data",
"//pkg/sql/covering:get_x_data",
"//pkg/sql/decodeusername:get_x_data",
"//pkg/sql/delegate:get_x_data",
Expand Down
13 changes: 9 additions & 4 deletions pkg/cli/clisqlclient/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ type TxBoundConn interface {
type DriverConn interface {
Query(ctx context.Context, query string, args ...interface{}) (driver.Rows, error)
Exec(ctx context.Context, query string, args ...interface{}) error
CopyFrom(ctx context.Context, reader io.Reader, query string) error
CopyFrom(ctx context.Context, reader io.Reader, query string) (int64, error)
}

type driverConnAdapter struct {
Expand All @@ -200,7 +200,12 @@ func (d *driverConnAdapter) Exec(ctx context.Context, query string, args ...inte
return d.c.Exec(ctx, query, args...)
}

func (d *driverConnAdapter) CopyFrom(ctx context.Context, reader io.Reader, query string) error {
_, err := d.c.conn.PgConn().CopyFrom(ctx, reader, query)
return err
func (d *driverConnAdapter) CopyFrom(
ctx context.Context, reader io.Reader, query string,
) (int64, error) {
cmdTag, err := d.c.conn.PgConn().CopyFrom(ctx, reader, query)
if err != nil {
return -1, err
}
return cmdTag.RowsAffected(), nil
}
2 changes: 1 addition & 1 deletion pkg/cli/nodelocal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func uploadFile(
}
}

if err := ex.CopyFrom(ctx, bytes.NewReader(send), stmt); err != nil {
if _, err := ex.CopyFrom(ctx, bytes.NewReader(send), stmt); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/userfile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ func uploadUserFile(
}
}

if err := ex.CopyFrom(ctx, bytes.NewReader(send), stmt); err != nil {
if _, err := ex.CopyFrom(ctx, bytes.NewReader(send), stmt); err != nil {
return "", err
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ go_library(
"control_schedules.go",
"copy.go",
"copy_file_upload.go",
"copyshim.go",
"crdb_internal.go",
"create_database.go",
"create_extension.go",
Expand Down Expand Up @@ -508,7 +507,6 @@ go_library(
"@com_github_cockroachdb_redact//:redact",
"@com_github_gogo_protobuf//proto",
"@com_github_gogo_protobuf//types",
"@com_github_jackc_pgproto3_v2//:pgproto3",
"@com_github_lib_pq//:pq",
"@com_github_lib_pq//oid",
"@com_github_prometheus_client_model//go",
Expand Down Expand Up @@ -545,7 +543,6 @@ go_test(
"conn_executor_test.go",
"conn_io_test.go",
"constraint_test.go",
"copy_from_test.go",
"copy_in_test.go",
"copy_test.go",
"crdb_internal_test.go",
Expand Down
14 changes: 13 additions & 1 deletion pkg/sql/copy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/buildutil"
"github.com/cockroachdb/cockroach/pkg/util/encoding/csv"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/mon"
Expand All @@ -46,6 +47,18 @@ const CopyBatchRowSizeDefault = 100
// When this many rows are in the copy buffer, they are inserted.
var copyBatchRowSize = util.ConstantWithMetamorphicTestRange("copy-batch-size", CopyBatchRowSizeDefault, 1, 10000)

// SetCopyFromBatchSize exports overriding copy batch size for test code.
func SetCopyFromBatchSize(i int) int {
old := copyBatchRowSize
if buildutil.CrdbTestBuild {
copyBatchRowSize = i
} else {
// We don't want non-test code mutating globals.
panic("SetCopyFromBatchSize is a test utility that requires crdb_test tag")
}
return old
}

type copyMachineInterface interface {
run(ctx context.Context) error
numInsertedRows() int
Expand Down Expand Up @@ -767,7 +780,6 @@ func (p *planner) preparePlannerForCopy(
txn = kv.NewTxnWithSteppingEnabled(ctx, p.execCfg.DB, nodeID, sessiondatapb.Normal)
txnTs = p.execCfg.Clock.PhysicalTime()
stmtTs = txnTs

}
txnOpt.resetPlanner(ctx, p, txn, txnTs, stmtTs)
if implicitTxn {
Expand Down
35 changes: 35 additions & 0 deletions pkg/sql/copy/BUILD.bazel
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data")
load("@io_bazel_rules_go//go:def.bzl", "go_test")

go_test(
name = "copy_test",
srcs = [
"copy_test.go",
"main_test.go",
],
args = ["-test.timeout=295s"],
data = glob(["testdata/**"]),
deps = [
"//pkg/base",
"//pkg/cli/clisqlclient",
"//pkg/security/securityassets",
"//pkg/security/securitytest",
"//pkg/security/username",
"//pkg/server",
"//pkg/settings/cluster",
"//pkg/sql",
"//pkg/testutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/randutil",
"@com_github_cockroachdb_apd_v3//:apd",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_jackc_pgtype//:pgtype",
"@com_github_stretchr_testify//require",
],
)

get_x_data(name = "get_x_data")
Loading

0 comments on commit 05f6c6e

Please sign in to comment.