Skip to content

Commit

Permalink
fix(sql): overwrite the original sort key on successive order_by call…
Browse files Browse the repository at this point in the history
…s ordering by the same key
  • Loading branch information
kszucs authored and jcrist committed Feb 27, 2024
1 parent 6ade4e9 commit 103dc68
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 2 deletions.
7 changes: 5 additions & 2 deletions ibis/backends/sql/rewrites.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,14 @@ def merge_select_select(_, **kwargs):
predicates = tuple(p.replace(subs, filter=ops.Value) for p in _.predicates)
sort_keys = tuple(s.replace(subs, filter=ops.Value) for s in _.sort_keys)

unique_predicates = toolz.unique(_.parent.predicates + predicates)
unique_sort_keys = {s.expr: s for s in _.parent.sort_keys + sort_keys}

return Select(
_.parent.parent,
selections=selections,
predicates=tuple(toolz.unique(_.parent.predicates + predicates)),
sort_keys=tuple(toolz.unique(_.parent.sort_keys + sort_keys)),
predicates=unique_predicates,
sort_keys=unique_sort_keys.values(),
)


Expand Down
20 changes: 20 additions & 0 deletions ibis/backends/tests/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1860,3 +1860,23 @@ def test_isnull_equality(con, backend, monkeypatch):
expected = pd.DataFrame({"out": [True, False, True]})

backend.assert_frame_equal(result, expected)


@pytest.mark.broken(
["druid"],
raises=PyDruidProgrammingError,
reason=(
"Query could not be planned. A possible reason is [SQL query requires ordering a "
"table by non-time column [[id]], which is not supported."
),
)
def test_subsequent_overlapping_order_by(con, backend, alltypes, df):
ts = alltypes.order_by(ibis.desc("id")).order_by("id")
result = con.execute(ts)
expected = df.sort_values("id").reset_index(drop=True)
backend.assert_frame_equal(result, expected)

ts2 = ts.order_by(ibis.desc("id"))
result = con.execute(ts2)
expected = df.sort_values("id", ascending=False).reset_index(drop=True)
backend.assert_frame_equal(result, expected)
7 changes: 7 additions & 0 deletions ibis/expr/tests/test_newrels.py
Original file line number Diff line number Diff line change
Expand Up @@ -1560,3 +1560,10 @@ def test_inner_join_convenience():
# finish to evaluate the collisions
result = fifth_join._finish().op()
assert result == expected


def test_subsequent_order_by_calls():
ts = t.order_by(ibis.desc("int_col")).order_by("int_col")
first = ops.Sort(t, [t.int_col.desc()]).to_expr()
second = ops.Sort(first, [first.int_col.asc()]).to_expr()
assert ts.equals(second)

0 comments on commit 103dc68

Please sign in to comment.