Skip to content

Commit 69fd4e2

Browse files
author
Ozgun Erdogan
committed
Changed hll_add_trans() functions to return hll_empty instead of NULL when aggregating over empty sets. This makes hll_add_agg() conform to PostgreSQL's count() semantics over empty sets, and closes issue citusdata#2. Also added two small regression tests.
1 parent 17a19e6 commit 69fd4e2

File tree

3 files changed

+104
-115
lines changed

3 files changed

+104
-115
lines changed

‎hll.c

Lines changed: 85 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -2814,10 +2814,26 @@ hll_add_trans4(PG_FUNCTION_ARGS)
28142814
(errcode(ERRCODE_DATA_EXCEPTION),
28152815
errmsg("hll_add_trans4 outside transition context")));
28162816

2817-
// Is the first argument a NULL?
2817+
// If the first argument is a NULL on first call, init an hll_empty
28182818
if (PG_ARGISNULL(0))
28192819
{
2820+
int32 log2m = PG_GETARG_INT32(2);
2821+
int32 regwidth = PG_GETARG_INT32(3);
2822+
int64 expthresh = PG_GETARG_INT64(4);
2823+
int32 sparseon = PG_GETARG_INT32(5);
2824+
28202825
msap = setup_multiset(aggctx);
2826+
2827+
check_modifiers(log2m, regwidth, expthresh, sparseon);
2828+
2829+
memset(msap, '\0', sizeof(multiset_t));
2830+
2831+
msap->ms_type = MST_EMPTY;
2832+
msap->ms_nbits = regwidth;
2833+
msap->ms_nregs = 1 << log2m;
2834+
msap->ms_log2nregs = log2m;
2835+
msap->ms_expthresh = expthresh;
2836+
msap->ms_sparseon = sparseon;
28212837
}
28222838
else
28232839
{
@@ -2829,28 +2845,6 @@ hll_add_trans4(PG_FUNCTION_ARGS)
28292845
{
28302846
int64 val = PG_GETARG_INT64(1);
28312847

2832-
// Was the first argument uninitialized?
2833-
if (msap->ms_type == MST_UNINIT)
2834-
{
2835-
int32 log2m = PG_GETARG_INT32(2);
2836-
int32 regwidth = PG_GETARG_INT32(3);
2837-
int64 expthresh = PG_GETARG_INT64(4);
2838-
int32 sparseon = PG_GETARG_INT32(5);
2839-
2840-
multiset_t ms;
2841-
2842-
check_modifiers(log2m, regwidth, expthresh, sparseon);
2843-
2844-
memset(msap, '\0', sizeof(ms));
2845-
2846-
msap->ms_type = MST_EMPTY;
2847-
msap->ms_nbits = regwidth;
2848-
msap->ms_nregs = 1 << log2m;
2849-
msap->ms_log2nregs = log2m;
2850-
msap->ms_expthresh = expthresh;
2851-
msap->ms_sparseon = sparseon;
2852-
}
2853-
28542848
multiset_add(msap, val);
28552849
}
28562850

@@ -2877,10 +2871,26 @@ hll_add_trans3(PG_FUNCTION_ARGS)
28772871
(errcode(ERRCODE_DATA_EXCEPTION),
28782872
errmsg("hll_add_trans3 outside transition context")));
28792873

2880-
// Is the first argument a NULL?
2874+
// If the first argument is a NULL on first call, init an hll_empty
28812875
if (PG_ARGISNULL(0))
28822876
{
2877+
int32 log2m = PG_GETARG_INT32(2);
2878+
int32 regwidth = PG_GETARG_INT32(3);
2879+
int64 expthresh = PG_GETARG_INT64(4);
2880+
int32 sparseon = g_default_sparseon;
2881+
28832882
msap = setup_multiset(aggctx);
2883+
2884+
check_modifiers(log2m, regwidth, expthresh, sparseon);
2885+
2886+
memset(msap, '\0', sizeof(multiset_t));
2887+
2888+
msap->ms_type = MST_EMPTY;
2889+
msap->ms_nbits = regwidth;
2890+
msap->ms_nregs = 1 << log2m;
2891+
msap->ms_log2nregs = log2m;
2892+
msap->ms_expthresh = expthresh;
2893+
msap->ms_sparseon = sparseon;
28842894
}
28852895
else
28862896
{
@@ -2892,28 +2902,6 @@ hll_add_trans3(PG_FUNCTION_ARGS)
28922902
{
28932903
int64 val = PG_GETARG_INT64(1);
28942904

2895-
// Was the first argument uninitialized?
2896-
if (msap->ms_type == MST_UNINIT)
2897-
{
2898-
int32 log2m = PG_GETARG_INT32(2);
2899-
int32 regwidth = PG_GETARG_INT32(3);
2900-
int64 expthresh = PG_GETARG_INT64(4);
2901-
int32 sparseon = g_default_sparseon;
2902-
2903-
multiset_t ms;
2904-
2905-
check_modifiers(log2m, regwidth, expthresh, sparseon);
2906-
2907-
memset(msap, '\0', sizeof(ms));
2908-
2909-
msap->ms_type = MST_EMPTY;
2910-
msap->ms_nbits = regwidth;
2911-
msap->ms_nregs = 1 << log2m;
2912-
msap->ms_log2nregs = log2m;
2913-
msap->ms_expthresh = expthresh;
2914-
msap->ms_sparseon = sparseon;
2915-
}
2916-
29172905
multiset_add(msap, val);
29182906
}
29192907

@@ -2940,10 +2928,26 @@ hll_add_trans2(PG_FUNCTION_ARGS)
29402928
(errcode(ERRCODE_DATA_EXCEPTION),
29412929
errmsg("hll_add_trans2 outside transition context")));
29422930

