Skip to content

Commit bc4723a

Browse files
serprexserprex
authored
Merge pull request #107 from citusdata/fix/thresholds
Fix out of range (rebased)
2 parents 7c39fbd + cc97135 commit bc4723a

File tree

6 files changed

+91
-67
lines changed

6 files changed

+91
-67
lines changed

‎expected/add_agg.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ select hll_print(hll_add_agg(hll_hash_integer(val), 10, 4, 512, 0))
7777
-- Check range checking.
7878
select hll_print(hll_add_agg(hll_hash_integer(val), -1))
7979
from test_khvengxf;
80-
ERROR: log2m modifier must be between 0 and 31
80+
ERROR: log2m modifier must be between 0 and 17
8181
select hll_print(hll_add_agg(hll_hash_integer(val), 32))
8282
from test_khvengxf;
83-
ERROR: log2m modifier must be between 0 and 31
83+
ERROR: log2m modifier must be between 0 and 17
8484
select hll_print(hll_add_agg(hll_hash_integer(val), 10, -1))
8585
from test_khvengxf;
8686
ERROR: regwidth modifier must be between 0 and 7
@@ -89,10 +89,10 @@ select hll_print(hll_add_agg(hll_hash_integer(val), 10, 8))
8989
ERROR: regwidth modifier must be between 0 and 7
9090
select hll_print(hll_add_agg(hll_hash_integer(val), 10, 4, -2))
9191
from test_khvengxf;
92-
ERROR: expthresh modifier must be between -1 and 2^32
92+
ERROR: expthresh modifier must be between -1 and 16383
9393
select hll_print(hll_add_agg(hll_hash_integer(val), 10, 4, 8589934592))
9494
from test_khvengxf;
95-
ERROR: expthresh modifier must be between -1 and 2^32
95+
ERROR: expthresh modifier must be between -1 and 16383
9696
select hll_print(hll_add_agg(hll_hash_integer(val), 10, 4, 512, -1))
9797
from test_khvengxf;
9898
ERROR: sparseon modifier must be 0 or 1

‎expected/typmod.out

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ LINE 1: CREATE TABLE test_qiundgkm (v1 hll(10, 4, 64, 0, 42));
5959
-- ----------------------------------------------------------------
6060
-- Range Check log2nregs
6161
-- ----------------------------------------------------------------
62-
-- ERROR: log2m modifier must be between 0 and 31
62+
-- ERROR: log2m modifier must be between 0 and 17
6363
CREATE TABLE test_qiundgkm (v1 hll(-1));
64-
ERROR: log2m modifier must be between 0 and 31
64+
ERROR: log2m modifier must be between 0 and 17
6565
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(-1));
6666
^
6767
CREATE TABLE test_qiundgkm (v1 hll(0));
@@ -73,16 +73,15 @@ CREATE TABLE test_qiundgkm (v1 hll(0));
7373

7474
DROP TABLE test_qiundgkm;
7575
CREATE TABLE test_qiundgkm (v1 hll(31));
76+
ERROR: log2m modifier must be between 0 and 17
77+
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(31));
78+
^
7679
\d test_qiundgkm
77-
Table "public.test_qiundgkm"
78-
Column | Type | Collation | Nullable | Default
79-
--------+----------------+-----------+----------+---------
80-
v1 | hll(31,5,-1,1) | | |
81-
8280
DROP TABLE test_qiundgkm;
83-
-- ERROR: log2m modifier must be between 0 and 31
81+
ERROR: table "test_qiundgkm" does not exist
82+
-- ERROR: log2m modifier must be between 0 and 17
8483
CREATE TABLE test_qiundgkm (v1 hll(32));
85-
ERROR: log2m modifier must be between 0 and 31
84+
ERROR: log2m modifier must be between 0 and 17
8685
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(32));
8786
^
8887
-- ----------------------------------------------------------------
@@ -117,9 +116,9 @@ LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 8));
117116
-- ----------------------------------------------------------------
118117
-- Range Check expthresh
119118
-- ----------------------------------------------------------------
120-
-- ERROR: expthresh modifier must be between -1 and 2^32
119+
-- ERROR: expthresh modifier must be between -1 and 16383
121120
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -2));
122-
ERROR: expthresh modifier must be between -1 and 2^32
121+
ERROR: expthresh modifier must be between -1 and 16383
123122
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, -2));
124123
^
125124
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -1));
@@ -147,16 +146,15 @@ CREATE TABLE test_qiundgkm (v1 hll(11, 5, 128));
147146

