Repository: genjidb/genji (go)

Issue:
Aggregate queries should allow selecting field used by GROUP BY
Currently, expressions used by GROUP BY are not selectable.

```sql
genji> CREATE TABLE foo;
genji> INSERT INTO foo (a, b) VALUES (1, 10), (2, 20), (3, 30);
genji> SELECT COUNT(a), a, b from foo GROUP BY a;
{
  "COUNT(a)": 1,
  "a": null,
  "b": null
}
{
  "COUNT(a)": 1,
  "a": null,
  "b": null
}
{
  "COUNT(a)": 1,
  "a": null,
  "b": null
}
```

Expected behavior:

```sql
genji> CREATE TABLE foo;
genji> INSERT INTO foo (a, b) VALUES (1, 10), (2, 20), (3, 30);
genji> SELECT COUNT(a), a, b from foo GROUP BY a;
{
  "COUNT(a)": 1,
  "a": 1,
  "b": null
}
{
  "COUNT(a)": 1,
  "a": 2,
  "b": null
}
{
  "COUNT(a)": 1,
  "a": 3,
  "b": null
}
```

Only expressions used in the GROUP BY clause should be SELECT'able though.

Pull request:
Allow selection expression used in GROUP BY clause
This PR improves how GROUP BY and SELECT work together. From now on, when GROUP BY is used, only aggregation functions or expressions used in the GROUP BY clause can be used in SELECT:

```sql
-- valid
SELECT COUNT(id), country FROM users GROUP BY country;
SELECT age % 10 FROM users GROUP BY age % 10;

-- invalid
SELECT COUNT(id), age FROM users GROUP BY country;
field "age" must appear in the GROUP BY clause or be used in an aggregate function

SELECT age % 10 FROM users GROUP BY age;
field "age % 10" must appear in the GROUP BY clause or be used in an aggregate function
```

This PR also adds a new Aggregation node to the planner to simplify the Projection node.

Fixes #317 

Patch:
diff --git a/cmd/genji/shell/command.go b/cmd/genji/shell/command.go
index ce57db34..8de03de7 100644
--- a/cmd/genji/shell/command.go
+++ b/cmd/genji/shell/command.go
@@ -243,7 +243,6 @@ func dumpTable(tx *genji.Tx, tableName string, w io.Writer) error {
 	if _, err = buf.WriteTo(w); err != nil {
 		return err
 	}
-	buf.Reset()
 
 	// Indexes statements.
 	indexes, err := t.Indexes()
@@ -287,8 +286,6 @@ func dumpTable(tx *genji.Tx, tableName string, w io.Writer) error {
 			return err
 		}
 
-		buf.Reset()
-
 		return nil
 	})
 }
diff --git a/document/iterator.go b/document/iterator.go
index cd1f3107..13dba74f 100644
--- a/document/iterator.go
+++ b/document/iterator.go
@@ -313,7 +313,7 @@ func (s Stream) Aggregate(aggregatorBuilders ...AggregatorBuilder) Stream {
 				groupKeys = append(groupKeys, groupKey)
 				aggs = make([]Aggregator, len(aggregatorBuilders))
 				for i, builder := range aggregatorBuilders {
-					aggs[i] = builder.NewAggregator(group)
+					aggs[i] = builder.Aggregator(group)
 				}
 				aggregates[groupKey] = aggs
 			}
