From e94c25245dda4986839c3b2de3d08e7087251dd1 Mon Sep 17 00:00:00 2001
From: WhiteGobo <richi.fechner@web.de>
Date: Sat, 8 Jul 2023 22:38:46 +0200
Subject: [PATCH] fix: `GROUP_CONCAT` handling of empty separator (issue)
 (#2474)

`GROUP_CONCAT` was handling an empty separator (i.e. `""`) incorrectly,
it would handle it as if the separator were not set, so essentially it was
treated as a single space (i.e. `" "`).

This change fixes it so that an empty separator with `GROUP_CONCAT`
results in a value with nothing between concatenated values.


Fixes <https://github.com/RDFLib/rdflib/issues/2473>

---------

Co-authored-by: WhiteGobo <richardfechner@posteo.net>
---
 rdflib/plugins/sparql/aggregates.py        |  9 +++++++--
 test/test_sparql/test_translate_algebra.py | 23 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/rdflib/plugins/sparql/aggregates.py b/rdflib/plugins/sparql/aggregates.py
index d4a7d6592..84ac8936d 100644
--- a/rdflib/plugins/sparql/aggregates.py
+++ b/rdflib/plugins/sparql/aggregates.py
@@ -245,11 +245,16 @@ def get_value(self) -> None:
 
 
 class GroupConcat(Accumulator):
-    def __init__(self, aggregation):
+    value: List[Literal]
+
+    def __init__(self, aggregation: CompValue):
         super(GroupConcat, self).__init__(aggregation)
         # only GROUPCONCAT needs to have a list as accumulator
         self.value = []
-        self.separator = aggregation.separator or " "
+        if aggregation.separator is None:
+            self.separator = " "
+        else:
+            self.separator = aggregation.separator
 
     def update(self, row: FrozenBindings, aggregator: "Aggregator") -> None:
         try:
diff --git a/test/test_sparql/test_translate_algebra.py b/test/test_sparql/test_translate_algebra.py
index 20b23327a..ca9e67bdf 100644
--- a/test/test_sparql/test_translate_algebra.py
+++ b/test/test_sparql/test_translate_algebra.py
@@ -11,6 +11,7 @@
 
 import rdflib.plugins.sparql.algebra as algebra
 import rdflib.plugins.sparql.parser as parser
+from rdflib import Graph, Literal, URIRef
 from rdflib.plugins.sparql.algebra import translateAlgebra
 
 
@@ -304,3 +305,25 @@ def test_roundtrip(test_spec: AlgebraTest, data_path: Path) -> None:
     # TODO: Execute the raw query (query_text) and the reconstituted query
     # (query_from_query_from_algebra) against a well defined graph and ensure
     # they yield the same result.
+
+
+def test_sparql_group_concat():
+    """Tests if GROUP_CONCAT correctly uses the separator keyword"""
+    query = """
+    PREFIX : <http://example.org/>
+
+    SELECT ?subject (GROUP_CONCAT(?object; separator="")
+                        AS ?concatenatedObjects)
+    WHERE {
+      VALUES (?subject ?object) {
+        (:pred "a")
+        (:pred "b")
+        (:pred "c")
+      }
+    }
+    GROUP BY ?subject
+    """
+
+    g = Graph()
+    q = dict(g.query(query))
+    assert q[URIRef("http://example.org/pred")] == Literal("abc")