Skip to content

Implement do_put_statement_ingest handler in FlightSqlService #6137

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

Closed

Conversation

djanderson
Copy link
Contributor

Which issue does this PR close?

Closes #6124.

Depends on #6133.

Rationale for this change

This implements the handler for the new CommandStatementIngest message defined in Arrow Flight SQL version 17.0.

What changes are included in this PR?

The generated CommandStatementIngest from #6133 is exposed as a public member of the arrow_flight sql module and the FlightSqlService trait is updated with a handler for that new command.

Are there any user-facing changes?

Yes, the trait FlightSqlService has a new method, do_put_statement_ingest.

This PR should be tagged api-change.

@github-actions github-actions bot added arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate labels Jul 27, 2024
@djanderson djanderson changed the title Do put statement ingest Implement do_put_statement_ingest handler in FlightSqlService Jul 27, 2024
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.

Thank you @djanderson 🙏

I wonder if you could also add a test for this code to show all the plumbing is hooked up correctly

Perhaps you could follow the model in https://github.com/apache/arrow-rs/blob/master/arrow-flight/tests/flight_sql_client.rs (thanks to @lewiszlw for setting up that pattern)

@djanderson djanderson force-pushed the do-put-statement-ingest branch from ba60aa3 to acf331a Compare July 27, 2024 19:07
@djanderson djanderson force-pushed the do-put-statement-ingest branch from acf331a to b505037 Compare July 29, 2024 20:14
@alamb alamb marked this pull request as draft July 29, 2024 21:00
@alamb
Copy link
Contributor

alamb commented Jul 29, 2024

Marking as draft as I think this PR is no longer waiting on feedback. Please mark it as ready for review when it is ready for another look

@alamb
Copy link
Contributor

alamb commented Aug 1, 2024

BTW when you next get a chance to work on this PR, can you please change it so it targets the master branch rather than the 53.0.0-dev branch? We are now making changes on main for the next release

@alamb alamb deleted the branch apache:53.0.0-dev August 1, 2024 10:57
@alamb alamb closed this Aug 1, 2024
@djanderson
Copy link
Contributor Author

@alamb of course.

I'm working on it, I'll open a PR against master tomorrow.

@alamb
Copy link
Contributor

alamb commented Aug 2, 2024

It appears that github automatically closes the PR when the target branch is deleted -- sorry about that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants