Skip to content

fix: make values method behave like keys method #2967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
robfrank merged 1 commit into from
Dec 17, 2025
Merged

fix: make values method behave like keys method #2967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions engine/src/main/java/com/arcadedb/query/sql/method/collection/SQLMethodValues.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.arcadedb.query.sql.executor.Result;
import com.arcadedb.query.sql.method.AbstractSQLMethod;

import java.util.Collection;
import java.util.List;
import java.util.Map;

Expand All @@ -50,6 +51,11 @@ else if (value instanceof Document document)
return List.of(document.toMap().values());
else if (value instanceof Result result)
return result.toMap().values();
else if (value instanceof Collection<?> collection) {
final List<Object> result = collection.stream()
.flatMap(o -> ((Collection<Object>) execute(o, currentRecord, context, params)).stream()).toList();
Comment on lines +55 to +56
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 16, 2025

Choose a reason for hiding this comment

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

critical

This implementation has two critical issues:

  1. NullPointerException: If an element in the collection does not match any of the handled types, execute() returns null, causing an NPE when .stream() is called.
  2. Incorrect result for Documents: For a Document, execute() returns a List<Collection<Object>>. The current code will throw a ClassCastException. The suggested fix below resolves the NPE and cast issue, but it will still produce a list of collections for Document inputs, not a flat list of values. This is due to an existing issue in how Document values are retrieved (List.of(document.toMap().values())). You should consider fixing that or adapting this logic to correctly flatten the results.
      final List<Object> result = collection.stream()
          .map(o -> execute(o, currentRecord, context, params))
          .filter(Collection.class::isInstance)
          .flatMap(c -> ((Collection<?>) c).stream()).toList();

return result;
}

return null;
}
Expand Down
14 changes: 14 additions & 0 deletions engine/src/test/java/com/arcadedb/query/sql/method/collection/SQLMethodValuesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,18 @@ void withResult() {
final Object result = function.execute(resultInternal, null, null, null);
assertThat(new ArrayList<>((Collection<String>) result)).isEqualTo(Arrays.asList("Foo", "Bar"));
}

@Test
void withCollection() {
List<Map<String, Object>> collection = List.of(Map.of("key1", "value1"), Map.of("key2", "value2"));

Object result = function.execute(collection, null, null, null);
assertThat(result).isEqualTo(List.of("value1", "value2"));
}

@Test
void withNull() {
Object result = function.execute(null, null, null, null);
assertThat(result).isNull();
}
Comment on lines +49 to +61
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 16, 2025

Choose a reason for hiding this comment

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

medium

The new tests are a good start, but they don't cover all scenarios. The current implementation has issues with certain inputs that are not covered by these tests. Please consider adding more test cases to ensure robustness:

  • A test with a collection of Document objects to verify correct flattening of values.
  • A test with a collection containing mixed or unsupported types (e.g., List.of(Map.of("k", "v"), "some string")) to ensure NullPointerException is not thrown.
  • A test with a collection of Result objects.

}
Loading