148147
DROP TABLE test_qiundgkm;
149148
CREATE TABLE test_qiundgkm (v1 hll(11, 5, 4294967296));
149+
ERROR: expthresh modifier must be between -1 and 16383
150+
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, 4294967296));
151+
^
150152
\d test_qiundgkm
151-
Table "public.test_qiundgkm"
152-
Column | Type | Collation | Nullable | Default
153-
--------+------------------------+-----------+----------+---------
154-
v1 | hll(11,5,4294967296,1) | | |
155-
156153
DROP TABLE test_qiundgkm;
157-
-- ERROR: expthresh modifier must be between -1 and 2^32
154+
ERROR: table "test_qiundgkm" does not exist
155+
-- ERROR: expthresh modifier must be between -1 and 16383
158156
CREATE TABLE test_qiundgkm (v1 hll(11, 5, 8589934592));
159-
ERROR: expthresh modifier must be between -1 and 2^32
157+
ERROR: expthresh modifier must be between -1 and 16383
160158
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, 8589934592));
161159
^
162160
-- ----------------------------------------------------------------

‎expected/typmod_0.out

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ LINE 1: CREATE TABLE test_qiundgkm (v1 hll(10, 4, 64, 0, 42));
5959
-- ----------------------------------------------------------------
6060
-- Range Check log2nregs
6161
-- ----------------------------------------------------------------
62-
-- ERROR: log2m modifier must be between 0 and 31
62+
-- ERROR: log2m modifier must be between 0 and 17
6363
CREATE TABLE test_qiundgkm (v1 hll(-1));
64-
ERROR: log2m modifier must be between 0 and 31
64+
ERROR: log2m modifier must be between 0 and 17
6565
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(-1));
6666
^
6767
CREATE TABLE test_qiundgkm (v1 hll(0));
@@ -73,16 +73,15 @@ CREATE TABLE test_qiundgkm (v1 hll(0));
7373

7474
DROP TABLE test_qiundgkm;
7575
CREATE TABLE test_qiundgkm (v1 hll(31));
76+
ERROR: log2m modifier must be between 0 and 17
77+
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(31));
78+
^
7679
\d test_qiundgkm
77-
Table "public.test_qiundgkm"
78-
Column | Type | Collation | Nullable | Default
79-
--------+----------------+-----------+----------+---------
80-
v1 | hll(31,5,-1,1) | | |
81-
8280
DROP TABLE test_qiundgkm;
83-
-- ERROR: log2m modifier must be between 0 and 31
81+
ERROR: table "test_qiundgkm" does not exist
82+
-- ERROR: log2m modifier must be between 0 and 17
8483
CREATE TABLE test_qiundgkm (v1 hll(32));
85-
ERROR: log2m modifier must be between 0 and 31
84+
ERROR: log2m modifier must be between 0 and 17
8685
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(32));
8786
^
8887
-- ----------------------------------------------------------------
@@ -117,9 +116,9 @@ LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 8));
117116
-- ----------------------------------------------------------------
118117
-- Range Check expthresh
119118
-- ----------------------------------------------------------------
120-
-- ERROR: expthresh modifier must be between -1 and 2^32
119+
-- ERROR: expthresh modifier must be between -1 and 16383
121120
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -2));
122-
ERROR: expthresh modifier must be between -1 and 2^32
121+
ERROR: expthresh modifier must be between -1 and 16383
123122
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, -2));
124123
^
125124
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -1));
@@ -147,16 +146,15 @@ CREATE TABLE test_qiundgkm (v1 hll(11, 5, 128));
147146

148147
DROP TABLE test_qiundgkm;
149148
CREATE TABLE test_qiundgkm (v1 hll(11, 5, 4294967296));
149+
ERROR: expthresh modifier must be between -1 and 16383
150+
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, 4294967296));
151+
^
150152
\d test_qiundgkm
151-
Table "public.test_qiundgkm"
152-
Column | Type | Collation | Nullable | Default
153-
--------+------------------------+-----------+----------+---------
154-
v1 | hll(11,5,4294967296,1) | | |
155-
156153
DROP TABLE test_qiundgkm;
157-
-- ERROR: expthresh modifier must be between -1 and 2^32
154+
ERROR: table "test_qiundgkm" does not exist
155+
-- ERROR: expthresh modifier must be between -1 and 16383
158156
CREATE TABLE test_qiundgkm (v1 hll(11, 5, 8589934592));
159-
ERROR: expthresh modifier must be between -1 and 2^32
157+
ERROR: expthresh modifier must be between -1 and 16383
160158
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, 8589934592));
161159
^
162160
-- ----------------------------------------------------------------