2943-
// Is the first argument a NULL?
2931+
// If the first argument is a NULL on first call, init an hll_empty
29442932
if (PG_ARGISNULL(0))
29452933
{
2934+
int32 log2m = PG_GETARG_INT32(2);
2935+
int32 regwidth = PG_GETARG_INT32(3);
2936+
int64 expthresh = g_default_expthresh;
2937+
int32 sparseon = g_default_sparseon;
2938+
29462939
msap = setup_multiset(aggctx);
2940+
2941+
check_modifiers(log2m, regwidth, expthresh, sparseon);
2942+
2943+
memset(msap, '\0', sizeof(multiset_t));
2944+
2945+
msap->ms_type = MST_EMPTY;
2946+
msap->ms_nbits = regwidth;
2947+
msap->ms_nregs = 1 << log2m;
2948+
msap->ms_log2nregs = log2m;
2949+
msap->ms_expthresh = expthresh;
2950+
msap->ms_sparseon = sparseon;
29472951
}
29482952
else
29492953
{
@@ -2955,28 +2959,6 @@ hll_add_trans2(PG_FUNCTION_ARGS)
29552959
{
29562960
int64 val = PG_GETARG_INT64(1);
29572961

2958-
// Was the first argument uninitialized?
2959-
if (msap->ms_type == MST_UNINIT)
2960-
{
2961-
int32 log2m = PG_GETARG_INT32(2);
2962-
int32 regwidth = PG_GETARG_INT32(3);
2963-
int64 expthresh = g_default_expthresh;
2964-
int32 sparseon = g_default_sparseon;
2965-
2966-
multiset_t ms;
2967-
2968-
check_modifiers(log2m, regwidth, expthresh, sparseon);
2969-
2970-
memset(msap, '\0', sizeof(ms));
2971-
2972-
msap->ms_type = MST_EMPTY;
2973-
msap->ms_nbits = regwidth;
2974-
msap->ms_nregs = 1 << log2m;
2975-
msap->ms_log2nregs = log2m;
2976-
msap->ms_expthresh = expthresh;
2977-
msap->ms_sparseon = sparseon;
2978-
}
2979-
29802962
multiset_add(msap, val);
29812963
}
29822964

@@ -3003,10 +2985,26 @@ hll_add_trans1(PG_FUNCTION_ARGS)
30032985
(errcode(ERRCODE_DATA_EXCEPTION),
30042986
errmsg("hll_add_trans1 outside transition context")));
30052987

3006-
// Is the first argument a NULL?
2988+
// If the first argument is a NULL on first call, init an hll_empty
30072989
if (PG_ARGISNULL(0))
30082990
{
2991+
int32 log2m = PG_GETARG_INT32(2);
2992+
int32 regwidth = g_default_regwidth;
2993+
int64 expthresh = g_default_expthresh;
2994+
int32 sparseon = g_default_sparseon;
2995+
30092996
msap = setup_multiset(aggctx);
2997+
2998+
check_modifiers(log2m, regwidth, expthresh, sparseon);
2999+
3000+
memset(msap, '\0', sizeof(multiset_t));
3001+
3002+
msap->ms_type = MST_EMPTY;
3003+
msap->ms_nbits = regwidth;
3004+
msap->ms_nregs = 1 << log2m;
3005+
msap->ms_log2nregs = log2m;
3006+
msap->ms_expthresh = expthresh;
3007+
msap->ms_sparseon = sparseon;
30103008
}
30113009
else
30123010
{
@@ -3018,28 +3016,6 @@ hll_add_trans1(PG_FUNCTION_ARGS)
30183016
{
30193017
int64 val = PG_GETARG_INT64(1);
30203018

3021-
// Was the first argument uninitialized?
3022-
if (msap->ms_type == MST_UNINIT)
3023-
{
3024-
int32 log2m = PG_GETARG_INT32(2);
3025-
int32 regwidth = g_default_regwidth;
3026-
int64 expthresh = g_default_expthresh;
3027-
int32 sparseon = g_default_sparseon;
3028-
3029-
multiset_t ms;
3030-
3031-
check_modifiers(log2m, regwidth, expthresh, sparseon);
3032-
3033-
memset(msap, '\0', sizeof(ms));
3034-
3035-
msap->ms_type = MST_EMPTY;
3036-
msap->ms_nbits = regwidth;
3037-
msap->ms_nregs = 1 << log2m;
3038-
msap->ms_log2nregs = log2m;
3039-
msap->ms_expthresh = expthresh;
3040-
msap->ms_sparseon = sparseon;
3041-
}
3042-
30433019
multiset_add(msap, val);
30443020
}
30453021

@@ -3066,10 +3042,26 @@ hll_add_trans0(PG_FUNCTION_ARGS)
30663042
(errcode(ERRCODE_DATA_EXCEPTION),
30673043
errmsg("hll_add_trans0 outside transition context")));
30683044

