LuciferYang commented on code in PR #37843:
URL: https://github.com/apache/spark/pull/37843#discussion_r967942983
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/Expression.java:
##########
@@ -44,7 +44,16 @@ public interface Expression {
* List of fields or columns that are referenced by this expression.
*/
default NamedReference[] references() {
- return Arrays.stream(children()).map(e -> e.references())
- .flatMap(Arrays::stream).distinct().toArray(NamedReference[]::new);
+ List<NamedReference> list = new ArrayList<>();
+ Set<NamedReference> uniqueValues = new HashSet<>();
+ for (Expression e : children()) {
+ NamedReference[] references = e.references();
+ for (NamedReference reference : references) {
+ if (uniqueValues.add(reference)) {
+ list.add(reference);
+ }
+ }
+ }
+ return list.toArray(new NamedReference[0]);
Review Comment:
The test results of GA are as follows:
- Java 8 : `ArrayList + HashSet` is 5 ~ 10% faster than `LinkedHashSet `
- Java 11: `ArrayList + HashSet` is 20 ~ 40% faster than `LinkedHashSet `
- Java 17: `ArrayList + HashSet` is 5 ~ 10% **slower** than `LinkedHashSet
`
For this reason, I added a comment, when Java 17 is default, we can consider
change to using `LinkedHashSet`, Is that ok?
Or should we change to
```
if (SystemUtils.isJavaVersionAtLeast("17")) {
use LinkedHashSet
} else {
use ArrayList + HashSet
}
```
?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
