Repository navigation
Conversation
|
In general this looks good, but I do have some concerns over how defaults are handled during overrides. For the column type specifically we would have retarget Something akin to this is happening in renaming because a default value is deduced from the content ( will be a no-op. Probably an edge case but super surprising if encountered. The better approach is for both structs to encode the content as an |
|
For this piece: I'd rather require For this piece: Yeah I agree |
|
hmm... I think I disagree... The default is on the column, not on the query. So However, if we have already given the column a type from a prior FORMAT clause then the column will respond with that This is not how it is directly implemented, I know, but that is how I think about it |
A FORMAT clause that never names a wildcard template now leaves the
field None instead of storing the default "{}", so merging stacked
FORMAT clauses on one column only overrides a template a clause
actually set. parse_renaming_clause returns Option<String> for the
template to keep an explicit * => '{}' distinguishable from no
wildcard, letting a later clause reset an earlier template.
A FORMAT clause without a target identifier (or with the new ANY keyword) now makes no claim about which table section its columns belong to: Format.target is Option<ColumnSection>, merging stacked clauses lets concrete BODY/STUB targets win over ANY, and unclaimed columns resolve to the body in setup_formats. Previously a bare FORMAT counted as a BODY claim and would silently pull columns out of the stub when combined with FORMAT STUB.
|
The solution I've taken is to have the |
FORMAT * sets a wildcard selection, expanded to the data's columns in setup_formats via the new expand_format_columns helper (which also absorbs SPAN id expansion and unknown-column validation). Wildcard clauses merge like any other, so an explicit clause can still refine a single column after a catch-all FORMAT *.
|
I'm now also allowing |
This PR implements suggestion here: #558 (comment)
@thomasp85 if you want this now rather than later, feel free to merge