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

Reply via email to