Merge pull request #887 from dolthub/fulghum/ping

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

Author: fulghum

postgres/parser/parser/parse.go

@@ -38,6 +38,7 @@ import (
 	"strings"
 
 	"github.com/cockroachdb/errors"
+	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
 	"github.com/dolthub/doltgresql/postgres/parser/pgcode"
 	"github.com/dolthub/doltgresql/postgres/parser/pgerror"
@@ -120,8 +121,10 @@ func (p *Parser) parseOneWithDepth(depth int, sql string) (Statement, error) {
 	if err != nil {
 		return Statement{}, err
 	}
-	if len(stmts) != 1 {
+	if len(stmts) > 1 {
 		return Statement{}, errors.AssertionFailedf("expected 1 statement, but found %d", len(stmts))
+	} else if len(stmts) == 0 {
+		return Statement{}, vitess.ErrEmpty
 	}
 	return stmts[0], nil
 }

postgres/parser/parser/sql/sql_parser.go

@@ -56,7 +56,7 @@ func (p *PostgresParser) ParseWithOptions(ctx context.Context, query string, del
 		return nil, "", "", fmt.Errorf("only a single statement at a time is currently supported")
 	}
 	if len(stmts) == 0 {
-		return nil, q, "", nil
+		return nil, q, "", vitess.ErrEmpty
 	}
 
 	vitessAST, err := ast.Convert(stmts[0])

testing/go/framework.go

@@ -130,6 +130,7 @@ func RunScript(t *testing.T, script ScriptTest, normalizeRows bool) {
 			Default: pgxConn,
 			Current: pgxConn,
 		}
+		require.NoError(t, pgxConn.Ping(ctx))
 		defer func() {
 			conn.Close(ctx)
 		}()
@@ -309,6 +310,9 @@ func CreateServer(t *testing.T, database string) (context.Context, *Connection,
 
 	conn, err := pgx.Connect(ctx, fmt.Sprintf("postgres://postgres:[email protected]:%d/%s", port, database))
 	require.NoError(t, err)
+
+	// Ping the connection to test that it's alive, and also to test that Doltgres handles empty queries correctly
+	require.NoError(t, conn.Ping(ctx))
 	return ctx, &Connection{
 		Default: conn,
 		Current: conn,

testing/go/smoke_test.go

@@ -71,6 +71,18 @@ func TestSmokeTests(t *testing.T) {
 					Query:    "SELECT NULL = NULL",
 					Expected: []sql.Row{{nil}},
 				},
+				{
+					Query:    ";",
+					Expected: []sql.Row{},
+				},
+				{
+					Query:    " ; ",
+					Expected: []sql.Row{},
+				},
+				{
+					Query:    "-- this is only a comment",
+					Expected: []sql.Row{},
+				},
 			},
 		},
 		{

testing/go/sql_parser_test.go

@@ -0,0 +1,52 @@
+// Copyright 2024 Dolthub, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package _go
+
+import (
+	"context"
+	"testing"
+
+	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
+	"github.com/stretchr/testify/require"
+
+	"github.com/dolthub/doltgresql/postgres/parser/parser/sql"
+)
+
+// TestParserBehaviors tests behaviors, such as empty statement handling, for the Doltgres parser.
+func TestParserBehaviors(t *testing.T) {
+	parser := sql.NewPostgresParser()
+
+	// Parser implementations should return Vitess' ErrEmpty error for empty statements.
+	t.Run("empty statement parsing", func(t *testing.T) {
+		emptyStatements := []string{"", " ", "\t", ";", "-- comment", "/* comment */"}
+		for _, statement := range emptyStatements {
+			parsed, err := parser.ParseSimple(statement)
+			require.Nil(t, parsed)
+			require.ErrorIs(t, err, vitess.ErrEmpty)
+
+			parsed, _, _, err = parser.Parse(nil, statement, true)
+			require.Nil(t, parsed)
+			require.ErrorIs(t, err, vitess.ErrEmpty)
+
+			parsed, _, err = parser.ParseOneWithOptions(context.Background(), statement, vitess.ParserOptions{})
+			require.Nil(t, parsed)
+			require.ErrorIs(t, err, vitess.ErrEmpty)
+
+			parsed, _, _, err = parser.ParseWithOptions(context.Background(), statement, ';', false, vitess.ParserOptions{})
+			require.Nil(t, parsed)
+			require.ErrorIs(t, err, vitess.ErrEmpty)
+		}
+	})
+}