Skip to content

Commit 757e102

Browse files
SpencerTorresSpencerTorres
authored
Merge pull request #1594 from ClickHouse/http_client_revision
Add `client_protocol_version` param for HTTP interface
2 parents c234a1f + f0c8a51 commit 757e102

File tree

3 files changed

+19
-59
lines changed

3 files changed

+19
-59
lines changed

‎conn_http.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"net/http"
3434
"net/url"
3535
"os"
36+
"strconv"
3637
"strings"
3738
"sync"
3839
"time"
@@ -215,8 +216,7 @@ func dialHttp(ctx context.Context, addr string, num int, opt *Options) (*httpCon
215216
}
216217

217218
query.Set("default_format", "Native")
218-
// TODO: we support newer revisions but for some reason this completely breaks Native format
219-
//query.Set("client_protocol_version", strconv.Itoa(ClientTCPProtocolVersion))
219+
query.Set("client_protocol_version", strconv.Itoa(ClientTCPProtocolVersion))
220220
u.RawQuery = query.Encode()
221221

222222
httpProxy := http.ProxyFromEnvironment
@@ -251,9 +251,9 @@ func dialHttp(ctx context.Context, addr string, num int, opt *Options) (*httpCon
251251
client: &http.Client{
252252
Transport: t,
253253
},
254-
url: u,
255-
// TODO: learn more about why revision is broken
256-
//revision: ClientTCPProtocolVersion,
254+
url: u,
255+
revision: ClientTCPProtocolVersion, // Preflight uses hardcoded revision, may break older versions.
256+
encodeRevision: 0, // Encoding data over HTTP must use 0. client_protocol_version does not apply to inserts.
257257
buffer: new(chproto.Buffer),
258258
compression: opt.Compression.Method,
259259
blockCompressor: compress.NewWriter(compress.Level(opt.Compression.Level), compress.Method(opt.Compression.Method)),
@@ -266,6 +266,7 @@ func dialHttp(ctx context.Context, addr string, num int, opt *Options) (*httpCon
266266
return nil, fmt.Errorf("failed to query server hello: %w", err)
267267
}
268268
conn.handshake = handshake
269+
conn.revision = conn.handshake.Revision
269270

270271
return &conn, nil
271272
}
@@ -277,6 +278,7 @@ type httpConnect struct {
277278
debugfFunc func(format string, v ...any)
278279
opt *Options
279280
revision uint64
281+
encodeRevision uint64
280282
url *url.URL
281283
client *http.Client
282284
buffer *chproto.Buffer
@@ -407,7 +409,7 @@ func createCompressionPool(compression *Compression) (Pool[HTTPReaderWriter], er
407409
func (h *httpConnect) writeData(block *proto.Block) error {
408410
// Saving offset of compressible data
409411
start := len(h.buffer.Buf)
410-
if err := block.Encode(h.buffer, h.revision); err != nil {
412+
if err := block.Encode(h.buffer, h.encodeRevision); err != nil {
411413
return fmt.Errorf("block encode: %w", err)
412414
}
413415
if h.compression == CompressionLZ4 || h.compression == CompressionZSTD {
@@ -551,7 +553,7 @@ func (h *httpConnect) createRequestWithExternalTables(ctx context.Context, query
551553
return nil, err
552554
}
553555
buf.Reset()
554-
err = table.Block().Encode(buf, h.revision)
556+
err = table.Block().Encode(buf, h.encodeRevision)
555557
if err != nil {
556558
return nil, err
557559
}

‎tests/datetime_test.go‎

Lines changed: 8 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,8 @@ func TestDateTime(t *testing.T) {
9696
assert.Equal(t, datetime.In(time.UTC), col1)
9797
assert.Equal(t, datetime.Unix(), col2.Unix())
9898
assert.Equal(t, datetime.Unix(), col3.Unix())
99-
if protocol == clickhouse.HTTP {
100-
// TODO: investigate client_protocol_version HTTP param
101-
assert.Equal(t, "UTC", col2.Location().String())
102-
assert.Equal(t, "UTC", col3.Location().String())
103-
} else {
104-
assert.Equal(t, "Europe/Moscow", col2.Location().String())
105-
assert.Equal(t, "Europe/London", col3.Location().String())
106-
}
99+
assert.Equal(t, "Europe/Moscow", col2.Location().String())
100+
assert.Equal(t, "Europe/London", col3.Location().String())
107101
assert.Equal(t, datetime.Unix(), col4.Unix())
108102
require.Len(t, col5, 2)
109103
assert.Equal(t, "Europe/Moscow", col5[0].Location().String())
@@ -115,12 +109,7 @@ func TestDateTime(t *testing.T) {
115109
assert.Equal(t, datetime.In(time.UTC), col7)
116110
assert.Equal(t, datetime.Unix(), col8.Unix())
117111
assert.Equal(t, datetime.Unix(), col9.Unix())
118-
if protocol == clickhouse.HTTP {
119-
// TODO: investigate client_protocol_version HTTP param
120-
assert.Equal(t, "UTC", col8.Location().String())
121-
} else {
122-
assert.Equal(t, "Asia/Shanghai", col8.Location().String())
123-
}
112+
assert.Equal(t, "Asia/Shanghai", col8.Location().String())
124113
require.Len(t, col10, 2)
125114
assert.Equal(t, "Asia/Shanghai", col10[0].Location().String())
126115
assert.Equal(t, "Asia/Shanghai", col10[1].Location().String())
@@ -224,33 +213,18 @@ func TestNullableDateTime(t *testing.T) {
224213
assert.Equal(t, datetime.In(time.UTC), col1)
225214
assert.Equal(t, datetime.Unix(), col1.Unix())
226215
require.Nil(t, col2Null)
227-
if protocol == clickhouse.HTTP {
228-
// TODO: investigate client_protocol_version HTTP param
229-
require.Equal(t, "UTC", col2.Location().String())
230-
} else {
231-
require.Equal(t, "Europe/Moscow", col2.Location().String())
232-
}
216+
require.Equal(t, "Europe/Moscow", col2.Location().String())
233217
assert.Equal(t, datetime.Unix(), col2.Unix())
234218
assert.Equal(t, datetime.Unix(), col2.Unix())
235219
require.Nil(t, col3Null)
236-
if protocol == clickhouse.HTTP {
237-
// TODO: investigate client_protocol_version HTTP param
238-
require.Equal(t, "UTC", col3.Location().String())
239-
} else {
240-
require.Equal(t, "Europe/London", col3.Location().String())
241-
}
220+
require.Equal(t, "Europe/London", col3.Location().String())
242221
assert.Equal(t, datetime.Unix(), col3.Unix())
243222
assert.Equal(t, datetime.Unix(), col3.Unix())
244223
require.Nil(t, col4Null)
245224
assert.Equal(t, datetime.In(time.UTC), col4)
246225
assert.Equal(t, datetime.Unix(), col4.Unix())
247226
require.Nil(t, col5Null)
248-
if protocol == clickhouse.HTTP {
249-
// TODO: investigate client_protocol_version HTTP param
250-
require.Equal(t, "UTC", col5.Location().String())
251-
} else {
252-
require.Equal(t, "Asia/Shanghai", col5.Location().String())
253-
}
227+
require.Equal(t, "Asia/Shanghai", col5.Location().String())
254228
assert.Equal(t, datetime.Unix(), col5.Unix())
255229
assert.Equal(t, datetime.Unix(), col5.Unix())
256230
}
@@ -454,12 +428,7 @@ func TestDateTimeTZ(t *testing.T) {
454428
assert.Equal(t, col8Expected.UTC(), col8)
455429
col9Expected, err := time.ParseInLocation("2006-01-02 15:04:05", "2022-07-20 17:42:48", time.Local)
456430
require.NoError(t, err)
457-
if protocol == clickhouse.HTTP {
458-
// TODO: investigate client_protocol_version HTTP param
459-
assert.Equal(t, col9Expected.UTC(), col9)
460-
} else {
461-
assert.Equal(t, col9Expected.In(asiaLoc), col9)
462-
}
431+
assert.Equal(t, col9Expected.In(asiaLoc), col9)
463432
// datetime - with tz
464433
col10Expected, err := time.ParseInLocation("2006-01-02 15:04:05", "2022-07-20 17:42:48", asiaLoc)
465434
require.NoError(t, err)
@@ -469,12 +438,7 @@ func TestDateTimeTZ(t *testing.T) {
469438
assert.Equal(t, col11Expected.UTC(), col11)
470439
col12Expected, err := time.ParseInLocation("2006-01-02 15:04:05", "2022-07-20 17:42:48", asiaLoc)
471440
require.NoError(t, err)
472-
if protocol == clickhouse.HTTP {
473-
// TODO: investigate client_protocol_version HTTP param
474-
assert.Equal(t, col12Expected.UTC(), col12)
475-
} else {
476-
assert.Equal(t, col12Expected.In(asiaLoc), col12)
477-
}
441+
assert.Equal(t, col12Expected.In(asiaLoc), col12)
478442
})
479443
}
480444

‎tests/std/datetime_test.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,8 @@ func TestStdDateTime(t *testing.T) {
9494
assert.Equal(t, datetime.In(time.UTC), col1)
9595
assert.Equal(t, datetime.Unix(), col2.Unix())
9696
assert.Equal(t, datetime.Unix(), col3.Unix())
97-
if protocol == clickhouse.HTTP {
98-
// TODO: investigate client_protocol_version HTTP param
99-
assert.Equal(t, "UTC", col2.Location().String())
100-
assert.Equal(t, "UTC", col3.Location().String())
101-
} else {
102-
assert.Equal(t, "Europe/Moscow", col2.Location().String())
103-
assert.Equal(t, "Europe/London", col3.Location().String())
104-
}
97+
assert.Equal(t, "Europe/Moscow", col2.Location().String())
98+
assert.Equal(t, "Europe/London", col3.Location().String())
10599
assert.Equal(t, datetime.Unix(), col4.Unix())
106100
require.Len(t, col5, 2)
107101
assert.Equal(t, "Europe/Moscow", col5[0].Location().String())

0 commit comments

Comments
 (0)