chaisql / chai

Modern embedded SQL database
MIT License
1.56k stars 95 forks source link

orderby: fix evaluation of ORDER BY expr when used with projection #471

Closed asdine closed 2 years ago

asdine commented 2 years ago

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:

table.Scan("test") | docs.Project(b) | docs.TempTreeSort(a)

Fixes #466

codecov[bot] commented 2 years ago

Codecov Report

Merging #471 (697d94e) into main (67bf95b) will increase coverage by 0.03%. The diff coverage is 71.42%.

@@            Coverage Diff             @@
##             main     #471      +/-   ##
==========================================
+ Coverage   79.18%   79.21%   +0.03%     
==========================================
  Files         128      128              
  Lines       10708    10715       +7     
==========================================
+ Hits         8479     8488       +9     
+ Misses       1531     1529       -2     
  Partials      698      698              
Impacted Files Coverage Δ
internal/stream/docs/temp_tree_sort.go 75.86% <60.00%> (-1.50%) :arrow_down:
types/value.go 80.83% <100.00%> (+0.16%) :arrow_up:
internal/encoding/helpers.go 91.89% <0.00%> (+0.90%) :arrow_up:
internal/expr/operator.go 74.57% <0.00%> (+3.38%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more