-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Remove use of deprecated record_batches_to_json_rows
function from arrow-json
#24981
Comments
I am interested in taking on this task. Is it possible to assign this ticket to me? |
Hi @JeanArhancet - that would be awesome, thank you! This issue is not in our immediate backlog, so if you would like to tackle it by opening a PR, you are welcome to do that. My recommendation is to start by solving (1.) in the issue description, and opening a PR for only that. Since that is using As for (2.), the use of Please keep in mind that you will need to read and sign our CLA in order for contributions to be accepted. You can find that here: https://www.influxdata.com/legal/cla/ |
Awesome, thank you @JeanArhancet! I had some comments on that PR. Re: (2.), I will clarify a bit, in case you want to tackle that in a separate PR. The influxdb/influxdb3_server/src/http/v1.rs Lines 340 to 377 in a5eba2f
It converts the arrow The objective here would be to remove the use of This would likely involve a bit more refactoring effort than solving (1.) did, but if you're up for giving it a shot, then go for it! |
Closing since #25046 is merged |
Realizing there were more parts to be added so I've reopened my bad |
We are using a function from
arrow-json
crate that is deprecated:record_batches_to_json_rows
It is used in the following places:
influxdb/influxdb3_server/src/http.rs
Line 810 in a5eba2f
influxdb/influxdb3_server/src/http/v1.rs
Line 342 in a5eba2f
The deprecation warning indicates that we should be using
Writer
. Doing so in the latter case should be easier since we are serializing directly to the bytes outputted in the API response, however, the former may be a bit trickier, as we are modifying the serialized JSON values before serializing to bytes in the response.The text was updated successfully, but these errors were encountered: