refactor: use REE field name constants across crates - #11064
akashchamp wants to merge 2 commits into
Conversation
Replace default run-end encoded field-name literals with the shared constants.
Rich-T-kid
left a comment
There was a problem hiding this comment.
seems straightforward, thank you @akashchamp
|
i think we should aim to rewire to more crates than just arrow-schema, like how maps was done |
Rewire the default run-end encoded child field names in the other workspace crates to Field::REE_RUN_ENDS_FIELD_DEFAULT_NAME and Field::REE_VALUES_FIELD_DEFAULT_NAME, following the approach used for the map field names in apache#10517: arrow-array (RunArray::try_new), arrow-data, arrow-cast, arrow-ord, arrow-row, arrow-ipc, arrow-json, arrow-avro, arrow-integration-test, arrow, parquet and parquet-variant-compute. Custom-name cases and unrelated "values" field names (dictionary values in arrow-flight, map value fallback in arrow-avro, plain column names in arrow-ipc tests) are intentionally left as literals. No behaviour change.
|
Good call, thanks for the pointer to #10517. I have now rewired the rest of the workspace the same way: arrow-array (RunArray::try_new), arrow-data, arrow-cast, arrow-ord, arrow-row, arrow-ipc, arrow-json, arrow-avro, arrow-integration-test, arrow, parquet and parquet-variant-compute all use Field::REE_RUN_ENDS_FIELD_DEFAULT_NAME / Field::REE_VALUES_FIELD_DEFAULT_NAME, so no default run_ends/values literals remain outside the constant definitions. arrow-select and arrow-flight had no REE name literals to change, and the remaining "values" literals are dictionary, map or ordinary column fields, not REE children, so I left them as they are. |
Rich-T-kid
left a comment
There was a problem hiding this comment.
this looks good, nice suggestion Jefffrey
Which issue does this PR close?
Rationale for this change
Field::REE_RUN_ENDS_FIELD_DEFAULT_NAMEandField::REE_VALUES_FIELD_DEFAULT_NAMEalready exist inarrow-schema, but the defaultrun_ends/valueschild names were still spelled out as string literals across the workspace. Using the constants everywhere keeps the default names in one place, in the same way #10517 did for the map field names.What changes are included in this PR?
Replace every default run-end encoded child field name literal with the shared constants, keeping behaviour identical:
arrow-schema: display, parse, ffi, fields and canonical extension code and testsarrow-array:RunArray::try_new(the default names everyRunArraygets) andnew_null_arraytestsarrow-data: run transform and validation testsarrow-cast,arrow-ord,arrow-row,arrow-select(no REE literals there),arrow-ipc,arrow-json,arrow-avro(codec, schema and writer round-trip test),arrow-integration-test(JSONrunendencodedplaceholder type),arrow(data_genand the cast benchmarks),parquet(arrow_writer bench) andparquet-variant-computetestsIntentional custom-name cases (for example the
named_valuesdisplay/parse tests) and unrelated"values"field names (dictionary values inarrow-flight, map value fallback inarrow-avro, plain column names inarrow-ipctests) are left as literals on purpose.Are these changes tested?
Covered by the existing tests; no behaviour changes. Run locally:
cargo fmt --all -- --checkcargo test --libforarrow-data,arrow-array,arrow-ord,arrow-row,arrow-ipc,arrow-json,arrow-cast,arrow-avro,arrow-integration-test,arrow(withtest_utils) andparquet-variant-computecargo checkfor the touchedarrow,arrow-jsonandparquetbenchmarkscargo clippy --all-targets --all-features -- -D warningson the touched cratesAre there any user-facing changes?
None.
AI assistance was used to identify and apply the mechanical substitutions across crates. I reviewed every changed construction and confirmed that custom-name cases and unrelated
"values"field names remain explicit literals.