Skip to content

Commit

Permalink
orderby: fix evaluation of ORDER BY expr when used with projection (#471
Browse files Browse the repository at this point in the history
)

When the ORDER BY expression uses fields that are not present in the preceding projection operator,
it was evaluating the expression to NULL, which was breaking the ordering.
It now evaluates the expr against the result of the projection first, then if it is NULL
evaluates it against the original document.

Example:
```bash
table.Scan("test") | docs.Project(b) | docs.TempTreeSort(a)
```
  • Loading branch information
asdine committed Jul 14, 2022
1 parent bfd7f60 commit c9f6a47
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 9 deletions.
12 changes: 12 additions & 0 deletions internal/stream/docs/temp_tree_sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package docs
import (
"fmt"

"github.com/cockroachdb/errors"
"github.com/genjidb/genji/internal/encoding"
"github.com/genjidb/genji/internal/environment"
"github.com/genjidb/genji/internal/expr"
Expand Down Expand Up @@ -51,6 +52,17 @@ func (op *TempTreeSortOperator) Iterate(in *environment.Environment, fn func(out
return err
}

if types.IsNull(v) {
// the expression might be pointing to the original document.
v, err = op.Expr.Eval(out.Outer)
if err != nil {
// the only valid error here is a missing field.
if !errors.Is(err, types.ErrFieldNotFound) {
return err
}
}
}

doc, ok := out.GetDocument()
if !ok {
panic("missing document")
Expand Down
65 changes: 56 additions & 9 deletions sqltests/SELECT/order_by.sql
Original file line number Diff line number Diff line change
@@ -1,37 +1,84 @@
-- setup:
CREATE TABLE test(a double);
INSERT INTO test (a) VALUES (1), (2), (3);
CREATE TABLE test(a double, b double);
INSERT INTO test (a, b) VALUES (50, 3), (100, 4), (10, 2), (null, 1);

-- suite: no index

-- suite: with index
CREATE INDEX ON test(a);

-- test: asc
SELECT b FROM test ORDER BY a;
/* result:
{
b: 1.0,
}
{
b: 2.0
}
{
b: 3.0
}
{
b: 4.0
}
*/


-- test: asc / wildcard
SELECT * FROM test ORDER BY a;
/* result:
{
a: 1.0
b: 1.0,
}
{
a: 2.0
a: 10.0,
b: 2.0
}
{
a: 3.0
a: 50.0,
b: 3.0
}
{
a: 100.0,
b: 4.0
}
*/


-- test: desc
SELECT * FROM test ORDER BY a DESC;
SELECT b FROM test ORDER BY a DESC;
/* result:
{
a: 3.0
b: 4.0,
}
{
b: 3.0
}
{
a: 2.0
b: 2.0
}
{
a: 1.0
b: 1.0
}
*/

-- test: desc / wildcard
SELECT * FROM test ORDER BY a DESC;
/* result:
{
a: 100.0,
b: 4.0,
}
{
a: 50.0,
b: 3.0
}
{
a: 10.0,
b: 2.0
}
{
b: 1.0
}
*/
4 changes: 4 additions & 0 deletions types/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ func Is[T any](v Value) (T, bool) {
return vv.v, true
}

func IsNull(v Value) bool {
return v == nil || v.Type() == NullValue
}

// IsTruthy returns whether v is not equal to the zero value of its type.
func IsTruthy(v Value) (bool, error) {
if v.Type() == NullValue {
Expand Down

0 comments on commit c9f6a47

Please sign in to comment.