ch-go icon indicating copy to clipboard operation
ch-go copied to clipboard

Data loss when EOF in first block

Open Hamper opened this issue 2 years ago • 3 comments

Describe the bug

If we don't fill any data before conn.Do and then return io.EOF instead of nil on first OnInput call client doesn't write tail of input data.

If we return nil at least once in OnInput and after that return io.EOF after adding some data then this data will be recorded to table

Steps to reproduce

  1. don't fill any column data in input before conn.Do
  2. return io.EOF on first OnInput call

Expected behaviour

This code must write 10 rows instead of 0

Code example

package main

import (
	"context"
	"io"
	"time"

	"github.com/ClickHouse/ch-go"
	"github.com/ClickHouse/ch-go/proto"
	"go.uber.org/zap"
)

func main() {
	ctx := context.Background()

	logger, _ := zap.NewDevelopment()

	conn, err := ch.Dial(ctx, ch.Options{
		Logger: logger,
	})
	if err != nil {
		panic(err)
	}

	if err := conn.Do(ctx, ch.Query{
		Body: `CREATE TABLE IF NOT EXISTS test_table_insert
(
    ts                DateTime64(9),
    severity_text     Enum8('INFO'=1, 'DEBUG'=2),
    severity_number   UInt8,
    body              String,
    name              String,
    arr               Array(String)
) ENGINE = Memory`,
	}); err != nil {
		panic(err)
	}

	var (
		body      proto.ColStr
		name      proto.ColStr
		sevText   proto.ColEnum
		sevNumber proto.ColUInt8

		ts  = new(proto.ColDateTime64).WithPrecision(proto.PrecisionNano)
		arr = new(proto.ColStr).Array()
		now = time.Date(2010, 1, 1, 10, 22, 33, 345678, time.UTC)
	)

	// just prepare empty data
	input := proto.Input{
		{Name: "ts", Data: ts},
		{Name: "severity_text", Data: &sevText},
		{Name: "severity_number", Data: &sevNumber},
		{Name: "body", Data: &body},
		{Name: "name", Data: &name},
		{Name: "arr", Data: arr},
	}

	var blocks int
	if err := conn.Do(ctx, ch.Query{
		Body:  input.Into("test_table_insert"),
		Input: input,
		OnInput: func(ctx context.Context) error {
			input.Reset()

			for i := 0; i < 10; i++ {
				body.AppendBytes([]byte("Hello"))
				ts.Append(now)
				name.Append("name")
				sevText.Append("DEBUG")
				sevNumber.Append(10)
				arr.Append([]string{"foo", "bar", "baz"})
			}

			if blocks >= 10 {
				// we call this after adding data, in that case we get 110 rows when nil returned in last statement
				return io.EOF
			}

			blocks++
			return io.EOF // instead of nil
		},
	}); err != nil {
		panic(err)
	}
}

Configuration

Environment

  • Client version: v0.58.2
  • Language version: go1.21.1
  • OS: linux

ClickHouse server

  • ClickHouse Server version: 22.1.3.7

Hamper avatar Sep 14 '23 07:09 Hamper

This bug is still there in the latest release v0.65.1.

dstruck avatar Apr 01 '25 07:04 dstruck

@ernado I haven't used this style of implementation before, is this the proper usage?

SpencerTorres avatar Apr 01 '25 16:04 SpencerTorres

@ernado I haven't used this style of implementation before, is this the proper usage?

This is the insert example from https://github.com/ClickHouse/ch-go/blob/main/examples/insert/main.go almost verbatim, except the insert is one block long instead of 10.

And it looks like very natural use of API. Pre-filling Query.Input, before first OnInput callback happens, is a hassle, as it requires you to special-case your first block.

earwin avatar Sep 03 '25 19:09 earwin