Skip to content

ORC-183: Add a method in Type to build type#115

Closed
wgtmac wants to merge 3 commits intoapache:masterfrom
wgtmac:ORC-183
Closed

ORC-183: Add a method in Type to build type#115
wgtmac wants to merge 3 commits intoapache:masterfrom
wgtmac:ORC-183

Conversation

@wgtmac
Copy link
Member

@wgtmac wgtmac commented Apr 28, 2017

Added static Type* buildTypeFromString(const std::string& input)

Added static Type* buildTypeFromString(const std::string& input)
/**
* Build a Type object from string text representation.
*/
static Type* buildTypeFromString(const std::string& input);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make it a unique_ptr to make it clear that the caller owns the pointer.

return std::unique_ptr<Type>(result);
}

Type * Type::buildTypeFromString(const std::string& input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be factored in to smaller functions. Take a look at the equivalent java https://github.com/apache/orc/blob/master/java/core/src/java/org/apache/orc/TypeDescription.java#L396 and you'll see parseName, parseCategory, parseInteger, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense. I will do the refactoring soon.

@asfgit asfgit closed this in 90f138b May 10, 2017
@wgtmac wgtmac deleted the ORC-183 branch February 23, 2022 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants