Skip to content

Commit 35483fb

Browse files
committed
fix(join): disallow "", require "{name}" instead
There is this code inside expr/types/join.py: ```python def disambiguate_fields( how, predicates, equalities, left_fields, right_fields, left_template, right_template, ): """Resolve name collisions between the left and right tables.""" collisions = set() left_template = left_template or "{name}" right_template = right_template or "{name}" ... ``` This made it so that "" was treated equivalently to "{name}". But I think this is inconsistent, it is a remnant from when we had the rsuffix and lsuffix API. I dug into this when I was confused as to the logic behind how the default values of "" resulted in the input column. This doesn't have any impact on default behavior, since I adjusted the default values to. If someone WAS passing in a falsy value, such as "", False, or None, then this is breaking.
1 parent 6e2664f commit 35483fb

File tree

3 files changed

+8
-10
lines changed

3 files changed

+8
-10
lines changed

ibis/expr/types/joins.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ def disambiguate_fields(
4242
):
4343
"""Resolve name collisions between the left and right tables."""
4444
collisions = set()
45-
left_renamer = left_renamer or "{name}"
46-
right_renamer = right_renamer or "{name}"
4745

4846
if how == "inner" and util.all_of(predicates, ops.Equals):
4947
# for inner joins composed exclusively of equality predicates, we can
@@ -229,7 +227,7 @@ def join(
229227
predicates: Any = (),
230228
how: JoinKind = "inner",
231229
*,
232-
lname: str = "",
230+
lname: str = "{name}",
233231
rname: str = "{name}_right",
234232
):
235233
if not isinstance(right, Table):
@@ -285,7 +283,7 @@ def asof_join(
285283
predicates=(),
286284
tolerance=None,
287285
*,
288-
lname: str = "",
286+
lname: str = "{name}",
289287
rname: str = "{name}_right",
290288
):
291289
predicates = util.promote_list(predicates)
@@ -363,7 +361,7 @@ def cross_join(
363361
self: Table,
364362
right: Table,
365363
*rest: Table,
366-
lname: str = "",
364+
lname: str = "{name}",
367365
rname: str = "{name}_right",
368366
):
369367
left = self.join(right, how="cross", predicates=(), lname=lname, rname=rname)

ibis/expr/types/relations.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ def f( # noqa: D417
6565
| Sequence[str | tuple[str | ir.Column, str | ir.Column] | ir.BooleanValue]
6666
) = (),
6767
*,
68-
lname: str = "",
68+
lname: str = "{name}",
6969
rname: str = "{name}_right",
7070
) -> ir.Table:
7171
"""Perform a join between two tables.
@@ -3007,7 +3007,7 @@ def join(
30073007
) = (),
30083008
how: JoinKind = "inner",
30093009
*,
3010-
lname: str = "",
3010+
lname: str = "{name}",
30113011
rname: str = "{name}_right",
30123012
) -> Table:
30133013
"""Perform a join between two tables.
@@ -3177,7 +3177,7 @@ def asof_join(
31773177
predicates: str | ir.Column | Sequence[str | ir.Column] = (),
31783178
tolerance: str | ir.IntervalScalar | None = None,
31793179
*,
3180-
lname: str = "",
3180+
lname: str = "{name}",
31813181
rname: str = "{name}_right",
31823182
) -> Table:
31833183
"""Perform an "as-of" join between `left` and `right`.
@@ -3301,7 +3301,7 @@ def cross_join(
33013301
left: Table,
33023302
right: Table,
33033303
*rest: Table,
3304-
lname: str = "",
3304+
lname: str = "{name}",
33053305
rname: str = "{name}_right",
33063306
) -> Table:
33073307
"""Compute the cross join of a sequence of tables.

ibis/tests/expr/test_table.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1780,7 +1780,7 @@ def test_join_lname_rname(how):
17801780
expr = method(right, rname="right_{name}")
17811781
assert expr.columns == ("id", "first_name", "right_id", "last_name")
17821782

1783-
expr = method(right, lname="left_{name}", rname="")
1783+
expr = method(right, lname="left_{name}", rname="{name}")
17841784
assert expr.columns == ("left_id", "first_name", "id", "last_name")
17851785

17861786
expr = method(right, rname="right_{name}", lname="left_{name}")

0 commit comments

Comments
 (0)