-
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
GH-37651: [C#] expose ArrowArrayConcatenator.Concatenate #37652
Conversation
|
@lidavidm - can you help with this one? |
Looks like there's test failures/you may want to rebase or merge? |
Oh, I guess this has to be updated to remove arrow/csharp/test/Apache.Arrow.Tests/ArrowArrayConcatenatorTests.cs Lines 112 to 113 in b30d961
|
Or really: presumably reflection is no longer required for this test |
good catch. I will fix the test. |
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 0d89741. There was 1 benchmark result indicating a performance regression:
The full Conbench report has more details. |
…e#37652) ### Rationale for this change New C# drivers need the ability to concatenate arrays, particularly for metadata calls. ### What changes are included in this PR? Converts a previously internal class and method to a public class and method. ### Are these changes tested? No substantive product changes were made. All tests still pass. ### Are there any user-facing changes? It exposes previously hidden functionality. Resolves apache#37651 * Closes: apache#37651 Lead-authored-by: David Coe <[email protected]> Co-authored-by: davidhcoe <[email protected]> Signed-off-by: David Li <[email protected]>
…e#37652) ### Rationale for this change New C# drivers need the ability to concatenate arrays, particularly for metadata calls. ### What changes are included in this PR? Converts a previously internal class and method to a public class and method. ### Are these changes tested? No substantive product changes were made. All tests still pass. ### Are there any user-facing changes? It exposes previously hidden functionality. Resolves apache#37651 * Closes: apache#37651 Lead-authored-by: David Coe <[email protected]> Co-authored-by: davidhcoe <[email protected]> Signed-off-by: David Li <[email protected]>
…e#37652) ### Rationale for this change New C# drivers need the ability to concatenate arrays, particularly for metadata calls. ### What changes are included in this PR? Converts a previously internal class and method to a public class and method. ### Are these changes tested? No substantive product changes were made. All tests still pass. ### Are there any user-facing changes? It exposes previously hidden functionality. Resolves apache#37651 * Closes: apache#37651 Lead-authored-by: David Coe <[email protected]> Co-authored-by: davidhcoe <[email protected]> Signed-off-by: David Li <[email protected]>
Rationale for this change
New C# drivers need the ability to concatenate arrays, particularly for metadata calls.
What changes are included in this PR?
Converts a previously internal class and method to a public class and method.
Are these changes tested?
No substantive product changes were made. All tests still pass.
Are there any user-facing changes?
It exposes previously hidden functionality.
Resolves #37651