@@ -359,7 +359,7 @@ type Aggregator interface {
 
 // An AggregatorBuilder can build aggregators on demand.
 type AggregatorBuilder interface {
-	NewAggregator(group Value) Aggregator
+	Aggregator(group Value) Aggregator
 }
 
 // groupedDocument tags a document with a group value.
diff --git a/sql/parser/select.go b/sql/parser/select.go
index 3315f13a..2d35612b 100644
--- a/sql/parser/select.go
+++ b/sql/parser/select.go
@@ -3,6 +3,7 @@ package parser
 import (
 	"fmt"
 
+	"github.com/genjidb/genji/document"
 	"github.com/genjidb/genji/sql/planner"
 	"github.com/genjidb/genji/sql/query/expr"
 	"github.com/genjidb/genji/sql/scanner"
@@ -244,8 +245,67 @@ func (cfg selectConfig) ToTree() (*planner.Tree, error) {
 		n = planner.NewSelectionNode(n, cfg.WhereExpr)
 	}
 
+	// when using GROUP BY, only aggregation functions or GroupByExpr can be selected
 	if cfg.GroupByExpr != nil {
+		// add Group node
 		n = planner.NewGroupingNode(n, cfg.GroupByExpr)
+
+		var invalidProjectedField planner.ProjectedField
+		var aggregators []document.AggregatorBuilder
+
+		for _, pe := range cfg.ProjectionExprs {
+			pre, ok := pe.(planner.ProjectedExpr)
+			if !ok {
+				invalidProjectedField = pe
+				break
+			}
+			e := pre.Expr
+
+			// check if the projected expression is an aggregation function
+			if agg, ok := e.(document.AggregatorBuilder); ok {
+				aggregators = append(aggregators, agg)
+				continue
+			}
+
+			// check if this is the same expression as the one used in the GROUP BY clause
+			if expr.Equal(e, cfg.GroupByExpr) {
+				aggregators = append(aggregators, &planner.ProjectedGroupAggregatorBuilder{Expr: pre.Expr})
+				continue
+			}
+
+			// otherwise it's an error
+			invalidProjectedField = pre
+			break
+		}
+
+		if invalidProjectedField != nil {
+			return nil, fmt.Errorf("field %q must appear in the GROUP BY clause or be used in an aggregate function", invalidProjectedField)
+		}
+
+		// add Aggregation node
+		n = planner.NewAggregationNode(n, aggregators)
+	} else {
+		// if there is no GROUP BY clause, check if there are any aggregation function
+		// and if so add an aggregation node
+		var aggregators []document.AggregatorBuilder
+
+		for _, pe := range cfg.ProjectionExprs {
+			pre, ok := pe.(planner.ProjectedExpr)
+			if !ok {
+				continue
+			}
+			e := pre.Expr
+
+			// check if the projected expression is an aggregation function
+			if agg, ok := e.(document.AggregatorBuilder); ok {
+				aggregators = append(aggregators, agg)
+			}
+		}
+
+		// add Aggregation node
+		if len(aggregators) > 0 {
+			n = planner.NewAggregationNode(n, aggregators)
+		}
 	}
 
 	n = planner.NewProjectionNode(n, cfg.ProjectionExprs, cfg.TableName)
diff --git a/sql/planner/aggregation.go b/sql/planner/aggregation.go
new file mode 100644
index 00000000..42b1d61e
--- /dev/null
+++ b/sql/planner/aggregation.go
@@ -0,0 +1,94 @@
+package planner
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/genjidb/genji/database"
+	"github.com/genjidb/genji/document"
+	"github.com/genjidb/genji/sql/query/expr"
+)
+
+// An AggregationNode is a node that uses aggregate functions to aggregate documents from the stream into one document.
+// If the stream contains a Group node, it will generate one document per group.
+type AggregationNode struct {
+	node
+
+	Aggregators []document.AggregatorBuilder
+}
+
+var _ operationNode = (*AggregationNode)(nil)
+
+// NewAggregationNode creates an AggregationNode.
+func NewAggregationNode(n Node, aggregators []document.AggregatorBuilder) Node {
+	return &AggregationNode{
+		node: node{
+			op:   Aggregation,
+			left: n,
+		},
+		Aggregators: aggregators,
+	}
+}
+
+// Bind database resources to this node.
+func (n *AggregationNode) Bind(tx *database.Transaction, params []expr.Param) (err error) {
+	return
+}
+
+func (n *AggregationNode) toStream(st document.Stream) (document.Stream, error) {
+	return st.Aggregate(n.Aggregators...), nil
+}
+
+func (n *AggregationNode) String() string {
+	var b strings.Builder
+
+	for i, ex := range n.Aggregators {
+		if i > 0 {
+			b.WriteString(", ")
+		}
+		b.WriteString(fmt.Sprintf("%v", ex))
+	}
+
+	return fmt.Sprintf("Aggregate(%s)", b.String())
+}
+
+// ProjectedGroupAggregatorBuilder references the expression used in the GROUP BY clause
+// so that it can be used in the SELECT clause.
+type ProjectedGroupAggregatorBuilder struct {
+	Expr     expr.Expr
+	exprName string
+}
+
+// Aggregator implements the document.AggregatorBuilder interface. It creates a projectedGroupAggregator.
+func (p *ProjectedGroupAggregatorBuilder) Aggregator(group document.Value) document.Aggregator {
+	return &projectedGroupAggregator{
+		Name:  p.String(),
+		Group: group,
+	}
+}
+
+func (p *ProjectedGroupAggregatorBuilder) String() string {
+	if p.exprName == "" {
+		p.exprName = fmt.Sprintf("%v", p.Expr)
+	}
+
+	return p.exprName
+}
+
+// projectedGroupAggregator implements the document.Aggregator interface.
+// It is used to project the GROUP BY expression in the resulting document, once it's aggregated.
+type projectedGroupAggregator struct {
+	Name  string
+	Group document.Value
+}
+
+// Add doesn't do anything.
+func (p *projectedGroupAggregator) Add(d document.Document) error {
+	return nil
+}
+
+// Aggregate adds a field to the given buffer with the group value.
+func (p *projectedGroupAggregator) Aggregate(fb *document.FieldBuffer) error {
+	fb.Add(p.Name, p.Group)
+	return nil
+}
diff --git a/sql/planner/input.go b/sql/planner/input.go
index 44bcae96..83a2b435 100644
--- a/sql/planner/input.go
+++ b/sql/planner/input.go
@@ -15,6 +15,7 @@ type tableInputNode struct {
 
 	tableName string
 	table     *database.Table
+	indexes   map[string]database.Index
 	tx        *database.Transaction
 	params    []expr.Param
 }
@@ -36,6 +37,10 @@ func (n *tableInputNode) Bind(tx *database.Transaction, params []expr.Param) (er
 	n.tx = tx
 	n.params = params
 	n.table, err = tx.GetTable(n.tableName)
+	if err != nil {
+		return err
+	}
+	n.indexes, err = n.table.Indexes()
 	return
 }
 
diff --git a/sql/planner/optimizer.go b/sql/planner/optimizer.go
index b9a9dda6..fa8670a1 100644
--- a/sql/planner/optimizer.go
+++ b/sql/planner/optimizer.go
@@ -336,13 +336,9 @@ func UseIndexBasedOnSelectionNodeRule(t *Tree) (*Tree, error) {
 		return t, nil
 	}
 
-	// then we get the table indexes. here we will assume that at this point
+	// Here we will assume that at this point
 	// inputNodes can only be instances of tableInputNode.
 	inpn := inputNode.(*tableInputNode)
-	indexes, err := inpn.table.Indexes()
-	if err != nil {
-		return nil, err
-	}
 
 	type candidate struct {
 		prevNode, nextNode Node
@@ -356,7 +352,7 @@ func UseIndexBasedOnSelectionNodeRule(t *Tree) (*Tree, error) {
 	for n != nil {
 		if n.Operation() == Selection {
 			sn := n.(*selectionNode)
-			indexedNode := selectionNodeValidForIndex(sn, inpn.tableName, indexes)
+			indexedNode := selectionNodeValidForIndex(sn, inpn.tableName, inpn.indexes)
 			if indexedNode != nil {
 				candidates = append(candidates, candidate{
 					prevNode: prev,
@@ -477,7 +473,10 @@ func opCanUseIndex(op expr.Operator) (bool, expr.Path, expr.Expr) {
 	}
 
 	// expr OP path
-	if rightIsField && !leftIsField {
+	// Special case for IN operator: only left operand is valid for index usage
+	// valid:   a IN [1, 2, 3]
+	// invalid: 1 IN a
+	if rightIsField && !leftIsField && !expr.IsInOperator(op) {
 		return true, rf, op.LeftHand()
 	}
 
diff --git a/sql/planner/projection.go b/sql/planner/projection.go
index ecede40a..aeed1481 100644
--- a/sql/planner/projection.go
+++ b/sql/planner/projection.go
@@ -53,33 +53,7 @@ func (n *ProjectionNode) Bind(tx *database.Transaction, params []expr.Param) (er
 	return
 }
 
-// An AggregatorBuilder can build aggregators on demand.
-type AggregatorBuilder interface {
-	document.AggregatorBuilder
-	SetAlias(string)
-}
-
 func (n *ProjectionNode) toStream(st document.Stream) (document.Stream, error) {
-	var aggBuilders []document.AggregatorBuilder
-
-	for _, e := range n.Expressions {
-		pe, ok := e.(ProjectedExpr)
-		if !ok {
-			continue
-		}
-		if builder, ok := pe.Expr.(AggregatorBuilder); ok {
-			aggBuilders = append(aggBuilders, builder)
-
-			if pe.ExprName != "" {
-				builder.SetAlias(pe.ExprName)
-			}
-		}
-	}
-
-	if len(aggBuilders) > 0 {
-		st = st.Aggregate(aggBuilders...)
-	}
-
 	if st.IsEmpty() {
 		d := documentMask{
 			resultFields: n.Expressions,
diff --git a/sql/planner/tree.go b/sql/planner/tree.go
index da51f381..a06501cf 100644
--- a/sql/planner/tree.go
+++ b/sql/planner/tree.go
@@ -43,6 +43,8 @@ const (
 	Unset
 	// Group is an operation that groups documents based on a given path.
 	Group
+	// Aggregation is an operation that creates one document per group of documents.
+	Aggregation
 	// Dedup is an operation that removes duplicate documents from a stream
 	Dedup
 )
@@ -485,5 +487,5 @@ func (n *GroupingNode) toStream(st document.Stream) (document.Stream, error) {
 }
 
 func (n *GroupingNode) String() string {
-	return fmt.Sprintf("G(%s)", n.Expr)
+	return fmt.Sprintf("Group(%s)", n.Expr)
 }
diff --git a/sql/query/expr/function.go b/sql/query/expr/function.go
index 89b22b56..efd88e04 100644
--- a/sql/query/expr/function.go
+++ b/sql/query/expr/function.go
@@ -169,7 +169,7 @@ func (c *CountFunc) SetAlias(alias string) {
 	c.Alias = alias
 }
 
-func (c *CountFunc) NewAggregator(group document.Value) document.Aggregator {
+func (c *CountFunc) Aggregator(group document.Value) document.Aggregator {
 	return &CountAggregator{
 		Fn: c,
 	}
@@ -199,6 +199,10 @@ func (c *CountFunc) String() string {
 		return c.Alias
 	}
 
+	if c.Wildcard {
+		return "COUNT(*)"
+	}
+
 	return fmt.Sprintf("COUNT(%v)", c.Expr)
 }
 
@@ -253,8 +257,8 @@ func (m *MinFunc) SetAlias(alias string) {
 	m.Alias = alias
 }
 
-// NewAggregator implements the planner.AggregatorBuilder interface.
-func (m *MinFunc) NewAggregator(group document.Value) document.Aggregator {
+// Aggregator implements the planner.AggregatorBuilder interface.
+func (m *MinFunc) Aggregator(group document.Value) document.Aggregator {
 	return &MinAggregator{
 		Fn: m,
 	}
@@ -353,8 +357,8 @@ func (m *MaxFunc) SetAlias(alias string) {
 	m.Alias = alias
 }
 
-// NewAggregator implements the planner.AggregatorBuilder interface.
-func (m *MaxFunc) NewAggregator(group document.Value) document.Aggregator {
+// Aggregator implements the planner.AggregatorBuilder interface.
+func (m *MaxFunc) Aggregator(group document.Value) document.Aggregator {
 	return &MaxAggregator{
 		Fn: m,
 	}
@@ -453,8 +457,8 @@ func (s *SumFunc) SetAlias(alias string) {
 	s.Alias = alias
 }
 
-// NewAggregator implements the planner.AggregatorBuilder interface.
-func (s *SumFunc) NewAggregator(group document.Value) document.Aggregator {
+// Aggregator implements the planner.AggregatorBuilder interface.
+func (s *SumFunc) Aggregator(group document.Value) document.Aggregator {
 	return &SumAggregator{
 		Fn: s,
 	}
@@ -568,8 +572,8 @@ func (s *AvgFunc) SetAlias(alias string) {
 	s.Alias = alias
 }
 
-// NewAggregator implements the planner.AggregatorBuilder interface.
-func (s *AvgFunc) NewAggregator(group document.Value) document.Aggregator {
+// Aggregator implements the planner.AggregatorBuilder interface.
+func (s *AvgFunc) Aggregator(group document.Value) document.Aggregator {
 	return &AvgAggregator{
 		Fn: s,
 	}
diff --git a/sql/query/expr/path.go b/sql/query/expr/path.go
index 13ad63ed..78363e32 100644
--- a/sql/query/expr/path.go
+++ b/sql/query/expr/path.go
@@ -34,17 +34,7 @@ func (p Path) IsEqual(other Expr) bool {
 		return false
 	}
 
-	if len(p) != len(o) {
-		return false
-	}
-
-	for i := range p {
-		if p[i] != o[i] {
-			return false
-		}
-	}
-
-	return true
+	return document.Path(p).IsEqual(document.Path(o))
 }
 
 func (p Path) String() string {


Test patch:
diff --git a/sql/parser/select_test.go b/sql/parser/select_test.go
index 4576f90c..2828603f 100644
--- a/sql/parser/select_test.go
+++ b/sql/parser/select_test.go
@@ -3,6 +3,7 @@ package parser
 import (
 	"testing"
 
+	"github.com/genjidb/genji/document"
 	"github.com/genjidb/genji/sql/planner"
 	"github.com/genjidb/genji/sql/query/expr"
 	"github.com/genjidb/genji/sql/scanner"
@@ -116,20 +117,25 @@ func TestParserSelect(t *testing.T) {
 					"test",
 				)),
 			false},
-		{"WithGroupBy", "SELECT * FROM test WHERE age = 10 GROUP BY a.b.c",
+		{"WithGroupBy", "SELECT a.b.c FROM test WHERE age = 10 GROUP BY a.b.c",
 			planner.NewTree(
 				planner.NewProjectionNode(
-					planner.NewGroupingNode(
-						planner.NewSelectionNode(
-							planner.NewTableInputNode("test"),
-							expr.Eq(expr.Path(parsePath(t, "age")), expr.IntegerValue(10)),
+					planner.NewAggregationNode(
+						planner.NewGroupingNode(
+							planner.NewSelectionNode(
+								planner.NewTableInputNode("test"),
+								expr.Eq(expr.Path(parsePath(t, "age")), expr.IntegerValue(10)),
+							),
+							expr.Path(parsePath(t, "a.b.c")),
 						),
-						expr.Path(parsePath(t, "a.b.c")),
+						[]document.AggregatorBuilder{&planner.ProjectedGroupAggregatorBuilder{Expr: expr.Path(parsePath(t, "a.b.c"))}},
 					),
-					[]planner.ProjectedField{planner.Wildcard{}},
+					[]planner.ProjectedField{planner.ProjectedExpr{Expr: expr.Path(parsePath(t, "a.b.c")), ExprName: "a.b.c"}},
 					"test",
 				)),
 			false},
+		{"With Invalid GroupBy: Wildcard", "SELECT * FROM test WHERE age = 10 GROUP BY a.b.c", nil, true},
+		{"With Invalid GroupBy: a.b", "SELECT a.b FROM test WHERE age = 10 GROUP BY a.b.c", nil, true},
 		{"WithOrderBy", "SELECT * FROM test WHERE age = 10 ORDER BY a.b.c",
 			planner.NewTree(
 				planner.NewSortNode(
@@ -221,6 +227,17 @@ func TestParserSelect(t *testing.T) {
 				)),
 			false},
 		{"WithOffsetThenLimit", "SELECT * FROM test WHERE age = 10 OFFSET 20 LIMIT 10", nil, true},
+		{"With aggregation function", "SELECT COUNT(*) FROM test",
+			planner.NewTree(
+				planner.NewProjectionNode(
+					planner.NewAggregationNode(
+						planner.NewTableInputNode("test"),
+						[]document.AggregatorBuilder{&expr.CountFunc{Wildcard: true}},
+					),
+					[]planner.ProjectedField{planner.ProjectedExpr{Expr: &expr.CountFunc{Wildcard: true}, ExprName: "COUNT(*)"}},
+					"test",
+				)),
+			false},
 		{"Invalid use of MIN() aggregator", "SELECT * FROM test LIMIT min(0)", nil, true},
 		{"Invalid use of COUNT() aggregator", "SELECT * FROM test OFFSET x(*)", nil, true},
 		{"Invalid use of MAX() aggregator", "SELECT * FROM test LIMIT max(0)", nil, true},
diff --git a/sql/planner/explain_test.go b/sql/planner/explain_test.go
index 749daf26..059fdfa7 100644
--- a/sql/planner/explain_test.go
+++ b/sql/planner/explain_test.go
@@ -24,7 +24,7 @@ func TestExplainStmt(t *testing.T) {
 		{"EXPLAIN SELECT a + 1 FROM test WHERE a > 10", false, `"Index(idx_a) -> ∏(a + 1)"`},
 		{"EXPLAIN SELECT a + 1 FROM test WHERE a > 10 AND b > 20 AND c > 30", false, `"Index(idx_b) -> σ(cond: c > 30) -> σ(cond: a > 10) -> ∏(a + 1)"`},
 		{"EXPLAIN SELECT a + 1 FROM test WHERE c > 30 ORDER BY a DESC LIMIT 10 OFFSET 20", false, `"Table(test) -> σ(cond: c > 30) -> ∏(a + 1) -> Sort(a DESC) -> Offset(20) -> Limit(10)"`},
-		{"EXPLAIN SELECT a + 1 FROM test WHERE c > 30 GROUP BY b ORDER BY a DESC LIMIT 10 OFFSET 20", false, `"Table(test) -> σ(cond: c > 30) -> G(b) -> ∏(a + 1) -> Sort(a DESC) -> Offset(20) -> Limit(10)"`},
+		{"EXPLAIN SELECT a + 1 FROM test WHERE c > 30 GROUP BY a + 1 ORDER BY a DESC LIMIT 10 OFFSET 20", false, `"Table(test) -> σ(cond: c > 30) -> Group(a + 1) -> Aggregate(a + 1) -> ∏(a + 1) -> Sort(a DESC) -> Offset(20) -> Limit(10)"`},
 		{"EXPLAIN UPDATE test SET a = 10", false, `"Table(test) -> Set(a = 10) -> Replace(test)"`},
 		{"EXPLAIN UPDATE test SET a = 10 WHERE c > 10", false, `"Table(test) -> σ(cond: c > 10) -> Set(a = 10) -> Replace(test)"`},
 		{"EXPLAIN UPDATE test SET a = 10 WHERE a > 10", false, `"Index(idx_a) -> Set(a = 10) -> Replace(test)"`},
diff --git a/sql/planner/optimizer_test.go b/sql/planner/optimizer_test.go
index a0f8928e..246c5261 100644
--- a/sql/planner/optimizer_test.go
+++ b/sql/planner/optimizer_test.go
@@ -459,6 +459,38 @@ func TestUseIndexBasedOnSelectionNodeRule(t *testing.T) {
 				"foo",
 			),
 		},
+		{
+			"FROM foo WHERE a IN [1, 2]",
+			planner.NewSelectionNode(planner.NewTableInputNode("foo"),
+				expr.In(
+					expr.Path{document.PathFragment{FieldName: "a"}},
+					expr.ArrayValue(document.NewValueBuffer(document.NewIntegerValue(1), document.NewIntegerValue(2))),
+				),
+			),
+			planner.NewIndexInputNode(
+				"foo",
+				"idx_foo_a",
+				expr.In(nil, nil).(planner.IndexIteratorOperator),
+				expr.Path(parsePath(t, "a")),
+				expr.ArrayValue(document.NewValueBuffer(document.NewIntegerValue(1), document.NewIntegerValue(2))),
+				scanner.ASC,
+			),
+		},
+		{
+			"FROM foo WHERE 1 IN a",
+			planner.NewSelectionNode(planner.NewTableInputNode("foo"),
+				expr.In(
+					expr.IntegerValue(1),
+					expr.Path(parsePath(t, "a")),
+				),
+			),
+			planner.NewSelectionNode(planner.NewTableInputNode("foo"),
+				expr.In(
+					expr.IntegerValue(1),
+					expr.Path(parsePath(t, "a")),
+				),
+			),
+		},
 	}
 
 	for _, test := range tests {
diff --git a/sql/query/expr/path_test.go b/sql/query/expr/path_test.go
index c8f3a267..e4f1d62e 100644
--- a/sql/query/expr/path_test.go
+++ b/sql/query/expr/path_test.go
@@ -2,9 +2,11 @@ package expr_test
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/genjidb/genji/document"
+	"github.com/genjidb/genji/sql/parser"
 	"github.com/genjidb/genji/sql/query/expr"
 	"github.com/stretchr/testify/require"
 )
@@ -49,3 +51,28 @@ func TestPathExpr(t *testing.T) {
 		testExpr(t, "a", expr.EvalStack{}, nullLitteral, true)
 	})
 }
+
+func TestPathIsEqual(t *testing.T) {
+	tests := []struct {
+		a, b    string
+		isEqual bool
+	}{
+		{`a`, `a`, true},
+		{`a[0].b`, `a[0].b`, true},
+		{`a[0].b`, `a[1].b`, false},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%s = %s", test.a, test.b), func(t *testing.T) {
+			pa, err := parser.ParsePath(test.a)
+			require.NoError(t, err)
+			ea := expr.Path(pa)
+
+			pb, err := parser.ParsePath(test.b)
+			require.NoError(t, err)
+			eb := expr.Path(pb)
+
+			require.Equal(t, test.isEqual, ea.IsEqual(eb))
+		})
+	}
+}
diff --git a/sql/query/select_test.go b/sql/query/select_test.go
index 37359b76..c814435f 100644
--- a/sql/query/select_test.go
+++ b/sql/query/select_test.go
@@ -50,7 +50,7 @@ func TestSelectStmt(t *testing.T) {
 		{"With IN op on PK", "SELECT color FROM test WHERE k IN [1.1, 1.0] ORDER BY k", false, `[{"color":"red"}]`, nil},
 		{"With NOT IN op", "SELECT color FROM test WHERE color NOT IN ['red', 'purple'] ORDER BY k", false, `[{"color":"blue"}]`, nil},
 		{"With field comparison", "SELECT * FROM test WHERE color < shape", false, `[{"k":1,"color":"red","size":10,"shape":"square"}]`, nil},
-		{"With group by", "SELECT * FROM test GROUP BY color", false, `[{"k":1,"color":"red","size":10,"shape":"square"},{"k":2,"color":"blue","size":10,"weight":100},{"k":3,"height":100,"weight":200}]`, nil},
+		{"With group by", "SELECT color FROM test GROUP BY color", false, `[{"color":"red"},{"color":"blue"},{"color":null}]`, nil},
 		{"With group by and count", "SELECT COUNT(k) FROM test GROUP BY size", false, `[{"COUNT(k)":2},{"COUNT(k)":1}]`, nil},
 		{"With group by and count wildcard", "SELECT COUNT(*  ) FROM test GROUP BY size", false, `[{"COUNT(*  )":2},{"COUNT(*  )":1}]`, nil},
 		{"With order by", "SELECT * FROM test ORDER BY color", false, `[{"k":3,"height":100,"weight":200},{"k":2,"color":"blue","size":10,"weight":100},{"k":1,"color":"red","size":10,"shape":"square"}]`, nil},