wgtmac commented on code in PR #1427:
URL: https://github.com/apache/orc/pull/1427#discussion_r1127394807
##########
c++/include/orc/Type.hh:
##########
@@ -65,6 +65,7 @@ namespace orc {
virtual std::vector<std::string> getAttributeKeys() const = 0;
virtual std::string getAttributeValue(const std::string& key) const = 0;
virtual std::string toString() const = 0;
+ virtual const Type* getTypeByColumnId(uint64_t colId) const = 0;
Review Comment:
We have supported C++17 now. It can be replaced by std::optional. Or we need
to add a comment to say what will happen if the column does not exist.
##########
c++/include/orc/Type.hh:
##########
@@ -97,6 +98,13 @@ namespace orc {
static std::unique_ptr<Type> buildTypeFromString(const std::string& input);
};
+ struct EnumClassHash {
Review Comment:
Can we move it into `.cc` file?
##########
c++/src/SchemaEvolution.cc:
##########
@@ -0,0 +1,233 @@
+#include "SchemaEvolution.hh"
+#include <unordered_map>
+#include <unordered_set>
+#include "orc/Exceptions.hh"
+
+namespace orc {
+
+ SchemaEvolution::SchemaEvolution(const std::shared_ptr<Type>& _readType,
const Type* fileType)
+ : readType(_readType) {
+ if (readType) {
+ buildConversion(readType.get(), fileType);
+ } else {
+ for (uint64_t i = 0; i <= fileType->getMaximumColumnId(); ++i) {
+ safePPDConversionMap.insert(i);
+ }
+ }
+ }
+
+ const Type* SchemaEvolution::getReadType(const Type& fileType) const {
+ auto ret = readTypeMap.find(fileType.getColumnId());
+ return ret == readTypeMap.cend() ? &fileType : ret->second;
+ }
+
+ inline void invalidConversion(const Type* readType, const Type* fileType) {
+ throw SchemaEvolutionError("Cannot convert from " + fileType->toString() +
" to " +
+ readType->toString());
+ }
+
+ // map from file type to read type. it does not contain identity mapping.
+ using TypeSet = std::unordered_set<TypeKind, EnumClassHash>;
+ using ConvertMap = std::unordered_map<TypeKind, TypeSet, EnumClassHash>;
+
+ inline bool supportConversion(const Type& readType, const Type& fileType) {
+ static const ConvertMap& SUPPORTED_CONVERSIONS = *new ConvertMap{
+ // support nothing now
+ };
+ auto iter = SUPPORTED_CONVERSIONS.find(fileType.getKind());
+ if (iter == SUPPORTED_CONVERSIONS.cend()) {
+ return false;
+ }
+ return iter->second.find(readType.getKind()) != iter->second.cend();
+ }
+
+ // return value: <valid convert, need convert>
Review Comment:
Can we use a struct for better readability?
##########
c++/src/SchemaEvolution.cc:
##########
@@ -0,0 +1,233 @@
+#include "SchemaEvolution.hh"
+#include <unordered_map>
+#include <unordered_set>
+#include "orc/Exceptions.hh"
Review Comment:
```suggestion
#include "SchemaEvolution.hh"
#include "orc/Exceptions.hh"
#include <unordered_map>
#include <unordered_set>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
