Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break out distribute_and_unnest_attrs #243

Merged
merged 4 commits into from Mar 21, 2022

Conversation

ijackson
Copy link
Contributor

RFC, as discussed in #241 (comment) . It seemed better to demonstrate it rather than talk in the abstract.

I think it has come out quite nicely. In particular, removing the duplicated specification of the output field variables now means it's not possible to accidentally get the unnesting and copying information out of step.

But if you prefer the code the way it is, please do feel free to say so and just close this MR.

Replaces two copies of the same code, in Field::unnest_attrs and
Options::unnest_attrs.

Also, replaces the double specification of the output attributes list
variables in each of these functions (once when calculating unnest,
and once when copying the fields to every output).

Discussion was here, et seq:
  colin-kiegel#241 (comment)
@TedDriggs TedDriggs merged commit 38610eb into colin-kiegel:master Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants