Changeset: 19e87afa319a for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=19e87afa319a
Modified Files:
gdk/gdk_string.c
sql/test/analytics/Tests/analytics17.stable.out
sql/test/group-concat/Tests/groupconcat05.sql
sql/test/group-concat/Tests/groupconcat05.stable.out
Branch: Jun2020
Log Message:
Small bugfix, group_concat global aggregate and window function must return
null on an empty input according to the SQL standard.
Back then, I looked at the psql output to confirm, but the NULL and empty
strings have the same output there, so I got confused :)
diffs (truncated from 386 to 300 lines):
diff --git a/gdk/gdk_string.c b/gdk/gdk_string.c
--- a/gdk/gdk_string.c
+++ b/gdk/gdk_string.c
@@ -866,8 +866,8 @@ concat_strings(BAT **bnp, ValPtr pt, BAT
}
canditer_reset(ci);
- if (nils == 0) {
- char *single_str;
+ if (nils == 0 && !empty) {
+ char *single_str = NULL;
if ((single_str = GDKmalloc(single_length + 1)) == NULL)
return GDK_FAIL;
@@ -1185,6 +1185,7 @@ GDKanalytical_str_group_concat(BAT *r, B
str sb, sl, single_str = NULL, next_single_str;
bool empty;
size_t separator_length = 0, next_group_length, max_group_length = 0,
next_length, offset;
+ bool hasnil = 0;
assert(s && e && ((sep && !separator && BATcount(b) == BATcount(sep))
|| (!sep && separator)));
start = (lng *) Tloc(s, 0);
@@ -1235,61 +1236,65 @@ GDKanalytical_str_group_concat(BAT *r, B
}
}
- empty = true;
-
- if (!single_str) { /* reuse the same buffer, resize it when
needed */
- max_group_length = next_group_length;
- if ((single_str = GDKmalloc(max_group_length + 1)) ==
NULL)
- goto allocation_error;
- } else if (next_group_length > max_group_length) {
- max_group_length = next_group_length;
- if ((next_single_str = GDKrealloc(single_str,
max_group_length + 1)) == NULL)
+ if (empty) {
+ if ((single_str = GDKstrdup(str_nil)) == NULL)
goto allocation_error;
- single_str = next_single_str;
- }
+ hasnil = true;
+ } else {
+ empty = true;
+ if (!single_str) { /* reuse the same buffer, resize it
when needed */
+ max_group_length = next_group_length;
+ if ((single_str = GDKmalloc(max_group_length +
1)) == NULL)
+ goto allocation_error;
+ } else if (next_group_length > max_group_length) {
+ max_group_length = next_group_length;
+ if ((next_single_str = GDKrealloc(single_str,
max_group_length + 1)) == NULL)
+ goto allocation_error;
+ single_str = next_single_str;
+ }
- for (j = start[i]; j < l; j++) {
- sb = BUNtvar(bi, (BUN) j);
+ for (j = start[i]; j < l; j++) {
+ sb = BUNtvar(bi, (BUN) j);
- if (separator) {
- if (strNil(sb))
- continue;
- if (!empty) {
- memcpy(single_str + offset, separator,
separator_length);
- offset += separator_length;
+ if (separator) {
+ if (strNil(sb))
+ continue;
+ if (!empty) {
+ memcpy(single_str + offset,
separator, separator_length);
+ offset += separator_length;
+ }
+ next_length = strlen(sb);
+ memcpy(single_str + offset, sb,
next_length);
+ offset += next_length;
+ empty = false;
+ } else { /* sep case */
+ assert(sep != NULL);
+ sl = BUNtvar(bis, (BUN) j);
+
+ if (strNil(sb))
+ continue;
+ if (!empty && !strNil(sl)) {
+ next_length = strlen(sl);
+ memcpy(single_str + offset, sl,
next_length);
+ offset += next_length;
+ }
+ next_length = strlen(sb);
+ memcpy(single_str + offset, sb,
next_length);
+ offset += next_length;
+ empty = false;
}
- next_length = strlen(sb);
- memcpy(single_str + offset, sb, next_length);
- offset += next_length;
- empty = false;
- } else { /* sep case */
- assert(sep != NULL);
- sl = BUNtvar(bis, (BUN) j);
+ }
- if (strNil(sb))
- continue;
- if (!empty && !strNil(sl)) {
- next_length = strlen(sl);
- memcpy(single_str + offset, sl,
next_length);
- offset += next_length;
- }
- next_length = strlen(sb);
- memcpy(single_str + offset, sb, next_length);
- offset += next_length;
- empty = false;
- }
+ single_str[offset] = '\0';
}
-
- single_str[offset] = '\0';
if (BUNappend(r, single_str, false) != GDK_SUCCEED)
goto allocation_error;
-
}
GDKfree(single_str);
BATsetcount(r, cnt);
- r->tnonil = true;
- r->tnil = false;
+ r->tnonil = !hasnil;
+ r->tnil = hasnil;
return GDK_SUCCEED;
allocation_error:
GDKfree(single_str);
diff --git a/sql/test/analytics/Tests/analytics17.stable.out
b/sql/test/analytics/Tests/analytics17.stable.out
--- a/sql/test/analytics/Tests/analytics17.stable.out
+++ b/sql/test/analytics/Tests/analytics17.stable.out
@@ -72,11 +72,11 @@ stdout of test 'analytics17` in director
# group_concat(aa, aa) over (partition by bb order by bb asc),
# group_concat(aa, aa) over (partition by bb order by bb desc),
# group_concat(aa, aa) over (order by bb desc) from analytics;
-% sys.%3, sys.%6, sys.%11, sys.%14 # table_name
-% %3, %6, %11, %14 # name
+% sys.%7, sys.%16, sys.%25, sys.%34 # table_name
+% %7, %16, %25, %34 # name
% clob, clob, clob, clob # type
% 6, 6, 6, 16 # length
-[ "", "", "", "" ]
+[ NULL, NULL, NULL, NULL ]
[ "155566", "155566", "155566", "155566" ]
[ "155566", "155566", "155566", "155566" ]
[ "155566", "155566", "155566", "155566" ]
@@ -90,8 +90,8 @@ stdout of test 'analytics17` in director
# group_concat(bb, bb) over (partition by bb order by bb asc),
# group_concat(bb, bb) over (partition by bb order by bb desc),
# group_concat(bb, bb) over (order by bb desc) from analytics;
-% sys.%3, sys.%6, sys.%11, sys.%14 # table_name
-% %3, %6, %11, %14 # name
+% sys.%7, sys.%16, sys.%25, sys.%34 # table_name
+% %7, %16, %25, %34 # name
% clob, clob, clob, clob # type
% 5, 5, 5, 19 # length
[ "4", "4", "4", "4" ]
@@ -108,11 +108,11 @@ stdout of test 'analytics17` in director
# group_concat(aa, bb) over (partition by bb order by bb asc),
# group_concat(aa, bb) over (partition by bb order by bb desc),
# group_concat(aa, bb) over (order by bb desc) from analytics;
-% sys.%3, sys.%6, sys.%11, sys.%14 # table_name
-% %3, %6, %11, %14 # name
+% sys.%7, sys.%16, sys.%25, sys.%34 # table_name
+% %7, %16, %25, %34 # name
% clob, clob, clob, clob # type
% 6, 6, 6, 16 # length
-[ "", "", "", "" ]
+[ NULL, NULL, NULL, NULL ]
[ "153536", "153536", "153536", "153536" ]
[ "153536", "153536", "153536", "153536" ]
[ "153536", "153536", "153536", "153536" ]
@@ -126,8 +126,8 @@ stdout of test 'analytics17` in director
# group_concat(bb, aa) over (partition by bb order by bb asc),
# group_concat(bb, aa) over (partition by bb order by bb desc),
# group_concat(bb, aa) over (order by bb desc) from analytics;
-% sys.%3, sys.%6, sys.%11, sys.%14 # table_name
-% %3, %6, %11, %14 # name
+% sys.%7, sys.%16, sys.%25, sys.%34 # table_name
+% %7, %16, %25, %34 # name
% clob, clob, clob, clob # type
% 5, 5, 5, 19 # length
[ "4", "4", "4", "4" ]
@@ -144,11 +144,11 @@ stdout of test 'analytics17` in director
# group_concat(aa, 1) over (partition by bb order by bb asc),
# group_concat(aa, 1) over (partition by bb order by bb desc),
# group_concat(aa, 1) over (order by bb desc) from analytics;
-% sys.%3, sys.%6, sys.%11, sys.%14 # table_name
-% %3, %6, %11, %14 # name
+% sys.%10, sys.%20, sys.%30, sys.%40 # table_name
+% %10, %20, %30, %40 # name
% clob, clob, clob, clob # type
% 6, 6, 6, 16 # length
-[ "", "", "", "" ]
+[ NULL, NULL, NULL, NULL ]
[ "151516", "151516", "151516", "151516" ]
[ "151516", "151516", "151516", "151516" ]
[ "151516", "151516", "151516", "151516" ]
@@ -162,8 +162,8 @@ stdout of test 'analytics17` in director
# group_concat(bb, -100) over (partition by bb order by bb asc),
# group_concat(bb, -100) over (partition by bb order by bb desc),
# group_concat(bb, -100) over (order by bb desc) from analytics;
-% sys.%3, sys.%6, sys.%11, sys.%14 # table_name
-% %3, %6, %11, %14 # name
+% sys.%10, sys.%20, sys.%30, sys.%40 # table_name
+% %10, %20, %30, %40 # name
% clob, clob, clob, clob # type
% 11, 11, 11, 46 # length
[ "4", "4", "4", "4" ]
@@ -182,8 +182,8 @@ stdout of test 'analytics17` in director
# group_concat(bb, aa) over (),
# group_concat(aa, 1) over (),
# group_concat(aa, 1) over () from analytics;
-% sys., sys., sys., sys., sys., sys. # table_name
-% %1, %2, %3, %4, %5, %6 # name
+% sys.%7, sys.%16, sys.%25, sys.%34, sys.%44,
sys.%54 # table_name
+% %7, %16, %25, %34, %44, %54 # name
% clob, clob, clob, clob, clob, clob # type
% 16, 19, 16, 17, 16, 16 # length
[ "1533225533446688", "3111133222211332244", "1513123523143628",
"33121532324163824", "1513121513141618", "1513121513141618" ]
@@ -201,20 +201,20 @@ stdout of test 'analytics17` in director
# group_concat(aa, NULL) over (),
# group_concat(NULL, aa) over (),
# group_concat(NULL, NULL) over () from analytics;
-% ., ., sys., ., . # table_name
-% %15, %16, %17, %20, %21 # name
+% .%12, .%24, sys.%35, .%46, .%61 # table_name
+% %12, %24, %35, %46, %61 # name
% clob, clob, clob, clob, clob # type
% 0, 10, 9, 0, 0 # length
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
-[ "", "2222222222", "153253468", "", "" ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
+[ NULL, "2222222222", "153253468", NULL, NULL ]
#create table testmore (a int, b clob);
#insert into testmore values (1, 'another'), (1, 'testing'), (1, 'todo'), (2,
'lets'), (3, 'get'), (2, 'harder'), (3, 'even'), (2, 'more'), (1, ''), (3,
'even'), (2, NULL), (1, '');
[ 12 ]
@@ -222,8 +222,8 @@ stdout of test 'analytics17` in director
# listagg(a) over (order by a),
# listagg(a, min(b)) over (),
# listagg(a, min(b)) over (partition by max(b)) from testmore group by a;
-% sys.testmore, sys., sys.%6, sys., sys.%11 # table_name
-% a, %13, %6, %15, %11 # name
+% sys.testmore, sys.%7, sys.%16, sys.%26, sys.%37 #
table_name
+% a, %7, %16, %26, %37 # name
% int, varchar, varchar, varchar, varchar # type
% 1, 5, 5, 9, 1 # length
[ 3, "3,2,1", "1,2,3", "3harder21", "3" ]
diff --git a/sql/test/group-concat/Tests/groupconcat05.sql
b/sql/test/group-concat/Tests/groupconcat05.sql
--- a/sql/test/group-concat/Tests/groupconcat05.sql
+++ b/sql/test/group-concat/Tests/groupconcat05.sql
@@ -30,6 +30,18 @@ select group_concat(b, a) from testmore;
select group_concat('😀', '😁') over () as "😃" from (values (1),(2),(3), (NULL))
v;
+select group_concat(null) || 'a';
+select group_concat(null) || 'a' from testmore;
+select group_concat(null) over () || 'a';
+select group_concat(null) over () || 'a' from testmore;
+
+select group_concat('') || 'a' where false;
+
+select group_concat('');
+select group_concat('') from testmore;
+select group_concat('') over () from testmore;
+select group_concat('', '') over () from testmore;
+
/* listagg is the SQL standard name of group_concat */
select listagg(a) from testmore;
select listagg(b) from testmore;
diff --git a/sql/test/group-concat/Tests/groupconcat05.stable.out
b/sql/test/group-concat/Tests/groupconcat05.stable.out
--- a/sql/test/group-concat/Tests/groupconcat05.stable.out
+++ b/sql/test/group-concat/Tests/groupconcat05.stable.out
@@ -150,6 +150,93 @@ stdout of test 'groupconcat05` in direct
[ "😀😁😀😁😀😁😀" ]
[ "😀😁😀😁😀😁😀" ]
[ "😀😁😀😁😀😁😀" ]
+#select group_concat(null) || 'a';
+% .%4 # table_name
+% %4 # name
+% varchar # type
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list