Skip to content

Commit

Permalink
Bug fix for db.Ping(ctx) against a Doltgres server
Browse files Browse the repository at this point in the history
  • Loading branch information
fulghum committed Oct 24, 2024
1 parent c84a300 commit 2c3276e
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 2 deletions.
5 changes: 4 additions & 1 deletion postgres/parser/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion postgres/parser/parser/sql/sql_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
4 changes: 4 additions & 0 deletions testing/go/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}()
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions testing/go/smoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
},
},
},
{
Expand Down
52 changes: 52 additions & 0 deletions testing/go/sql_parser_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}

0 comments on commit 2c3276e

Please sign in to comment.