sql: support placeholders in logictest

[mberhault]

We really don't test placeholders very much, and I'm running into a lot of issues with them when changing out sql shell to the postgres driver.

We should probably add support for those in the logic tests.
The tedious part is handling all types. We could do something similar to the query <col types> statement.

[petermattis]

I think sql/pgwire_test.go is a more appropriate place to test our placeholder support.

[tamird]

Agreed.

On Thu, Jan 14, 2016 at 2:01 PM, Peter Mattis notifications@github.com
wrote:

I think sql/pgwire_test.go is a more appropriate place to test our
placeholder support.

—
Reply to this email directly or view it on GitHub
#3854 (comment)
.

[mberhault]

wouldn't the testdata format be much simpler to do vast coverage of things? it shouldn't be too complicated to make the logictest handle these.

[bdarnell]

In #3819, we've discovered that some commands can fail when the extended query protocol is used, but fail with the simple query protocol used in the logic tests. To make sure we've fixed all of these we need to make sure that all statement types are covered by tests that use placeholders. We'd be more likely to have comprehensive test coverage of placeholder usage if they could be written in the more familiar logic test format, although I'm not sure if that justifies the complexity of all the parsing and reflection that would be required.

[maddyblue]

As of #4037, there are many tests in pgwire_test.go to test placeholders. Are these suffient?

[petermattis]

I'm not convinced about adding something logic-test specific to support placeholders, but adding support for PREPARE and EXECUTE statements would allow us to test placeholders from the logic test framework and elsewhere. The downside of PREPARE and EXECUTE is that they would not be the same code path as pgwire prepare/execute.

[RaduBerinde]

I'll add my 2 cents - AFAICT pgwire_test has some limitations which makes a lot of things hard to test:

• statements can't be executed in a certain order (they're in a map), so you can't do even simple things like insert then select
• queries can only return exactly one row (technically they have to return at least one row and we only verify the first one)
• tests are broken up into Exec and Queries; the infrastructure should allow both kinds in a single test, and tests should be split up according to what area they test (similar to how logic tests are split up)

If we do not go the logic test route and pgwire_test has to suffice, then we should fix these limitations.

[maddyblue]

I'm in favor of fixing all of the limitations listed by Radu in pgwire_test and leaving the logic tests alone.

[tamird]

+1 to fixing limitations in pgwire_test and leaving logic tests alone. trying to do the correct thing with respect to types in logic tests won't be possible without heavyweight additions to the syntax to support type annotations.

[tamird]

Doesn't sound like anyone's rushing to do this, and the PREPARE/EXECUTE/RELEASE work is tracked in #7123.