ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL

[lidavidm]

Also, enable Flight SQL in Windows CI builds.

[github-actions]

https://issues.apache.org/jira/browse/ARROW-16902

[github-actions]

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

[kou]

How about creating ARROW_FLIGHT_SQL_EXPORT instead of reusing ARROW_FLIGHT_EXPORT because Flight and Flight SQL provide separated DLLs?

If we reuse ARROW_FLIGHT_EXPORTING, Flight SQL uses __declspec(dllexport) not __declspec(dllimport) to use Flight API.

[lidavidm]

Ah, thanks Kou, you're right - I'll fix this up.

[lidavidm]

Protobuf does not interact very well with dllexport declarations (seemingly on purpose/the team considers it a bad idea: https://groups.google.com/g/protobuf/c/PDR1bqRazts) so hopefully this works to get Flight SQL on Windows (MinGW/MSVC).

The root of the issue here is that client_test.cc uses the Protobuf classes directly, which then means we have to deal with getting dllexport/dllimport annotated in all the right places in the generated Protobuf sources (and the compiler apparently does not do this fully correctly). The tests can't really be rewritten to not use the Protobuf classes directly, but given those tests rely exclusively on mocks, I wonder if they're even worth keeping. CC @jduo

[lidavidm]

MinGW works now; something about MSVC is not working. I can reproduce the issue, but not sure why it's happening yet. (The suggestions of adding -DGMOCK_LINKED_AS_SHARED_LIBRARY lead to multiple symbol definition issues.)

[jduo]

Protobuf does not interact very well with dllexport declarations (seemingly on purpose/the team considers it a bad idea: https://groups.google.com/g/protobuf/c/PDR1bqRazts) so hopefully this works to get Flight SQL on Windows (MinGW/MSVC).

The root of the issue here is that client_test.cc uses the Protobuf classes directly, which then means we have to deal with getting dllexport/dllimport annotated in all the right places in the generated Protobuf sources (and the compiler apparently does not do this fully correctly). The tests can't really be rewritten to not use the Protobuf classes directly, but given those tests rely exclusively on mocks, I wonder if they're even worth keeping. CC @jduo

I'm OK with removing these tests to deal with this problem.

[lidavidm]

I made the client_tests run only on Unix to just sidestep the problem.

[lidavidm]

Looks like this builds successfully on Windows now.

[kou]

+1