Skip to content

Conversation

@WenyXu
Copy link
Member

@WenyXu WenyXu commented Apr 14, 2023

Which issue does this PR close?

Rationale for this change

make JsonOpener and CsvOpener public

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the core Core DataFusion crate label Apr 14, 2023
@WenyXu WenyXu force-pushed the chore/json-open-and-csv-opener branch from 697a16f to 19e13c2 Compare April 14, 2023 04:52
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me -- @WenyXu in order to avoid future regressions, I recommend adding an example of using CsvOpener and JsonOpener into https://github.com/apache/arrow-datafusion/blob/main/datafusion-examples/examples/

Without an example / end user test, it is quite easy for this kind of change to get accidentally broke (for example if we move the structs around). I did something similar with the simplification API when we got broken downstream a few times, and have been quite happy with the result

https://github.com/apache/arrow-datafusion/blob/main/datafusion-examples/examples/expr_api.rs

Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good 👍 I've checked the examples and they also look good to me.

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
@waynexia waynexia merged commit 56bceb8 into apache:main Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants