Skip to content
New issue

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

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

Already on GitHub? # to your account

Add support for CopyTo::partition_by field in datafusion_proto #9248

Closed
alamb opened this issue Feb 16, 2024 · 4 comments · Fixed by #9306
Closed

Add support for CopyTo::partition_by field in datafusion_proto #9248

alamb opened this issue Feb 16, 2024 · 4 comments · Fixed by #9306
Assignees
Labels
good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Feb 16, 2024

After #9240 datafusion protobuf will will silently ignore partition_by options;

The task is

  1. add code to serialize / deserialize the CopyTo::partition_by field, added in Support Copy To Partitioned Files #9240 to the existing serialization code (similarly to how copy_to is done) https://github.com/apache/arrow-datafusion/blob/f1dcbf978210be6aa69b07892cf47321905aeece/datafusion/proto/src/logical_plan/mod.rs#L850

  2. Update the test in datafusion/proto/tests/cases/roundtrip_logical_plan.rs to send a non empty partition column list

Originally posted by @devinjdangelo in #9240 (comment)

@alamb alamb changed the title Note that I did not add support for partition_by in proto. We should add a follow up ticket for this. Add support for CopyTo::partition_by field in datafusion_proto Feb 16, 2024
@alamb
Copy link
Contributor Author

alamb commented Feb 16, 2024

Once #9240 is merged, I think this would be a good first issue as it will be small and self contained

@alamb alamb added the good first issue Good for newcomers label Feb 16, 2024
@PhVHoang
Copy link
Contributor

I can work on this one.

@alamb
Copy link
Contributor Author

alamb commented Feb 19, 2024

I can work on this one.

Thanks @PhVHoang 🙏

#9240 was just merged so I think this one is now actionable

@PhVHoang
Copy link
Contributor

take

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants