-
Notifications
You must be signed in to change notification settings - Fork 4k
[CPP] Add missing metadata on Arrow schemas returned by Flight SQL #11999 #12002
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
[CPP] Add missing metadata on Arrow schemas returned by Flight SQL #11999 #12002
Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename pull request title in the following format? or See also: |
c0d4c51 to
e6891f2
Compare
|
|
||
| /// \brief Constant variable to hold the value of the key that | ||
| /// will be used in the KeyValueMetadata class. | ||
| static const char* CATALOG_NAME; |
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.
nit: constants should be named kCatalogName according to the style guide
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.
Fixed
|
|
||
| #include <utility> | ||
|
|
||
| namespace { |
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.
nit: the anonymous namespace could probably be nested into the arrow::flight::sql namespace below
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.
Fixed
|
|
||
| /// \brief Return the catalog name set in the KeyValueMetadata. | ||
| /// \return The catalog name. | ||
| arrow::Result<std::string> GetCatalogName(); |
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.
Can these be declared const?
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.
Fixed
|
Closed this pull request because both implementation JAVA and C++ are now in the same Pull Request |
This add an auxiliary class FlightSqlColumnMetadata meant to read and write known metadata for Arrow schema fields, such as CATALOG_NAME, SCHEMA_NAME, TABLE_NAME, PRECISION, SCALE, IS_AUTO_INCREMENT, IS_CASE_SENSITIVE, IS_READ_ONLY and IS_SEARCHABLE.