3069-
// Is the first argument a NULL?
3045+
// If the first argument is a NULL on first call, init an hll_empty
30703046
if (PG_ARGISNULL(0))
30713047
{
3048+
int32 log2m = g_default_log2m;
3049+
int32 regwidth = g_default_regwidth;
3050+
int64 expthresh = g_default_expthresh;
3051+
int32 sparseon = g_default_sparseon;
3052+
30723053
msap = setup_multiset(aggctx);
3054+
3055+
check_modifiers(log2m, regwidth, expthresh, sparseon);
3056+
3057+
memset(msap, '\0', sizeof(multiset_t));
3058+
3059+
msap->ms_type = MST_EMPTY;
3060+
msap->ms_nbits = regwidth;
3061+
msap->ms_nregs = 1 << log2m;
3062+
msap->ms_log2nregs = log2m;
3063+
msap->ms_expthresh = expthresh;
3064+
msap->ms_sparseon = sparseon;
30733065
}
30743066
else
30753067
{
@@ -3081,28 +3073,6 @@ hll_add_trans0(PG_FUNCTION_ARGS)
30813073
{
30823074
int64 val = PG_GETARG_INT64(1);
30833075

3084-
// Was the first argument uninitialized?
3085-
if (msap->ms_type == MST_UNINIT)
3086-
{
3087-
int32 log2m = g_default_log2m;
3088-
int32 regwidth = g_default_regwidth;
3089-
int64 expthresh = g_default_expthresh;
3090-
int32 sparseon = g_default_sparseon;
3091-
3092-
multiset_t ms;
3093-
3094-
check_modifiers(log2m, regwidth, expthresh, sparseon);
3095-
3096-
memset(msap, '\0', sizeof(ms));
3097-
3098-
msap->ms_type = MST_EMPTY;
3099-
msap->ms_nbits = regwidth;
3100-
msap->ms_nregs = 1 << log2m;
3101-
msap->ms_log2nregs = log2m;
3102-
msap->ms_expthresh = expthresh;
3103-
msap->ms_sparseon = sparseon;
3104-
}
3105-
31063076
multiset_add(msap, val);
31073077
}
31083078

‎regress/add_agg.ref

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,5 +102,18 @@ psql:add_agg.sql:56: ERROR: sparseon modifier must be 0 or 1
102102
select hll_print(hll_add_agg(hll_hash_integer(val), 10, 4, 512, 2))
103103
from test_khvengxf;
104104
psql:add_agg.sql:59: ERROR: sparseon modifier must be 0 or 1
105+
-- Check that we return hll_empty on null input.
106+
select hll_print(hll_add_agg(NULL));
107+
hll_print
108+
-----------------------------------------------------------
109+
EMPTY, nregs=2048, nbits=5, expthresh=-1(160), sparseon=1
110+
(1 row)
111+
112+
select hll_print(hll_add_agg(NULL, 10));
113+
hll_print
114+
----------------------------------------------------------
115+
EMPTY, nregs=1024, nbits=5, expthresh=-1(80), sparseon=1
116+
(1 row)
117+
105118
DROP TABLE test_khvengxf;
106119
DROP TABLE

‎regress/add_agg.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,10 @@ select hll_print(hll_add_agg(hll_hash_integer(val), 10, 4, 512, -1))
5858
select hll_print(hll_add_agg(hll_hash_integer(val), 10, 4, 512, 2))
5959
from test_khvengxf;
6060

61+
-- Check that we return hll_empty on null input.
62+
63+
select hll_print(hll_add_agg(NULL));
64+
65+
select hll_print(hll_add_agg(NULL, 10));
66+
6167
DROP TABLE test_khvengxf;

0 commit comments

Comments
 (0)