ESQL:DS: Align declared text-binding contract docs - #157589
Conversation
Three documentation surfaces described a positional binding contract for declared (strict) text schemas that the reader had not implemented since contract: - Rewrites DatasetFieldMapping's binding paragraph: declared text columns bind by header name, not positionally; headerless files self-bind via colN names; a missing declared name reads null with a warning. - Renames declaredPathBinding → declaredProvenanceBinding throughout (private field, SPI method withDeclaredPathBinding → withDeclaredProvenanceBinding, constructor param, all call sites and overrides) to eliminate the misleading suggestion that path presence — rather than SchemaProvenance.DECLARED — drives the flag. - Rewrites validateDeclaredHeaderBinding's javadoc to accurately describe its two branches: by-name dispatch for declared schemas; width tripwire for pinned inferred schemas. - Fixes the tripwire error message from "declared schema binds text columns in order" to "pinned schema has N columns … file may have drifted", since the tripwire fires only for pinned inferred schemas (declared schemas bypass it via the by-name path). - Updates the tripwire unit test comments and assertion strings to match.
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Thesis checks out — flag's set from whether the user declared the schema, not from 1. The new 2. Old wording still around: 3. 4. 5. Error message says it twice ("has only 2" / "fewer columns than the pinned schema"), and still suggests NDJSON's missing warning I'd file separately. Corner cases all check out, and |
quackaplop
left a comment
There was a problem hiding this comment.
Requesting changes on the five points in #157589 (comment) — mainly (1), since the new DatasetFieldMapping line claims a warning that only CSV/TSV emit.
quackaplop
left a comment
There was a problem hiding this comment.
All five addressed, thanks. Nice catch on the "headered path-bound read" error string, I'd missed that one.
One loose end, not worth another round: in that paragraph "text" means CSV/TSV, so NDJSON now falls between the two clauses rather than being described either way. Went from wrong to unmentioned, which is fine by me — worth a word if you touch it again.
Three documentation surfaces described a positional binding contract for declared (strict) text schemas that the reader had not implemented since contract:
Closes https://github.com/elastic/esql-planning/issues/1740