Skip to content

Commit ad8071f

Browse files
serprexserprex
committed
Fix gaps in clickhouse-c driver
Verify TLS certificates Fix LowCardinality(Nullable(String)) INSERT crash Fix DateTime64 sub-second precision for scale > 6 Reject NaN/Infinity on Decimal INSERT Update clickhouse-c with fixes around overflow detection
1 parent cbe39e5 commit ad8071f

7 files changed

Lines changed: 65 additions & 23 deletions

File tree

‎src/binary/connection.c‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#include <openssl/err.h>
2626
#include <openssl/ssl.h>
27+
#include <openssl/x509v3.h>
2728

2829
#include "binary_internal.h"
2930
#include "engine.h"
@@ -139,7 +140,13 @@ tls_connect(struct ch_binary_state *s, const char *host)
139140
(errcode(ERRCODE_FDW_UNABLE_TO_ESTABLISH_CONNECTION),
140141
errmsg("pg_clickhouse: failed to initialize opensssl"),
141142
errdetail("SSL_CTX_new failed")));
142-
SSL_CTX_set_verify(s->ssl_ctx, SSL_VERIFY_NONE, NULL);
143+
/* Authenticate server: verify chain against system CAs and hostname. */
144+
SSL_CTX_set_verify(s->ssl_ctx, SSL_VERIFY_PEER, NULL);
145+
if (SSL_CTX_set_default_verify_paths(s->ssl_ctx) != 1)
146+
ereport(ERROR,
147+
(errcode(ERRCODE_FDW_UNABLE_TO_ESTABLISH_CONNECTION),
148+
errmsg("pg_clickhouse: failed to initialize openssl"),
149+
errdetail("could not load default CA certificates")));
143150

