Skip to content

Commit fafe980

Browse files
theorytheory
committed
Improve unsigned integer support
Add a regression test that fetches the min and max integer and float values from ClickHouse. Fix a few issues they showed: * Teach the binary engine to use an `int32` instead of a `int16` when converting from UInt16 * Improve the error message raised by the binary engine for `UInt64` values greater than the `BIGINT` max to align with how Postgres itself displays the error The handling of Float32 and Float64 values changed for the better in ClickHouse 23, so add `test/expected/import_schema_2.out` to cover ClickHouse 22. While at it, remove `strtoint32`, originally copied from the PostgreSQL source, and use the `pg_strtoint32` function directly.
1 parent 2bdad66 commit fafe980

File tree

7 files changed

+954
-86
lines changed

7 files changed

+954
-86
lines changed

‎src/binary.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -697,8 +697,11 @@ static Datum make_datum(clickhouse::ColumnRef col, size_t row, Oid * valtype, bo
697697
break;
698698
case Type::Code::UInt64: {
699699
uint64 val = col->As<ColumnUInt64>()->At(row);
700+
/* XXX Consider using, e.g., https://pgxn.org/dist/uint128. */
700701
if (val > LONG_MAX)
701-
throw std::overflow_error("pg_clickhouse: int64 overflow");
702+
throw std::overflow_error(
703+
"value " + std::to_string(val) + " is out of range of bigint"
704+
);
702705

703706
ret = Int64GetDatum((int64)val);
704707
*valtype = INT8OID;

‎src/connection.c‎

Lines changed: 1 addition & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@
3636
*/
3737
static HTAB * ConnectionHash = NULL;
3838
static void chfdw_inval_callback(Datum arg, int cacheid, uint32 hashvalue);
39-
static int32 strtoint32(const char *s);
40-
4139

4240
static ch_connection
4341
clickhouse_connect(ForeignServer * server, UserMapping * user)
@@ -200,7 +198,6 @@ chfdw_inval_callback(Datum arg, int cacheid, uint32 hashvalue)
200198
}
201199
}
202200

203-
204201
ch_connection_details *
205202
connstring_parse(const char *connstring)
206203
{
@@ -223,7 +220,7 @@ connstring_parse(const char *connstring)
223220
}
224221
else if (strcmp(pname, "port") == 0)
225222
{
226-
details->port = strtoint32(pval);
223+
details->port = pg_strtoint32(pval);
227224
}
228225
else if (strcmp(pname, "username") == 0)
229226
{
@@ -247,82 +244,3 @@ connstring_parse(const char *connstring)
247244

248245
return details;
249246
}
250-
251-
/*
252-
* Convert input string to a signed 32 bit integer.
253-
*
254-
* Allows any number of leading or trailing whitespace characters. Will throw
255-
* ereport() upon bad input format or overflow.
256-
*
257-
* NB: Accumulate input as a negative number, to deal with two's complement
258-
* representation of the most negative number, which can't be represented as a
259-
* positive number.
260-
*
261-
* ---
262-
* Copied from postgresql source.
263-
*/
264-
static int32
265-
strtoint32(const char *s)
266-
{
267-
const char *ptr = s;
268-
int32 tmp = 0;
269-
bool neg = false;
270-
271-
/* skip leading spaces */
272-
while (likely(*ptr) && isspace((unsigned char) *ptr))
273-
ptr++;
274-
275-
/* handle sign */
276-
if (*ptr == '-')
277-
{
278-
ptr++;
279-
neg = true;
280-
}
281-
else if (*ptr == '+')
282-
ptr++;
283-
284-
/* require at least one digit */
285-
if (unlikely(!isdigit((unsigned char) *ptr)))
286-
goto invalid_syntax;
287-
288-
/* process digits */
289-
while (*ptr && isdigit((unsigned char) *ptr))
290-
{
291-
int8 digit = (*ptr++ - '0');
292-
293-
if (unlikely(pg_mul_s32_overflow(tmp, 10, &tmp)) ||
294-
unlikely(pg_sub_s32_overflow(tmp, digit, &tmp)))
295-
goto out_of_range;
296-
}
297-
298-
/* allow trailing whitespace, but not other trailing chars */
299-
while (*ptr != '\0' && isspace((unsigned char) *ptr))
300-
ptr++;
301-
302-
if (unlikely(*ptr != '\0'))
303-
goto invalid_syntax;
304-
305-
if (!neg)
306-
{
307-
/* could fail if input is most negative number */
308-
if (unlikely(tmp == PG_INT32_MIN))
309-
goto out_of_range;
310-
tmp = -tmp;
311-
}
312-
313-
return tmp;
314-
315-
out_of_range:
316-
ereport(ERROR,
317-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
318-
errmsg("value \"%s\" is out of range for type %s",
319-
s, "integer")));
320-
321-
invalid_syntax:
322-
ereport(ERROR,
323-
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
324-
errmsg("invalid input syntax for type %s: \"%s\"",
325-
"integer", s)));
326-
327-
return 0; /* keep compiler quiet */
328-
}

