Repository navigation
fix: align list.contains null handling with Polars on lazy backends - #3990
Conversation
False\ from \list.contains\ on no match when the list holds nullsFalse from list.contains on no match when the list holds nulls
There was a problem hiding this comment.
Hey @jonasdedden thanks for taking care of this. As I saw array_compact I had two questions:
- What happens if
item=None? - That seem like an expensive op to run, are there going to be performance regressions?
I double checked with Opus 5.5, and it's proposing an alternative workaround keeping array_contains as is and only turning its null into False when the list itself is not null gives the same results at no extra cost:
return F.coalesce(
F.array_contains(expr, F.lit(item)),
F.when(expr.isNotNull(), F.lit(False)),
)which takes care of both questions.
Claude generated benchmark for sqlframe and pyspark
"""Benchmark spark-like `list.contains` variants on PySpark and SQLFrame.
Usage: uv run --with="pandas==2.3" --with="pyspark==4.2.0" --with="sqlframe==4.4.0" --with="duckdb==1.5.5" t.py [pyspark|sqlframe|all]
"""
from __future__ import annotations
import statistics
import sys
import tempfile
import time
from typing import Any, Callable
REPEATS = 10
# (rows, list length); every 5th element is null, and the item (-1) is never found.
CASES = [(5_000_000, 10), (1_000_000, 100)]
def variants(F: Any) -> dict[str, Callable[[Any], Any]]:
item = F.lit(-1)
return {
"main": lambda a: F.array_contains(a, item),
"array_compact": lambda a: F.array_contains(F.array_compact(a), item),
"coalesce": lambda a: F.coalesce(
F.array_contains(a, item), F.when(a.isNotNull(), F.lit(False))
),
}
def timeit(run: Callable[[], object]) -> float:
run()
times = []
for _ in range(REPEATS):
start = time.perf_counter()
run()
times.append(time.perf_counter() - start)
return statistics.median(times)
def bench_pyspark() -> None:
from pyspark.sql import SparkSession, functions as F
spark = (
SparkSession.builder.master("local[4]")
.config("spark.ui.enabled", "false")
.config("spark.driver.memory", "6g")
.getOrCreate()
)
spark.sparkContext.setLogLevel("ERROR")
with tempfile.TemporaryDirectory() as tmp:
for rows, length in CASES:
path = f"{tmp}/data_{length}.parquet"
spark.range(rows).select(
F.transform(
F.sequence(F.lit(0), F.lit(length - 1)),
lambda x: F.when(x % 5 == 0, None).otherwise(x + F.col("id") % 3),
).alias("a")
).write.parquet(path)
df = spark.read.parquet(path)
for name, fn in variants(F).items():
# A parquet source and a noop sink force every row to be evaluated;
# `cache()` + `collect()` of an aggregate gave misleading timings.
q = df.select(fn(F.col("a")).alias("r"))
counts = {r["r"]: r["count"] for r in q.groupBy("r").count().collect()}
elapsed = timeit(lambda: q.write.format("noop").mode("overwrite").save())
print(f"pyspark rows={rows:>9,} len={length:>3} {name:<14} {elapsed:.3f}s {counts}")
def bench_sqlframe() -> None:
import duckdb
from sqlframe.duckdb import DuckDBSession, functions as F
con = duckdb.connect()
for rows, length in CASES:
con.execute(
f"CREATE OR REPLACE TABLE t AS SELECT list_transform(range({length}), "
f"x -> CASE WHEN x % 5 = 0 THEN NULL ELSE x + i % 3 END) AS a "
f"FROM range({rows}) r(i)"
)
df = DuckDBSession(conn=con).table("t")
for name, fn in variants(F).items():
q = df.select(F.sum(fn(F.col("a")).cast("int")).alias("s"))
result = q.collect()[0][0]
elapsed = timeit(q.collect)
print(f"sqlframe rows={rows:>9,} len={length:>3} {name:<14} {elapsed:.3f}s sum={result}")
if __name__ == "__main__":
target = sys.argv[1] if len(sys.argv) > 1 else "all"
if target in {"sqlframe", "all"}:
bench_sqlframe()
if target in {"pyspark", "all"}:
bench_pyspark()Median of 5 runs. Every 5th element is null, and the item is never found (the worst case for the bug):
| backend | rows | list len | main |
array_compact (this PR) |
coalesce |
|---|---|---|---|---|---|
| PySpark | 5M | 10 | 0.162s | 1.212s | 0.135s |
| PySpark | 1M | 100 | 0.178s | 2.378s | 0.183s |
| SQLFrame | 5M | 10 | 0.008s | 0.053s | 0.009s |
| SQLFrame | 1M | 100 | 0.015s | 0.098s | 0.015s |
On top of this, it would be nice if you can add a test with item=None, which we don't have yet 🫥
…compact` in Spark Use `coalesce(array_contains(...), when(list is not null, False))` for Spark instead of compacting the list first, which was up to ~10x slower. Handle `item=None` explicitly on Spark-like, DuckDB and Ibis, where native `contains` returned null (or raised on PySpark) instead of matching Polars. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ba9JftUrcct4rBVY5qsB1C
|
@FBruzzesi thank you so much for your investigation!! I had an interactive look with Claude and I think it implemented your suggestion quite well. |
FBruzzesi
left a comment
There was a problem hiding this comment.
Thank you for checking and resolving the issue. I’ve carried out a similar review, taking performance into account. I believe there are still some potential performance issues - Added inline suggestions to address them. Local runtimes:
| backend | variant | null first | null last | no nulls |
|---|---|---|---|---|
| duckdb | PR (list_count) | 0.034s | 0.034s | 0.043s |
| duckdb | array_position | 0.009s | 0.014s | 0.011s |
| pyspark | PR (array_compact) | 2.562s | 2.537s | 2.542s |
| pyspark | exists | 0.199s | 2.480s | 2.491s |
… old Polars Use `array_position(..., NULL)` on DuckDB>=1.3 and `exists` on PySpark, which can short-circuit instead of scanning the whole list. Handle `list.contains(None)` on Polars<1.24, which returned null. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ba9JftUrcct4rBVY5qsB1C
…rk and Ibis On PySpark, `exists` only short-circuits when a null comes early; with a late or no null it is as slow as `array_compact` (lambda evaluation). Sorting (`sort_array` places nulls first) is ~4-7x faster in that case. On Ibis, filtering for nulls is cheaper than filtering for non-nulls and comparing lengths. Extend the `item=None` test with duplicate elements. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ba9JftUrcct4rBVY5qsB1C
False from list.contains on no match when the list holds nullslist.contains null handling with Polars on lazy backends
FBruzzesi
left a comment
There was a problem hiding this comment.
Thanks a ton @jonasdedden - I really like where we landed after these iterations, especially the last commit is a really good improvements I didn't know about
|
Ah, it maybe wasn't the best idea to have this merged already, this was still in progress 😔 I think the last commit was just a hallucination, and mostly based on really stupid test data (basically already sorted data, where Spark's TimSort is returning results way faster). I'm still running benchmarks, but now it seems:
I.e. the "sort values and test for first one" probably is garbage, performing abysmal on very large data (think of O(n log n) vs. O(n), and highly aggregate computation [distributed sorting] vs. embarrassingly parallel single data passes [exist]). |
|
I'd probably recommend reverting the last commit and reopening this PR. Will have a look tomorrow |
|
Thanks for raising this, I should have run my own benchmark 🙈 Reverting now |
|
@jonasdedden I don't think I have a way to re-open the PR directly. You can open a new one once you think it's ready! |
|
Reply: (commit is f7b5af4 but unfortunately PR is merged already) Yes, the sorting fix got worse and worse with list length, and my earlier benchmark hid that. My test lists were already in ascending order, which Spark's sort handles in one pass. On random data it falls behind Nanoseconds per element, 20M elements in total, PySpark 4.2
Holding total elements fixed means a one-pass method shows a flat cost per element.
|
|
Feel free to do things, if you want, but I'll have a actual look earliest tomorrow. Sorry for the hassle! |

Description
list.containsdisagreed with Polars around nulls on the lazy backends:[1, None],.contains(2)Falsenull(Spark'sarray_containsuses three-valued logic)[1, None],.contains(None)Truenullfor every row; PySpark: raisesAnalysisException.contains(None)on Polars < 1.24nullThis PR aligns all of them with Polars, using a native check where the backend has one.
A null list still returns
nullon every backend.Surfaced in review of #3915, whose numeric coercion cases hit this on PySpark. This should be merged first; #3915 then merges on top without a rebase.
What type of PR is this? (check all applicable)
Related issues
InvalidOperationErrorfor mismatchedlist.containsitems on pyarrow-backed backends #3915AI assistance
Checklist
Code follows style guide (ruff)
Tests added
Documented the changes (N/A, aligns PySpark with the other backends)
If this is your first PR to narwhals, attach a screenshot of
pytestpassing locally (not CI):Note: the full-suite command above was not run. Targeted run instead:
pytest tests/expr_and_series/list --constructors=pyspark,sqlframe,duckdb,ibis,polars[eager],polars[lazy],pandas,pyarrowgives 167 passed, 19 xfailed (PySpark 4.2.0). The new test fails onpysparkwithout the fix. PySpark Connect and the 3.5.0 minimum were not run locally;array_compacthas been available since Spark 3.4.