-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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-37440: [C#][Docs] Add Flight SQL supported functions to status.rst #37441
Conversation
|
docs/source/status.rst
Outdated
+--------------------------------------------+-------+-------+-------+------------+-------+-------+-------+-------+ | ||
|
||
Notes: | ||
|
||
* \(1) GetSqlInfo filtering is not currently supported in C#. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the "filtering" mean?
It seems that GetSqlInfo
doesn't have filtering feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that I was mixing up our implementation with the base support provided by Arrow Flight SQL. Our implementation doesn't allow a CommandGetSqlInfo
Info
field to be populated when the command is executed. I've seen implementations in other languages where the Info
field is used as a metadata filter list. However, the C# base implementation does not impose any restrictions.
I fixed the documentation. Thanks for the catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. :-)
The C# notes had an invalid comment about the Flight SQL functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 4612998. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them. |
…us.rst (apache#37441) ### Rationale for this change Update the feature support documentation to reflect that Flight SQL is now supported with C# ### What changes are included in this PR? The feature support matrix for Flight SQL now lists functions supported with C# ### Are these changes tested? N/A ### Are there any user-facing changes? Yes, the documentation. No Closes issue apache#37440 * Closes: apache#37440 Authored-by: jeremyosterhoudt <jeremyosterhoudt@yahoo.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
…us.rst (apache#37441) ### Rationale for this change Update the feature support documentation to reflect that Flight SQL is now supported with C# ### What changes are included in this PR? The feature support matrix for Flight SQL now lists functions supported with C# ### Are these changes tested? N/A ### Are there any user-facing changes? Yes, the documentation. No Closes issue apache#37440 * Closes: apache#37440 Authored-by: jeremyosterhoudt <jeremyosterhoudt@yahoo.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Rationale for this change
Update the feature support documentation to reflect that Flight SQL is now supported with C#
What changes are included in this PR?
The feature support matrix for Flight SQL now lists functions supported with C#
Are these changes tested?
N/A
Are there any user-facing changes?
Yes, the documentation.
No
Closes issue #37440