apache / apache/datafusion

Improve performance of `PartialOrd` for logical nodes

Open
#17,477 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

`PartialOrd` / `PartialEq` are used during planning and thus affect planning performance.

We found some improvements that are possible in
- https://github.com/apache/datafusion/pull/17438

I think `self == other` will (re) compare all entries in `self.values` and `other.values` again

we can probably avoid checking `values == other.values` here by just checking the missing field.

however, it's important to write the PartialOrd / Ord implementation so that every field is explicitly handled (addition of a new field without changing the impl should result in the compile-time error)

```suggestion
.filter(|cmp| *cmp != Ordering::Equal || self.schema == other.schema)
```

_Originally posted by @alamb in https://github.com/apache/datafusion/pull/17438#discussion_r2325757148_

Contributor guide

Open the contributing guide

Research direction

Start by reading the PartialOrd and PartialEq implementations for logical nodes in PR 17438, focusing on the planning-performance discussion and the comparison of values and schema. Update the implementation so every field is explicitly handled and redundant comparisons are avoided; done means the comparison behavior is covered without relying on implicit field additions.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.