Skip to content

Commit e5fe34c

Browse files
NickCrewscpcloud
andauthored
perf(struct): drop unused fields on Struct.__getitem__ (#10996)
Co-authored-by: Phillip Cloud <[email protected]>
1 parent a9e01b4 commit e5fe34c

File tree

6 files changed

+37
-12
lines changed

6 files changed

+37
-12
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
SELECT
2+
123 + 1 + 1 + 1 + 1 + 1 AS "Add(Add(Add(Add(Add(123, 1), 1), 1), 1), 1)"
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
SELECT
2+
123 + 1 + 1 + 1 + 1 + 1 AS "Add(Add(Add(Add(Add(123, 1), 1), 1), 1), 1)"

ibis/backends/tests/sql/test_sql.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,3 +663,17 @@ def test_ctes_in_order():
663663

664664
sql = ibis.to_sql(expr, dialect="duckdb")
665665
assert sql.find('"first" AS (') < sql.find('"second" AS (')
666+
667+
668+
@pytest.mark.parametrize(
669+
"accessor",
670+
[
671+
pytest.param(lambda s, f: getattr(s, f), id="getattr"),
672+
pytest.param(lambda s, f: s[f], id="getitem"),
673+
],
674+
)
675+
def test_struct_field_simplified(snapshot, accessor):
676+
s = ibis.struct({"i": 123, "s": "foo"})
677+
for _i in range(5):
678+
s = ibis.struct({"i": accessor(s, "i") + 1, "s": accessor(s, "s") + "bar"})
679+
snapshot.assert_match(ibis.to_sql(s.i, dialect="duckdb"), "out.sql")

ibis/backends/tests/test_struct.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,11 @@ def test_all_fields(struct, struct_df):
7878

7979

8080
@pytest.mark.notimpl(["postgres", "risingwave"])
81-
@pytest.mark.notyet(["datafusion"], raises=Exception, reason="unsupported syntax")
82-
@pytest.mark.parametrize("field", ["a", "b", "c"])
83-
def test_literal(backend, con, field):
84-
query = _STRUCT_LITERAL[field]
85-
dtype = query.type().to_pandas()
86-
result = pd.Series([con.execute(query)], dtype=dtype)
81+
def test_literal(backend, con):
82+
dtype = _STRUCT_LITERAL.type().to_pandas()
83+
result = pd.Series([con.execute(_STRUCT_LITERAL)], dtype=dtype)
8784
result = result.replace({np.nan: None})
88-
expected = pd.Series([_SIMPLE_DICT[field]])
85+
expected = pd.Series([_SIMPLE_DICT])
8986
backend.assert_series_equal(result, expected.astype(dtype))
9087

9188

ibis/expr/types/structs.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,18 @@ def __getitem__(self, name: str) -> ir.Value:
207207
"""
208208
if name not in self.names:
209209
raise KeyError(name)
210-
return ops.StructField(self, name).to_expr()
210+
211+
op = self.op()
212+
213+
# if the underlying operation is a simple struct column access, then
214+
# just inline the underlying field access
215+
if isinstance(op, ops.StructColumn):
216+
return op.values[op.names.index(name)].to_expr()
217+
# and then do the same if the underlying value is a field access
218+
elif isinstance(op, ops.Literal):
219+
return ops.Literal(op.value[name], dtype=self.fields[name]).to_expr()
220+
else:
221+
return ops.StructField(self, name).to_expr()
211222

212223
def __setstate__(self, instance_dictionary):
213224
self.__dict__ = instance_dictionary

ibis/tests/expr/test_struct.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,14 @@ def test_struct_operations():
3131
]
3232
)
3333
expr = ibis.literal(value)
34-
assert isinstance(expr, ir.StructValue)
35-
assert isinstance(expr["b"], ir.ArrayValue)
36-
assert isinstance(expr["a"].op(), ops.StructField)
34+
assert isinstance(expr, ir.StructScalar)
35+
assert isinstance(expr["b"], ir.ArrayScalar)
36+
assert isinstance(expr["a"], ir.IntegerScalar)
3737

3838

3939
def test_struct_getattr():
4040
expr = ibis.struct({"a": 1, "b": 2})
4141
assert isinstance(expr.a, ir.IntegerValue)
42-
assert expr.a.get_name() == "a"
4342
with pytest.raises(AttributeError, match="bad"):
4443
expr.bad # # noqa: B018
4544

0 commit comments

Comments
 (0)