dolthub / doltgresql

DoltgreSQL - Version Controlled PostgreSQL
Apache License 2.0
1.12k stars 25 forks source link

Bug fix: Make `db.Ping(ctx)` work correctly with Doltgres #887

Closed fulghum closed 1 month ago

fulghum commented 1 month ago

Implementations of db.Ping(ctx) from Go's database/sql package typically send some sort of empty query to test the liveness of a database connection. For example, the pq library sends ; and the pgx library sends -- ping.

Before this change, Doltgres' parser would return an empty statement, instead of Vitess' ErrEmpty error, which caused GMS to try and execute a nil analyzed plan for these statements, return an error, and the ping check would fail. This change makes the Doltgres parser return the same Vitess ErrEmpty error that the Vitess parser throws, sending the same signal to GMS to handle an empty statement without returning an error to the client. The related PR https://github.com/dolthub/go-mysql-server/pull/2716 updates the Parser interface documentation to explicitly call out the requirement to return ErrEmpty in order for empty statements to be handled correctly.

For testing, I've added some no-op statements to the smoke tests, but since these tests go through as prepared statements, they don't follow the exact code path as db.Ping(ctx), so I've also added calls to db.Ping(ctx) in the test framework, which do reproduce this error. I've also added a unit test for Doltgres' Parser implementation that explicitly tests empty statement parsing.

Fixes: https://github.com/dolthub/doltgresql/issues/884 Related to: https://github.com/dolthub/go-mysql-server/pull/2716

github-actions[bot] commented 1 month ago
Main PR
Total 42090 42090
Successful 12867 12876
Failures 29223 29214
Partial Successes[^1] 4892 4892
Main PR
Successful 30.5702% 30.5916%
Failures 69.4298% 69.4084%

${\color{lightgreen}Progressions}$

comments

QUERY: /* and this is the end of the file */

insert_conflict

QUERY: ;
QUERY: ;
QUERY: ;
QUERY: ;

portals

QUERY: CLOSE ALL;
QUERY: CLOSE ALL;

[^1]: These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.