leonfin commented on code in PR #7930:
URL: https://github.com/apache/geode/pull/7930#discussion_r2363379022
##########
geode-core/src/main/java/org/apache/geode/cache/query/internal/QCompiler.java:
##########
@@ -148,9 +148,9 @@ public void compileOrderByClause(final int numOfChildren) {
}
public void compileGroupByClause(final int numOfChildren) {
- final List<CompiledPath> list = new ArrayList<>();
+ final List<CompiledValue> list = new ArrayList<>();
Review Comment:
just FYI with (3 different AI reviews and they all say the same)
`
- Complexity: Repeatedly inserting at index 0 on an ArrayList is O(n) per
insertion due to shifting elements. For n items, this becomes O(n^2).`
```java
public void compileGroupByClause(final int numOfChildren) {
final List<CompiledValue> list = new ArrayList<>();
for (int i = 0; i < numOfChildren; i++) {
list.add(TypeUtils.checkCast(pop(), CompiledValue.class));
}
// reverse to preserve original left-to-right order without O(n^2)
insert-at-zero
java.util.Collections.reverse(list);
push(list);
}
```
But this is just a 'group by' case so I don't think it matters since group
by can't be big at all to matter
##########
geode-core/src/main/java/org/apache/geode/cache/query/internal/QCompiler.java:
##########
@@ -148,9 +148,9 @@ public void compileOrderByClause(final int numOfChildren) {
}
public void compileGroupByClause(final int numOfChildren) {
- final List<CompiledPath> list = new ArrayList<>();
+ final List<CompiledValue> list = new ArrayList<>();
Review Comment:
just FYI with (3 different AI reviews and they all say the same)
- Complexity: Repeatedly inserting at index 0 on an ArrayList is O(n) per
insertion due to shifting elements. For n items, this becomes O(n^2).
```java
public void compileGroupByClause(final int numOfChildren) {
final List<CompiledValue> list = new ArrayList<>();
for (int i = 0; i < numOfChildren; i++) {
list.add(TypeUtils.checkCast(pop(), CompiledValue.class));
}
// reverse to preserve original left-to-right order without O(n^2)
insert-at-zero
java.util.Collections.reverse(list);
push(list);
}
```
But this is just a 'group by' case so I don't think it matters since group
by can't be big at all to matter
--
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]
