Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dont panic batch on send after invalid append #830

Merged
merged 2 commits into from
Nov 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clickhouse.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type (
)

var (
ErrBatchInvalid = errors.New("clickhouse: batch is invalid. check appended data is correct")
ErrBatchAlreadySent = errors.New("clickhouse: batch has already been sent")
ErrAcquireConnTimeout = errors.New("clickhouse: acquire conn timeout. you can increase the number of max open conn or the dial timeout")
ErrUnsupportedServerRevision = errors.New("clickhouse: unsupported server revision")
Expand Down
2 changes: 2 additions & 0 deletions conn_batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package clickhouse
import (
"context"
"fmt"
"github.com/pkg/errors"
"os"
"regexp"
"strings"
Expand Down Expand Up @@ -105,6 +106,7 @@ func (b *batch) Append(v ...interface{}) error {
}
//
if err := b.block.Append(v...); err != nil {
b.err = errors.Wrap(ErrBatchInvalid, err.Error())
b.release(err)
return err
}
Expand Down
53 changes: 53 additions & 0 deletions tests/issues/798_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package issues

import (
"context"
"github.com/ClickHouse/clickhouse-go/v2"
clickhouse_tests "github.com/ClickHouse/clickhouse-go/v2/tests"
"github.com/stretchr/testify/require"
"testing"
)

func Test798(t *testing.T) {
var (
conn, err = clickhouse_tests.GetConnection("issues", clickhouse.Settings{
"max_execution_time": 60,
}, nil, &clickhouse.Compression{
Method: clickhouse.CompressionLZ4,
})
)
ctx := context.Background()
require.NoError(t, err)
const ddl = `
CREATE TABLE test_issue_798 (
Col1 Bool
, Col2 Bool
, Col3 Array(Bool)
) Engine MergeTree() ORDER BY tuple()
`
defer func() {
conn.Exec(ctx, "DROP TABLE IF EXISTS test_issue_798")
}()
require.NoError(t, conn.Exec(ctx, ddl))
batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_798")
require.NoError(t, err)
require.NoError(t, batch.Append(true, false, []bool{true, false, true}))
require.NoError(t, batch.Send())
// test resend
require.ErrorIs(t, batch.Send(), clickhouse.ErrBatchAlreadySent)
batch, err = conn.PrepareBatch(ctx, "INSERT INTO test_issue_798")
require.NoError(t, err)
// test empty batch
require.NoError(t, batch.Send())
batch, err = conn.PrepareBatch(ctx, "INSERT INTO test_issue_798")
// append invalid batch
require.Error(t, batch.Append("true", false, []bool{true, false, true}))
// send invalid batch
require.ErrorIs(t, batch.Flush(), clickhouse.ErrBatchInvalid)
require.ErrorIs(t, batch.Send(), clickhouse.ErrBatchInvalid)
// test append, send, append
batch, err = conn.PrepareBatch(ctx, "INSERT INTO test_issue_798")
require.NoError(t, batch.Append(true, false, []bool{true, false, true}))
require.NoError(t, batch.Send())
require.ErrorIs(t, batch.Append(true, false, []bool{true, false, true}), clickhouse.ErrBatchAlreadySent)
}