‎test/expected/import_schema.out‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,73 @@ EXPLAIN VERBOSE SELECT * FROM clickhouse.custom_option;
656656
Remote SQL: SELECT a FROM import_test.custom_option
657657
(3 rows)
658658

659+
-- check overflows.
660+
SELECT clickhouse_raw_query($$
661+
INSERT INTO import_test.ints (c1, c2, c3, c4, c5, c6, c7, c8, c9, c10) VALUES
662+
(
663+
-- Min values
664+
-128, -32768, -2147483648, -9223372036854775808,
665+
0, 0, 0, 0,
666+
1.175494351e-38, 2.2250738585072014e-308
667+
),
668+
(
669+
-- Max values
670+
127, 32767, 2147483647, 9223372036854775807,
671+
255, 65535, 4294967295, 18446744073709551615,
672+
3.402823466e+38, 1.7976931348623158e+308
673+
)
674+
$$);
675+
clickhouse_raw_query
676+
----------------------
677+
678+
(1 row)
679+
680+
SELECT clickhouse_raw_query($$
681+
SELECT * FROM import_test.ints
682+
WHERE c1 IN (127, -128)
683+
ORDER BY c1;
684+
$$);
685+
clickhouse_raw_query
686+
--------------------------------------------------------------------------------------------------------------------------------------------------------
687+
-128 -32768 -2147483648 -9223372036854775808 0 0 0 0 1.1754942e-38 2.2250738585072014e-308 +
688+
127 32767 2147483647 9223372036854775807 255 65535 4294967295 18446744073709551615 3.4028233e38 1.7976931348623157e308+
689+
690+
(1 row)
691+
692+
-- Error on 18446744073709551615.
693+
SELECT * FROM clickhouse_bin.ints
694+
WHERE c1 IN (127, -128)
695+
ORDER BY c1;
696+
ERROR: pg_clickhouse: error while reading row: value 18446744073709551615 is out of range of bigint
697+
SELECT * FROM clickhouse.ints
698+
WHERE c1 IN (127, -128)
699+
ORDER BY c1;
700+
ERROR: value "18446744073709551615" is out of range for type bigint
701+
-- Ignore 18446744073709551615
702+
SELECT * FROM clickhouse_bin.ints WHERE c1 = -128
703+
UNION
704+
SELECT c1, c2, c3, c4, c5, c6, c7, NULL, c9, c10
705+
FROM clickhouse_bin.ints
706+
WHERE c1 = 127
707+
ORDER BY c1;
708+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10
709+
------+--------+-------------+----------------------+-----+----+------------+----+---------------+-------------------------
710+
-128 | -32768 | -2147483648 | -9223372036854775808 | 0 | 0 | 0 | 0 | 1.1754942e-38 | 2.2250738585072014e-308
711+
127 | 32767 | 2147483647 | 9223372036854775807 | 255 | -1 | 4294967295 | | 3.4028233e+38 | 1.7976931348623157e+308
712+
(2 rows)
713+
714+
SELECT * FROM clickhouse.ints WHERE c1 = -128
715+
UNION
716+
SELECT c1, c2, c3, c4, c5, c6, c7, NULL, c9, c10
717+
FROM clickhouse.ints
718+
WHERE c1 = 127
719+
ORDER BY c1;
720+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10
721+
------+--------+-------------+----------------------+-----+-------+------------+----+---------------+-------------------------
722+
-128 | -32768 | -2147483648 | -9223372036854775808 | 0 | 0 | 0 | 0 | 1.1754942e-38 | 2.2250738585072014e-308
723+
127 | 32767 | 2147483647 | 9223372036854775807 | 255 | 65535 | 4294967295 | | 3.4028233e+38 | 1.7976931348623157e+308
724+
(2 rows)
725+
659726
DROP USER MAPPING FOR CURRENT_USER SERVER import_loopback;
660727
DROP USER MAPPING FOR CURRENT_USER SERVER import_loopback_bin;
661728
SELECT clickhouse_raw_query('DROP DATABASE import_test');