‎expected/typmod_1.out

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ LINE 1: CREATE TABLE test_qiundgkm (v1 hll(10, 4, 64, 0, 42));
5959
-- ----------------------------------------------------------------
6060
-- Range Check log2nregs
6161
-- ----------------------------------------------------------------
62-
-- ERROR: log2m modifier must be between 0 and 31
62+
-- ERROR: log2m modifier must be between 0 and 17
6363
CREATE TABLE test_qiundgkm (v1 hll(-1));
64-
ERROR: log2m modifier must be between 0 and 31
64+
ERROR: log2m modifier must be between 0 and 17
6565
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(-1));
6666
^
6767
CREATE TABLE test_qiundgkm (v1 hll(0));
@@ -73,16 +73,15 @@ CREATE TABLE test_qiundgkm (v1 hll(0));
7373

7474
DROP TABLE test_qiundgkm;
7575
CREATE TABLE test_qiundgkm (v1 hll(31));
76+
ERROR: log2m modifier must be between 0 and 17
77+
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(31));
78+
^
7679
\d test_qiundgkm
77-
Table "public.test_qiundgkm"
78-
Column | Type | Modifiers
79-
--------+----------------+-----------
80-
v1 | hll(31,5,-1,1) |
81-
8280
DROP TABLE test_qiundgkm;
83-
-- ERROR: log2m modifier must be between 0 and 31
81+
ERROR: table "test_qiundgkm" does not exist
82+
-- ERROR: log2m modifier must be between 0 and 17
8483
CREATE TABLE test_qiundgkm (v1 hll(32));
85-
ERROR: log2m modifier must be between 0 and 31
84+
ERROR: log2m modifier must be between 0 and 17
8685
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(32));
8786
^
8887
-- ----------------------------------------------------------------
@@ -117,9 +116,9 @@ LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 8));
117116
-- ----------------------------------------------------------------
118117
-- Range Check expthresh
119118
-- ----------------------------------------------------------------
120-
-- ERROR: expthresh modifier must be between -1 and 2^32
119+
-- ERROR: expthresh modifier must be between -1 and 16383
121120
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -2));
122-
ERROR: expthresh modifier must be between -1 and 2^32
121+
ERROR: expthresh modifier must be between -1 and 16383
123122
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, -2));
124123
^
125124
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -1));
@@ -147,16 +146,15 @@ CREATE TABLE test_qiundgkm (v1 hll(11, 5, 128));
147146

148147
DROP TABLE test_qiundgkm;
149148
CREATE TABLE test_qiundgkm (v1 hll(11, 5, 4294967296));
149+
ERROR: expthresh modifier must be between -1 and 16383
150+
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, 4294967296));
151+
^
150152
\d test_qiundgkm
151-
Table "public.test_qiundgkm"
152-
Column | Type | Modifiers
153-
--------+------------------------+-----------
154-
v1 | hll(11,5,4294967296,1) |
155-
156153
DROP TABLE test_qiundgkm;
157-
-- ERROR: expthresh modifier must be between -1 and 2^32
154+
ERROR: table "test_qiundgkm" does not exist
155+
-- ERROR: expthresh modifier must be between -1 and 16383
158156
CREATE TABLE test_qiundgkm (v1 hll(11, 5, 8589934592));
159-
ERROR: expthresh modifier must be between -1 and 2^32
157+
ERROR: expthresh modifier must be between -1 and 16383
160158
LINE 1: CREATE TABLE test_qiundgkm (v1 hll(11, 5, 8589934592));
161159
^
162160
-- ----------------------------------------------------------------

‎sql/typmod.sql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ CREATE TABLE test_qiundgkm (v1 hll(10, 4, 64, 0, 42));
3535
-- Range Check log2nregs
3636
-- ----------------------------------------------------------------
3737

38-
-- ERROR: log2m modifier must be between 0 and 31
38+
-- ERROR: log2m modifier must be between 0 and 17
3939
CREATE TABLE test_qiundgkm (v1 hll(-1));
4040

4141
CREATE TABLE test_qiundgkm (v1 hll(0));
@@ -46,7 +46,7 @@ CREATE TABLE test_qiundgkm (v1 hll(31));
4646
\d test_qiundgkm
4747
DROP TABLE test_qiundgkm;
4848

49-
-- ERROR: log2m modifier must be between 0 and 31
49+
-- ERROR: log2m modifier must be between 0 and 17
5050
CREATE TABLE test_qiundgkm (v1 hll(32));
5151

5252
-- ----------------------------------------------------------------
@@ -71,7 +71,7 @@ CREATE TABLE test_qiundgkm (v1 hll(11, 8));
7171
-- Range Check expthresh
7272
-- ----------------------------------------------------------------
7373

74-
-- ERROR: expthresh modifier must be between -1 and 2^32
74+
-- ERROR: expthresh modifier must be between -1 and 16383
7575
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -2));
7676

