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

Multi-language support issues with Arrow FlightSQL client's execute_update and execute_ingest methods #6545

Open
niebayes opened this issue Oct 11, 2024 · 2 comments
Labels

Comments

@niebayes
Copy link

niebayes commented Oct 11, 2024

Describe the bug

We designed an Arrow FlightSQL server which implements do_put_statement_update and do_put_statement_ingest. This server works well with the Arrow FlightSQL client of Rust. However, it does not work as expected when interacting with the clients of Go and Python. Specifically, the returned affected rows of the ExecuteUpdate and ExecuteIngest methods is always 0, despite the SQL is executed successfully on the server.

You can find our Go demo codes at https://github.com/niebayes/examples/blob/chore/add_execute_update_examples/go/main.go

You can find our Python demo codes at https://github.com/niebayes/examples/blob/chore/add_execute_update_examples/python/main.py
Note, we utilize the flightsql-dbapi library provided by InfluxDB in the Python demo, since Arrow does not provide a native implementation of Arrow FlightSQL yet.

And the Rust demo codes https://github.com/niebayes/examples/blob/chore/add_execute_update_examples/rust/bin/main.rs

To Reproduce

Since our database project is private for now, we cannot provide ways to reproduce the error. But honestly I think this error does not involve the server-side implementation and is solely affected by the client-side deserialization of the DoPutResult.

Expected behavior

We expect the Arrow FlightSQL clients of Go and Python could decode the affected rows correctly, just like the Rust client does.

Additional context

@niebayes niebayes added the bug label Oct 11, 2024
@tustvold
Copy link
Contributor

tustvold commented Oct 11, 2024

Tagging @djanderson who added these in #6201

This also sounds a lot like #5731

@djanderson
Copy link
Contributor

djanderson commented Oct 11, 2024

Thanks for tagging me, and good timing, I am just wrapping up a concrete implementation of this as well and I have am able to test it with the influx flightsql python client. IIRC I reused an existing codepath that serialized the DoPutResult because it appeared consistent with the spec for do_put_statement_ingest, but #5731 indicates that was not a safe assumption.

I'll try and reproduce this today and get back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants