You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/12/24 03:58:04 UTC
[GitHub] [orc] belugabehr opened a new pull request #977: ORC-1064: Rework Attribute Map in TypeDescription class
belugabehr opened a new pull request #977:
URL: https://github.com/apache/orc/pull/977
### What changes were proposed in this pull request?
Simplify interactions with Map in TypeDescription class
### Why are the changes needed?
Reduce code complexity
### How was this patch tested?
No functional changes. Existing unit tests ran.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] belugabehr commented on a change in pull request #977: ORC-1064: Rework Attribute Map in TypeDescription class
Posted by GitBox <gi...@apache.org>.
belugabehr commented on a change in pull request #977:
URL: https://github.com/apache/orc/pull/977#discussion_r774861777
##########
File path: java/core/src/java/org/apache/orc/TypeDescription.java
##########
@@ -372,9 +372,7 @@ public TypeDescription clone() {
result.children.add(clone);
}
}
- for (Map.Entry<String,String> pair: attributes.entrySet()) {
- result.attributes.put(pair.getKey(), pair.getValue());
- }
+ result.attributes.putAll(attributes);
Review comment:
No need to iterate like this.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] belugabehr commented on a change in pull request #977: ORC-1064: Rework Attribute Map in TypeDescription class
Posted by GitBox <gi...@apache.org>.
belugabehr commented on a change in pull request #977:
URL: https://github.com/apache/orc/pull/977#discussion_r774861949
##########
File path: java/core/src/java/org/apache/orc/TypeDescription.java
##########
@@ -420,15 +418,9 @@ public boolean equals(Object other, boolean checkAttributes) {
}
if (checkAttributes) {
// make sure the attributes are the same
- List<String> attributeNames = getAttributeNames();
Review comment:
Calling `getAttributeNames()` method here actually causes an ArrayList instantiation, and a sort of that list, for ever invocation.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] belugabehr commented on a change in pull request #977: ORC-1064: Rework Attribute Map in TypeDescription class
Posted by GitBox <gi...@apache.org>.
belugabehr commented on a change in pull request #977:
URL: https://github.com/apache/orc/pull/977#discussion_r774861949
##########
File path: java/core/src/java/org/apache/orc/TypeDescription.java
##########
@@ -420,15 +418,9 @@ public boolean equals(Object other, boolean checkAttributes) {
}
if (checkAttributes) {
// make sure the attributes are the same
- List<String> attributeNames = getAttributeNames();
Review comment:
Calling `getAttributeNames()` method here actually causes an ArrayList instantiation, and a sort of that list, for every invocation.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] belugabehr commented on a change in pull request #977: ORC-1064: Rework Attribute Map in TypeDescription class
Posted by GitBox <gi...@apache.org>.
belugabehr commented on a change in pull request #977:
URL: https://github.com/apache/orc/pull/977#discussion_r774861842
##########
File path: java/core/src/java/org/apache/orc/TypeDescription.java
##########
@@ -420,15 +418,9 @@ public boolean equals(Object other, boolean checkAttributes) {
}
if (checkAttributes) {
// make sure the attributes are the same
- List<String> attributeNames = getAttributeNames();
- if (castOther.getAttributeNames().size() != attributeNames.size()) {
+ if (!this.attributes.equals(castOther.attributes)) {
Review comment:
The native `Map` equals method implements this already.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] belugabehr commented on a change in pull request #977: ORC-1064: Rework Attribute Map in TypeDescription class
Posted by GitBox <gi...@apache.org>.
belugabehr commented on a change in pull request #977:
URL: https://github.com/apache/orc/pull/977#discussion_r774862105
##########
File path: java/core/src/java/org/apache/orc/TypeDescription.java
##########
@@ -565,9 +557,7 @@ public int getScale() {
* @return a list of sorted attribute names
*/
public List<String> getAttributeNames() {
- List<String> result = new ArrayList<>(attributes.keySet());
- Collections.sort(result);
- return result;
+ return new ArrayList<>(attributes.keySet());
Review comment:
Keep the `attributes` Map sorted so it doesn't have to be sorted every time this method is invoked.
--
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: dev-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org