Global/LocalLimitExec::required_ordering is not serialized, losing order-sensitivity of a pushed-down LIMIT #24173

Describe the bug

GlobalLimitExec::required_ordering and LocalLimitExec::required_ordering are not serialized. GlobalLimitExecNode carries only input, skip and fetch; LocalLimitExecNode only input and fetch. After a protobuf round-trip the field decodes as None.

This is not cosmetic state. It is the only surviving record that a limit is order-sensitive.

The analysis below is @andygrove's, from review of #24167; I've verified each link against main (92f4e8f3ee) and the citations are current.

How the field gets set. enforce_sorting sets it precisely when it deletes a redundant SortExec that had a fetch — the ordering requirement has nowhere else to live once the sort node is gone (enforce_sorting/mod.rs:518,522):

if let Some(fetch) = sort_exec.fetch() {
    let required_ordering = sort_exec.properties().output_ordering().cloned();
    // ... GlobalLimitExec or LocalLimitExec ...
    global_limit.set_required_ordering(required_ordering);

How it is consumed. limit_pushdown turns it straight into preserve_order (limit_pushdown.rs:420,428):

preserve_order: global_limit.required_ordering().is_some(),

which reaches FileScanConfig.preserve_order, whose own docs say files must be read in exact order "to produce correct results (e.g. for ORDER BY ... LIMIT)".

Why it doesn't bite in the common case. FileScanConfigBuilder::build() re-derives the flag (file_scan_config/mod.rs:548):

let preserve_order = preserve_order || !output_ordering.is_empty();

and output_ordering is serialized — so a plan whose limit has already been pushed into the scan self-heals on decode.

Where the hole is. DataSource::with_preserve_order bypasses that builder entirely (file_scan_config/mod.rs:1153):

let new_config = FileScanConfig {
    preserve_order,
    ..self.clone()
};

so a false written through this path actively clears the flag rather than being re-derived. Decode a plan that still carries the limit node, re-run the optimizer, and required_ordering is now None → preserve_order: false → the scan is free to read files out of order. ORDER BY a LIMIT 10 can then return the wrong ten rows.

To Reproduce

No reproducer yet — this is a code-path argument, not a demonstrated failure. It needs a consumer that (a) serializes a plan while the limit node is still present, and (b) re-runs the physical optimizer after decoding. That is a normal shape for engines that re-plan per stage, so it seems worth closing rather than waiting for someone to hit it.

Expected behavior

required_ordering should survive a round-trip: add it to GlobalLimitExecNode / LocalLimitExecNode (it is a LexOrdering, and sort expressions already have a proto representation), or otherwise guarantee order-sensitivity cannot be silently lost.

Additional context

Lower-severity sibling, same class: SortPreservingMergeExec::enable_round_robin_repartition (sort_preserving_merge.rs:101) is also not on the wire, and decode restores the true default. Per its own docs false selects a stable merge, so the lossy direction can change which of several tied rows is emitted — not purely a performance knob. Nothing in-tree sets it to false today (only tests and benches), so priority is low, but "results differ only among tied keys, and only after a round-trip" is an unpleasant thing to debug if a downstream consumer starts using it.

Both fields were surfaced by the destructuring refactor in #24167, which documents them in comments rather than changing behaviour — fixing either means a wire-format addition, which didn't belong in a no-op refactor.

Related: #23494 (serde migration EPIC), #24165 (HashJoinExec::fetch, the same class of silently-dropped field), #24171 (test coverage gaps that let this class survive), #24170 (unchecked as usize in the same decode paths).