gimgit commented on PR #16827: URL: https://github.com/apache/iceberg/pull/16827#issuecomment-5354594882
Nice writeup — the neighbour-locality argument over Z-order is well made, and reusing the existing Z-order byte encodings keeps the surface small. One design question from adjacent work (#17504, overlap-based selection): 1. **Re-selection after clustering degrades.** The Hilbert runner plugs into the same size-based planner as sort/zorder, so once a table is Hilbert-clustered and its files sit in the healthy size band, newly ingested files that overlap in curve space are invisible to selection — `rewrite_data_files` reports `rewritten=0` as success and clustering quietly degrades (#17489 shows this for linear sort). For linear orders, bounds on the first sort field give a cheap detection signal (#17504). For Hilbert, per-column bounds get wide by construction, so I don't see an obvious metadata-only signal for "this table needs re-clustering". Does the companion doc have thoughts on detecting when a Hilbert rewrite is due, as opposed to how to perform it? Even a rough curve-space health metric would make the strategy operable long-term, not just at initial layout. 2. Small one: `SparkHilbertUDF` reuses a `ThreadLocal<byte[][]>` holder and re-initializes it in `readObject` — same pattern as the Z-order UDF, looks right. Did the JMH run compare against Z-order interleaving on the same inputs? A one-line relative number in the PR body would help reviewers weigh the added curve math. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
