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

ARROW-17254: [C++][Go][Java][FlightRPC] Implement and test Flight SQL GetSchema #13898

Merged
merged 2 commits into from
Aug 18, 2022

Conversation

lidavidm
Copy link
Member

Consistently implements and tests the GetSchema method in Flight SQL.

Builds on #13897.

@lidavidm
Copy link
Member Author

I expect Go will fail, need to fix that…

@lidavidm lidavidm changed the title [C++][Java][FlightRPC] Implement and test Flight SQL GetSchema ARROW-17254: [C++][Java][FlightRPC] Implement and test Flight SQL GetSchema Aug 16, 2022
@lidavidm lidavidm changed the title ARROW-17254: [C++][Java][FlightRPC] Implement and test Flight SQL GetSchema ARROW-17254: [C++][Go][Java][FlightRPC] Implement and test Flight SQL GetSchema Aug 16, 2022
@github-actions
Copy link

@lidavidm
Copy link
Member Author

@zeroshade I implemented the GetSchema methods for Go here as well (the original PRs didn't implement them fully, so this PR is to rectify that)

@jduo I noticed GetSchema is in the protos but not the implementations; this rectifies that (I expect these methods won't see much use, but might as well)

@lidavidm lidavidm marked this pull request as ready for review August 17, 2022 11:38
Copy link
Member

@zeroshade zeroshade left a comment

Choose a reason for hiding this comment

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

@lidavidm Thanks! I didn't remember seeing those GetSchema methods anywhere otherwise I would have implemented them the first time around haha.

The Go changes look good from my end, i'm glad you were able to pick it up easily :)

@lidavidm
Copy link
Member Author

Thankfully Go is straightforward!

And yeah, they're in the spec and in Java, but got omitted from C++ (whoops)

@lidavidm
Copy link
Member Author

CI failures are unrelated or already tracked, merging

@lidavidm lidavidm merged commit bc52f9f into apache:master Aug 18, 2022
@lidavidm lidavidm deleted the arrow-17254 branch August 18, 2022 16:04
@ursabot
Copy link

ursabot commented Aug 18, 2022

Benchmark runs are scheduled for baseline = 3615d7f and contender = bc52f9f. bc52f9f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.48% ⬆️0.2%] test-mac-arm
[Failed ⬇️1.1% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️1.57% ⬆️0.11%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] bc52f9f0 ec2-t3-xlarge-us-east-2
[Finished] bc52f9f0 test-mac-arm
[Failed] bc52f9f0 ursa-i9-9960x
[Finished] bc52f9f0 ursa-thinkcentre-m75q
[Finished] 3615d7f5 ec2-t3-xlarge-us-east-2
[Failed] 3615d7f5 test-mac-arm
[Failed] 3615d7f5 ursa-i9-9960x
[Finished] 3615d7f5 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot
Copy link

ursabot commented Aug 18, 2022

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
… GetSchema (apache#13898)

Consistently implements and tests the GetSchema method in Flight SQL.

Builds on apache#13897.

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants