Skip to content

Commit

Permalink
fix(ir): error for order_by on nonexistent column
Browse files Browse the repository at this point in the history
  • Loading branch information
jcrist authored and kszucs committed Jan 19, 2023
1 parent e4bae26 commit 57b1dd8
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 5 deletions.
15 changes: 10 additions & 5 deletions ibis/expr/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,16 @@ def sort_key_from(table_ref, key, **kwargs):
else:
key, order = key, True

key = one_of(
(function_of(table_ref), column_from(table_ref), any),
key,
**kwargs,
)
if isinstance(key, (str, int)):
# Actual str/int keys must refer to columns in the table, we don't want
# to fallback to converting them to expressions with ibis.literal
key = column_from(table_ref, key, **kwargs)
else:
key = one_of(
(function_of(table_ref), column_from(table_ref), any),
key,
**kwargs,
)

if isinstance(order, str):
order = order.lower()
Expand Down
30 changes: 30 additions & 0 deletions ibis/tests/expr/test_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,36 @@ def test_order_by_scalar(table, key, expected):
assert result.op().sort_keys == (ops.SortKey(expected),)


@pytest.mark.parametrize(
"key",
[
"bogus",
("bogus", False),
ibis.desc("bogus"),
1000,
(1000, False),
_.bogus,
_.bogus.desc(),
],
)
@pytest.mark.parametrize(
"expr_func",
[
param(lambda t: t, id="table"),
param(lambda t: t.select("a", "b"), id="selection"),
param(lambda t: t.group_by("a").agg(new=_.b.sum()), id="aggregation"),
],
)
def test_order_by_nonexistent_column_errors(table, expr_func, key):
# `order_by` is implemented on a few different operations, we check them
# all in turn here.
expr = expr_func(table)
# Depending on the expression, this can raise a few different errors.
# For now just check that an exception is raised.
with pytest.raises(Exception):
expr.order_by(key)


def test_slice(table):
expr = table[:5]
expr2 = table[:5:1]
Expand Down

0 comments on commit 57b1dd8

Please sign in to comment.