Enable more clippy lints - #24322

The `.peekable()` in the sqllogictest Postgres engine was never peeked,
so it only added an extra layer of indirection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Catches `From` implementations that can panic, where `TryFrom` would be
the honest signature.

All three existing hits would need a breaking API change to fix, so they
get `#[expect]` for now. Two of them (`Constraint`) panic on a protobuf
message with an unset `constraint_mode`, i.e. on malformed input.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Catches exact float comparisons against constants, e.g. `x == 0.0`.

The two existing hits in `value_transition!` really do want an exact
comparison against `f32::MIN`/`MAX`, so they get `#[expect]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Catches float literals that silently round, e.g. `let x: f32 = 0.1234567890123;`.

The three existing hits are false positives: they spell out exact powers
of two (2^64 and 2^64-2^41), which float `Display` renders with fewer
digits, so the lint thinks precision was lost.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`(a + b) / 2` overflows when `a + b` exceeds the type's range;
`a.midpoint(b)` does not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Catches strings that contain `{...}` but are never actually formatted.

This found two real bugs where the placeholder was silently printed
verbatim:
* `benchmarks/src/nlj.rs`: `"NLJ benchmark Q{query_id} failed…".to_string()`
* `parquet_advanced_index.rs`: `.expect("metadata for file not found: {filename}")`

The remaining hits are intentional: shell-style `${VAR:-default}`
placeholders, `{rows}`-style templates substituted with `str::replace`,
and braces inside expected struct output. Those get `#[expect]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`&x as *const T` silently picks a pointer type; `std::ptr::from_ref(&x)`
keeps the referent type explicit and cannot accidentally change it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Catches `// SAFETY:` comments that do not sit in front of anything
unsafe, so that a `SAFETY:` comment reliably means "an unsafe block
follows, and here is why it is sound".

* Three comments documented an `unwrap` or a safe copy rather than
  unsafe code, so they lose the `SAFETY:` prefix.
* Three sat in front of an `if` while the `unsafe` block was inside it,
  so they move next to the block they justify.
* Two were prose false positives, where the lint matched "safety:" in
  the middle of a doc comment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Catches large types passed by value, which forces a memcpy at every call.

The single existing hit is `HyperLogLog::new_with_registers`, whose 16 KiB
array is moved into the returned struct, so a reference would only add a
copy. It gets `#[expect]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Catches `-> Box<T>` where the caller gains nothing from the indirection.

The single existing hit is a test helper that both takes and returns
`Box<Expr>` so it can hand back the same allocation, so it gets
`#[expect]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`.filter_map(f).next()` is `.find_map(f)`, which stops at the first hit
without building the intermediate adapter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Enforces uniform digit grouping in long literals, where an odd group is
usually a typo.

The three existing hits are deliberate: the grouping spells out the
decimal scale, so `180_00000000` reads as 180 with scale 8. They get
`#[expect]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`opt.map(f).unwrap_or_default()` and `opt.filter(f).is_some()` are both
`opt.is_some_and(f)`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`Duration::from_secs(60)` says "60 seconds" where the code means one
minute. `from_mins(1)` / `from_hours(..)` say it directly.

Both constructors are stable since Rust 1.91, below our 1.94 MSRV.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A named lifetime that is used only once carries no information; `'_`
makes it obvious that nothing is being tied together.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Initializing fields in declaration order makes a struct literal easy to
check against the definition, and makes it obvious when a field is
missing. All hits are pure reorderings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`if cond { 1 } else { 0 }` is `T::from(cond)`, which cannot get the two
branches the wrong way round.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`T { ..Default::default() }` and `Self { ..self.clone() }` are just
`T::default()` and `self.clone()`.

Where clippy suggested a bare `Default::default()` the concrete type name
is kept, since it is what tells the reader what is being built.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`if !cond { panic!(msg) }` is `assert!(cond, msg)`, which states the
invariant instead of its negation.

One of clippy's rewrites produced a double negative
(`!...is_none()`); that one is written as `.is_some()` instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A `..` in a pattern that already binds every field does nothing today,
but silently swallows any field added later. Removing it turns that into
a compile error.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`if let Some(true) = x` reads as a binding but is really an equality
check; `x == Some(true)` (or `matches!`) says so.

Clippy suggested one tuple comparison, `(is_valid, is_included) == (true,
Some(true))`; that one is written as a plain `&&` instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`x.deref()` and `x.deref_mut()` are the operator spelled the long way;
`&*x` / `&mut *x` is the idiomatic form and does not need `Deref` in
scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

This reverts commit c7e25ff.

Addresses review feedback on the `equatable_if_let` commit: clippy
suggested `matches!` in a few places where a plain equality check reads
better.

`predicate_bounds.rs` uses `.ok() == Some(false)` because
`DataFusionError` does not implement `PartialEq`, so `== Ok(false)` does
not compile.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

This reverts commit d82dc41.

This reverts commit 36abeb7.

This reverts commit 85c06c6.

* `test_parse_duration_with_overflow_check` uses `Duration::from_mins`
  for the `"…m"` input again, so the constructor mirrors the unit suffix
  in the string being parsed. That trips
  `duration_suboptimal_units`, so the test gets an `#[expect]` saying why.
* Restore the `TODO`/`Issue` comments above the ignored
  `sort_with_mem_limit_2_cols_2` test, keeping a short `#[ignore]` reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>