144151
s->ssl = SSL_new(s->ssl_ctx);
145152
if (!s->ssl)
@@ -148,6 +155,12 @@ tls_connect(struct ch_binary_state *s, const char *host)
148155
errmsg("pg_clickhouse: failed to initialize opensssl"),
149156
errdetail("SSL_new failed")));
150157
SSL_set_tlsext_host_name(s->ssl, host);
158+
SSL_set_hostflags(s->ssl, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
159+
if (SSL_set1_host(s->ssl, host) != 1)
160+
ereport(ERROR,
161+
(errcode(ERRCODE_FDW_UNABLE_TO_ESTABLISH_CONNECTION),
162+
errmsg("pg_clickhouse: failed to initialize openssl"),
163+
errdetail("could not set certificate verification host")));
151164
SSL_set_fd(s->ssl, s->fd);
152165
if (SSL_connect(s->ssl) != 1)
153166
{

‎src/binary/decode.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,8 +718,9 @@ read_value(const chc_column * col, const chc_type * type, uint64_t row,
718718
int64 power = pow10i[scale];
719719

720720
*valtype = TIMESTAMPTZOID;
721+
/* multiply before divide so scale > 6 keeps sub-second us */
721722
return TimestampTzGetDatum(time_t_to_timestamptz(raw / power)
722-
+ (raw % power) * (USECS_PER_SEC / power));
723+
+ (raw % power) * USECS_PER_SEC / power);
723724
}
724725
case CHC_UUID:
725726
*valtype = UUIDOID;

‎src/binary/insert.c‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,13 @@ classify_column(ic_col * ic, const chc_type * t)
203203
errmsg("pg_clickhouse: unsupported LowCardinality variant: %s",
204204
chc_type_name(base, NULL))));
205205
}
206+
207+
/*
208+
* Nullable lives inside LowCardinality, not as an outer wrapper, so
209+
* record it here: resolve_col tracks the per-row null bits
210+
* build_lc_dict reads to map nulls onto dict slot 0.
211+
*/
212+
ic->is_nullable = inner_nullable;
206213
ic->layout = IC_LC_STRING;
207214
ic->elem_t = base;
208215
return;
@@ -545,6 +552,7 @@ append_string_row(ic_col * c, const void *p, size_t n)
545552
static void
546553
decimal_text_to_bytes(const char *s, uint32_t scale, size_t width, uint8_t * out)
547554
{
555+
const char *input = s;
548556
bool neg = false;
549557

550558
if (!s)
@@ -575,6 +583,13 @@ decimal_text_to_bytes(const char *s, uint32_t scale, size_t width, uint8_t * out
575583
char c = i < ilen ? s[i]
576584
: i - ilen < flen ? frac[i - ilen]
577585
: '0';
586+
587+
/* reject NaN / Infinity from numeric_out */
588+
if (c < '0' || c > '9')
589+
ereport(ERROR,
590+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
591+
errmsg("pg_clickhouse: cannot encode \"%s\" as ClickHouse Decimal",
592+
input)));
578593
uint64_t carry = (uint64_t) (c - '0');
579594

580595
for (size_t b = 0; b < nwords; b++)

‎test/expected/binary_inserts.out‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ SELECT clickhouse_raw_query($$
411411
-- TEXTOID
412412
c16 Nullable(String), c17 Nullable(FixedString(1)),
413413
c18 Nullable(Enum('x'=1)), c19 Nullable(Enum16('x'=1)),
414-
-- c20 LowCardinality(Nullable(String)),
414+
c20 LowCardinality(Nullable(String)),
415415
-- DATEOID
416416
c21 Nullable(Date), c22 Nullable(Date32),
417417
-- TIMESTAMPOID, TIMESTAMPTZOID
@@ -433,14 +433,14 @@ IMPORT FOREIGN SCHEMA binary_inserts_test LIMIT TO (null_vals)
433433
FROM SERVER binary_inserts_loopback INTO binary_inserts_test;
434434
INSERT INTO null_vals VALUES(
435435
1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
436-
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, -- NULL,
436+
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
437437
NULL, NULL, NULL, NULL, -- ARRAY[NULL]::int[],
438438
NULL, NULL, NULL
439439
);
440440
SELECT * FROM null_vals;
441-
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10 | c11 | c12 | c13 | c14 | c15 | c16 | c17 | c18 | c19 | c21 | c22 | c23 | c24 | c26 | c27 | c28
442-
----+----+----+----+----+----+----+----+----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----
443-
1 | | | | | | | | | | | | | | | | | | | | | | | | |
441+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10 | c11 | c12 | c13 | c14 | c15 | c16 | c17 | c18 | c19 | c20 | c21 | c22 | c23 | c24 | c26 | c27 | c28
442+
----+----+----+----+----+----+----+----+----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----
443+
1 | | | | | | | | | | | | | | | | | | | | | | | | | |
444444
(1 row)
445445

446446
-- Test default values.
@@ -496,9 +496,15 @@ SELECT * FROM default_vals;
496496
----+----+----+----+----+----+----+----+----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----
497497
(0 rows)
498498

499-
-- Test unsupported Nullables.
500-
INSERT INTO not_nullable VALUES (1, 'x', 'x', NULL);
501-
ERROR: pg_clickhouse: cannot append NULL to NOT NULL LowCardinality(Nullable(String)) column
499+
-- LowCardinality(Nullable(String)) round-trips NULL and non-NULL.
500+
INSERT INTO not_nullable VALUES (1, 'x', 'x', NULL), (2, 'x', 'x', 'lc');
501+
SELECT * FROM not_nullable ORDER BY c1;
502+
c1 | c2 | c3 | c4
503+
----+----+----+----
504+
1 | x | x |
505+
2 | x | x | lc
506+
(2 rows)
507+
502508
DROP USER MAPPING FOR CURRENT_USER SERVER binary_inserts_loopback;
503509
SELECT clickhouse_raw_query('DROP DATABASE binary_inserts_test');
504510
clickhouse_raw_query

‎test/expected/binary_inserts_1.out‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ SELECT clickhouse_raw_query($$
402402
-- TEXTOID
403403
c16 Nullable(String), c17 Nullable(FixedString(1)),
404404
c18 Nullable(Enum('x'=1)), c19 Nullable(Enum16('x'=1)),
405-
-- c20 LowCardinality(Nullable(String)),
405+
c20 LowCardinality(Nullable(String)),
406406
-- DATEOID
407407
c21 Nullable(Date), c22 Nullable(Date32),
408408
-- TIMESTAMPOID, TIMESTAMPTZOID
@@ -424,14 +424,14 @@ IMPORT FOREIGN SCHEMA binary_inserts_test LIMIT TO (null_vals)
424424
FROM SERVER binary_inserts_loopback INTO binary_inserts_test;
425425
INSERT INTO null_vals VALUES(
426426
1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
427-
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, -- NULL,
427+
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
428428
NULL, NULL, NULL, NULL, -- ARRAY[NULL]::int[],
429429
NULL, NULL, NULL
430430
);
431431
SELECT * FROM null_vals;
432-
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10 | c11 | c12 | c13 | c14 | c15 | c16 | c17 | c18 | c19 | c21 | c22 | c23 | c24 | c26 | c27 | c28
433-
----+----+----+----+----+----+----+----+----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----
434-
1 | | | | | | | | | | | | | | | | | | | | | | | | |
432+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10 | c11 | c12 | c13 | c14 | c15 | c16 | c17 | c18 | c19 | c20 | c21 | c22 | c23 | c24 | c26 | c27 | c28
433+
----+----+----+----+----+----+----+----+----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----
434+
1 | | | | | | | | | | | | | | | | | | | | | | | | | |
435435
(1 row)
436436

437437
-- Test default values.
@@ -487,9 +487,15 @@ SELECT * FROM default_vals;
487487
----+----+----+----+----+----+----+----+----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+-----
488488
(0 rows)
489489

490-
-- Test unsupported Nullables.
491-
INSERT INTO not_nullable VALUES (1, 'x', 'x', NULL);
492-
ERROR: pg_clickhouse: cannot append NULL to NOT NULL LowCardinality(Nullable(String)) column
490+
-- LowCardinality(Nullable(String)) round-trips NULL and non-NULL.
491+
INSERT INTO not_nullable VALUES (1, 'x', 'x', NULL), (2, 'x', 'x', 'lc');
492+
SELECT * FROM not_nullable ORDER BY c1;
493+
c1 | c2 | c3 | c4
494+
----+----+----+----
495+
1 | x | x |
496+
2 | x | x | lc
497+
(2 rows)
498+
493499
DROP USER MAPPING FOR CURRENT_USER SERVER binary_inserts_loopback;
494500
SELECT clickhouse_raw_query('DROP DATABASE binary_inserts_test');
495501
clickhouse_raw_query

‎test/sql/binary_inserts.sql‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ SELECT clickhouse_raw_query($$
185185
-- TEXTOID
186186
c16 Nullable(String), c17 Nullable(FixedString(1)),
187187
c18 Nullable(Enum('x'=1)), c19 Nullable(Enum16('x'=1)),
188-
-- c20 LowCardinality(Nullable(String)),
188+
c20 LowCardinality(Nullable(String)),
189189
-- DATEOID
190190
c21 Nullable(Date), c22 Nullable(Date32),
191191
-- TIMESTAMPOID, TIMESTAMPTZOID
@@ -204,7 +204,7 @@ FROM SERVER binary_inserts_loopback INTO binary_inserts_test;
204204

205205
INSERT INTO null_vals VALUES(
206206
1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
207-
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, -- NULL,
207+
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
208208
NULL, NULL, NULL, NULL, -- ARRAY[NULL]::int[],
209209
NULL, NULL, NULL
210210
);
@@ -258,8 +258,9 @@ INSERT INTO default_vals VALUES(
258258

259259
SELECT * FROM default_vals;
260260

261-
-- Test unsupported Nullables.
262-
INSERT INTO not_nullable VALUES (1, 'x', 'x', NULL);
261+
-- LowCardinality(Nullable(String)) round-trips NULL and non-NULL.
262+
INSERT INTO not_nullable VALUES (1, 'x', 'x', NULL), (2, 'x', 'x', 'lc');
263+
SELECT * FROM not_nullable ORDER BY c1;
263264

264265
DROP USER MAPPING FOR CURRENT_USER SERVER binary_inserts_loopback;
265266
SELECT clickhouse_raw_query('DROP DATABASE binary_inserts_test');

‎vendor/clickhouse-c‎

0 commit comments

Comments
 (0)