‎test/expected/import_schema_1.out‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,73 @@ EXPLAIN VERBOSE SELECT * FROM clickhouse.custom_option;
556556
Remote SQL: SELECT a FROM import_test.custom_option
557557
(3 rows)
558558

559+
-- check overflows.
560+
SELECT clickhouse_raw_query($$
561+
INSERT INTO import_test.ints (c1, c2, c3, c4, c5, c6, c7, c8, c9, c10) VALUES
562+
(
563+
-- Min values
564+
-128, -32768, -2147483648, -9223372036854775808,
565+
0, 0, 0, 0,
566+
1.175494351e-38, 2.2250738585072014e-308
567+
),
568+
(
569+
-- Max values
570+
127, 32767, 2147483647, 9223372036854775807,
571+
255, 65535, 4294967295, 18446744073709551615,
572+
3.402823466e+38, 1.7976931348623158e+308
573+
)
574+
$$);
575+
clickhouse_raw_query
576+
----------------------
577+
578+
(1 row)
579+
580+
SELECT clickhouse_raw_query($$
581+
SELECT * FROM import_test.ints
582+
WHERE c1 IN (127, -128)
583+
ORDER BY c1;
584+
$$);
585+
clickhouse_raw_query
586+
--------------------------------------------------------------------------------------------------------------------------------------------------------
587+
-128 -32768 -2147483648 -9223372036854775808 0 0 0 0 1.1754942e-38 2.2250738585072014e-308 +
588+
127 32767 2147483647 9223372036854775807 255 65535 4294967295 18446744073709551615 3.4028233e38 1.7976931348623157e308+
589+
590+
(1 row)
591+
592+
-- Error on 18446744073709551615.
593+
SELECT * FROM clickhouse_bin.ints
594+
WHERE c1 IN (127, -128)
595+
ORDER BY c1;
596+
ERROR: pg_clickhouse: error while reading row: value 18446744073709551615 is out of range of bigint
597+
SELECT * FROM clickhouse.ints
598+
WHERE c1 IN (127, -128)
599+
ORDER BY c1;
600+
ERROR: value "18446744073709551615" is out of range for type bigint
601+
-- Ignore 18446744073709551615
602+
SELECT * FROM clickhouse_bin.ints WHERE c1 = -128
603+
UNION
604+
SELECT c1, c2, c3, c4, c5, c6, c7, NULL, c9, c10
605+
FROM clickhouse_bin.ints
606+
WHERE c1 = 127
607+
ORDER BY c1;
608+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10
609+
------+--------+-------------+----------------------+-----+----+------------+----+---------------+-------------------------
610+
-128 | -32768 | -2147483648 | -9223372036854775808 | 0 | 0 | 0 | 0 | 1.1754942e-38 | 2.2250738585072014e-308
611+
127 | 32767 | 2147483647 | 9223372036854775807 | 255 | -1 | 4294967295 | | 3.4028233e+38 | 1.7976931348623157e+308
612+
(2 rows)
613+
614+
SELECT * FROM clickhouse.ints WHERE c1 = -128
615+
UNION
616+
SELECT c1, c2, c3, c4, c5, c6, c7, NULL, c9, c10
617+
FROM clickhouse.ints
618+
WHERE c1 = 127
619+
ORDER BY c1;
620+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10
621+
------+--------+-------------+----------------------+-----+-------+------------+----+---------------+-------------------------
622+
-128 | -32768 | -2147483648 | -9223372036854775808 | 0 | 0 | 0 | 0 | 1.1754942e-38 | 2.2250738585072014e-308
623+
127 | 32767 | 2147483647 | 9223372036854775807 | 255 | 65535 | 4294967295 | | 3.4028233e+38 | 1.7976931348623157e+308
624+
(2 rows)
625+
559626
DROP USER MAPPING FOR CURRENT_USER SERVER import_loopback;
560627
DROP USER MAPPING FOR CURRENT_USER SERVER import_loopback_bin;
561628
SELECT clickhouse_raw_query('DROP DATABASE import_test');

0 commit comments

Comments
 (0)