clisqlshell: implement COPY ... FROM STDIN for CLI

[otan]

Resolves #16392

Steps:

• Add a lower level API to lib/pq for use.
• Add some abstraction boundary breakers in clisqlclient that allow a
  lower level handling of the COPY protocol.
• Altered the state machine in clisqlshell to account for copy.

Release note (cli change): COPY ... FROM STDIN now works from the
cockroach CLI. It is not supported inside transactions.

[cockroach-teamcity]

This change is 

[knz]

The general design is very nice 💯

I have one remaiining concern on semantics, see below.

Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @rafiss)

---

pkg/cli/clisqlshell/sql.go, line 1703 at r1 (raw file):

	c.exitErr = c.runWithInterruptableCtx(func(ctx context.Context) error {
		if scanner.FirstLexicalToken(c.concatLines) == lexbase.COPY {
			return c.beginCopyFrom(ctx, c.concatLines)

wowzer, how does this behave if the user enters copy ...; select ... as one line at the prompt?
I want to see tests for this.

I think the code should refuse to treat COPY as a special case unless c.singleStatement is true.

[otan]

pkg/cli/clisqlshell/sql.go, line 1703 at r1 (raw file):

Previously, knz (kena) wrote…

wowzer, how does this behave if the user enters copy ...; select ... as one line at the prompt?
I want to see tests for this.

I think the code should refuse to treat COPY as a special case unless c.singleStatement is true.

there is a test for this:

start_test "multi statement with COPY"
send "SELECT 1; COPY t FROM STDIN CSV;\r"
eexpect "COPY together with other statements in a query string is not supported"
eexpect root@
send "COPY t FROM STDIN CSV;SELECT 1;\r"
eexpect "COPY together with other statements in a query string is not supported"
eexpect root@
end_test

[otan]

pkg/cli/clisqlshell/sql.go, line 1703 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

there is a test for this:

start_test "multi statement with COPY"
send "SELECT 1; COPY t FROM STDIN CSV;\r"
eexpect "COPY together with other statements in a query string is not supported"
eexpect root@
send "COPY t FROM STDIN CSV;SELECT 1;\r"
eexpect "COPY together with other statements in a query string is not supported"
eexpect root@
end_test

(it's already handled by the driver)

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @otan)

---

go.mod, line 373 at r2 (raw file):

replace github.com/maruel/panicparse/v2 => github.com/cockroachdb/panicparse/v2 v2.0.0-20211103220158-604c82a44f1e

replace github.com/lib/pq => github.com/cockroachdb/pq v0.0.0-20220408053752-c3ffc8d4376f

i think this fork is missing some of the latest lib/pq changes (they don't look too important, but might as well include them, or merge them into the upstream)

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @otan)

---

pkg/cli/clisqlshell/sql.go, line 1703 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

(it's already handled by the driver)

to clarify, the "COPY together with other statements in a query string is not supported" message is actually coming from the CRDB server:

cockroach/pkg/sql/pgwire/conn.go

Lines 846 to 854 in e4a9f66

	// NOTE(andrei): I don't know if Postgres supports receiving a COPY
	// together with other statements in the "simple" protocol, but I'd
	// rather not worry about it since execution of COPY is special - it
	// takes control over the connection.
	return c.stmtBuf.Push(
	ctx,
	sql.SendError{
	Err: pgwirebase.NewProtocolViolationErrorf(
	"COPY together with other statements in a query string is not supported"),

[otan]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @rafiss)

---

go.mod, line 373 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i think this fork is missing some of the latest lib/pq changes (they don't look too important, but might as well include them, or merge them into the upstream)

last time i did some tests failed; also in terms of backportability this is much neater.

---

pkg/cli/clisqlshell/sql.go, line 1703 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

to clarify, the "COPY together with other statements in a query string is not supported" message is actually coming from the CRDB server:

cockroach/pkg/sql/pgwire/conn.go

Lines 846 to 854 in e4a9f66

	// NOTE(andrei): I don't know if Postgres supports receiving a COPY
	// together with other statements in the "simple" protocol, but I'd
	// rather not worry about it since execution of COPY is special - it
	// takes control over the connection.
	return c.stmtBuf.Push(
	ctx,
	sql.SendError{
	Err: pgwirebase.NewProtocolViolationErrorf(
	"COPY together with other statements in a query string is not supported"),

even better!

[knz]

huh I hadn't seen you introduced a fork of lib/pq here. can you upstream them directly instead? we have a commit bit there.

[otan]

i've tried upgrading lib/pq instead, let's see if all tests pass.

[otan]

ok looks good

[knz]

Reviewed 1 of 1 files at r2, 5 of 6 files at r3, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @rafiss)

[knz]

thank you!

[otan]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)