7777
CREATE TABLE test_qiundgkm (v1 hll(11, 5, -1));
@@ -90,7 +90,7 @@ CREATE TABLE test_qiundgkm (v1 hll(11, 5, 4294967296));
9090
\d test_qiundgkm
9191
DROP TABLE test_qiundgkm;
9292

93-
-- ERROR: expthresh modifier must be between -1 and 2^32
93+
-- ERROR: expthresh modifier must be between -1 and 16383
9494
CREATE TABLE test_qiundgkm (v1 hll(11, 5, 8589934592));
9595

9696
-- ----------------------------------------------------------------

‎src/hll.c

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,30 @@ typedef struct
586586

587587
} bitstream_read_cursor_t;
588588

589+
590+
/*
591+
* The result of mse_nelem_max()/msc_regs_idx_limit() is a const in a specific platform.
592+
* But we couldn't hard code it explicitly because we do not know current alignment schema
593+
* when compiling it.
594+
*/
595+
static size_t
596+
mse_nelem_max(void)
597+
{
598+
multiset_t *msp = NULL; /* Safely! */
599+
uint8_t *ms_data_start = (uint8_t*)(&msp->ms_data);
600+
uint8_t *ms_data_end = ms_data_start + sizeof(msp->ms_data);
601+
uint8_t *mse_elems_start = (uint8_t*)(&msp->ms_data.as_expl.mse_elems);
602+
return ((size_t)(ms_data_end - mse_elems_start)) / sizeof(uint64_t);
603+
}
604+
605+
static size_t
606+
msc_regs_idx_limit(void)
607+
{
608+
multiset_t * o_msp = NULL;
609+
uint8_t * const ms_data_limit = ((uint8_t*)&o_msp->ms_data) + sizeof(o_msp->ms_data);
610+
return (ms_data_limit - (uint8_t*)&o_msp->ms_data.as_comp.msc_regs[0]) / sizeof(compreg_t);
611+
}
612+
589613
static uint32_t
590614
bitstream_unpack(bitstream_read_cursor_t * brcp)
591615
{
@@ -935,6 +959,8 @@ compressed_add(multiset_t * o_msp, uint64_t elem)
935959

936960
size_t p_w = ss_val == 0 ? 0 : __builtin_ctzll(ss_val) + 1;
937961

962+
Assert(ndx < msc_regs_idx_limit());
963+
938964
if (p_w > maxregval)
939965
p_w = maxregval;
940966

@@ -1170,6 +1196,7 @@ multiset_add(multiset_t * o_msp, uint64_t element)
11701196
size_t expval = expthresh_value(o_msp->ms_expthresh,
11711197
o_msp->ms_nbits,
11721198
o_msp->ms_nregs);
1199+
Assert(expval <= mse_nelem_max());
11731200

11741201
switch (o_msp->ms_type)
11751202
{
@@ -1389,7 +1416,7 @@ multiset_unpack(multiset_t * o_msp,
13891416
}
13901417

13911418
// Make sure the explicit array fits in memory.
1392-
if ((i_size - hdrsz) > MS_MAXDATA)
1419+
if (nelem > mse_nelem_max())
13931420
{
13941421
ereport(ERROR,
13951422
(errcode(ERRCODE_DATA_EXCEPTION),
@@ -1993,21 +2020,24 @@ hll_hashval_int4(PG_FUNCTION_ARGS)
19932020
static void
19942021
check_modifiers(int32 log2m, int32 regwidth, int64 expthresh, int32 sparseon)
19952022
{
2023+
int64 expthresh_max = mse_nelem_max();
2024+
int32 log2m_max = integer_log2(msc_regs_idx_limit()); /* const expression. */
2025+
19962026
// Range check each of the modifiers.
1997-
if (log2m < 0 || log2m > MAX_BITVAL(LOG2M_BITS))
2027+
if (log2m < 0 || log2m > log2m_max)
19982028
ereport(ERROR,
19992029
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2000-
errmsg("log2m modifier must be between 0 and 31")));
2030+
errmsg("log2m modifier must be between 0 and %d", log2m_max)));
20012031

20022032
if (regwidth < 0 || regwidth > MAX_BITVAL(REGWIDTH_BITS))
20032033
ereport(ERROR,
20042034
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
20052035
errmsg("regwidth modifier must be between 0 and 7")));
20062036

2007-
if (expthresh < -1 || expthresh > 4294967296LL)
2037+
if (expthresh < -1 || expthresh > expthresh_max)
20082038
ereport(ERROR,
20092039
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2010-
errmsg("expthresh modifier must be between -1 and 2^32")));
2040+
errmsg("expthresh modifier must be between -1 and %ld", expthresh_max)));
20112041

20122042
if (expthresh > 0 && (1LL << integer_log2(expthresh)) != expthresh)
20132043
ereport(ERROR,

0 commit comments

Comments
 (0)