weiqingy commented on code in PR #997:
URL: https://github.com/apache/flink-agents/pull/997#discussion_r3780489730
##########
integrations/vector-stores/elasticsearch/src/main/java/org/apache/flink/agents/integrations/vectorstores/elasticsearch/ElasticsearchVectorStore.java:
##########
@@ -336,23 +336,29 @@ public Map<String, Object> getStoreKwargs() {
/**
* Retrieve documents from the vector store.
*
- * <p>If ids is not provided, this method will retrieve documents
according to {@code limit},
- * {@code offset}, and {@code filter_query} in additional arguments. If
{@code limit} is null,
- * up to {@link ElasticsearchVectorStore#MAX_RESULT_WINDOW} documents are
returned (an
- * Elasticsearch ceiling).
+ * <p>When {@code ids} is non-empty, documents are retrieved directly by
ID and the filter,
+ * limit, offset, and {@code filter_query} arguments are not applied.
*
- * <p>The unified {@code filters} DSL parameter is not yet translated to
Elasticsearch's native
- * query DSL — callers needing structured filtering should pass a raw
{@code filter_query} via
- * {@code extraArgs}. TODO: implement equality-DSL translation parallel to
the Python Chroma
- * implementation.
+ * <p>Otherwise, {@code filters} provides equality-only matching against
document metadata. Each
+ * entry is translated to an Elasticsearch {@code term} query on {@code
+ * <metadataField>.<key>.keyword}, and multiple entries are combined with
AND semantics. Because
+ * Elasticsearch dynamic mapping creates `.keyword` sub-fields only for
strings, filters on
Review Comment:
Thanks for addressing comments.
nit: The new sentences use markdown backticks for `` `.keyword` `` and ``
`filter_query` ``, but Javadoc has no markdown, so those render as literal
backtick characters in the generated docs. The file uses `{@code ...}` 71 times
elsewhere, and `main` has no backticks at all.
The `queryEmbedding` copy shows the split within one sentence: `{@code
filter_query}` at 586 sits next to `` `.keyword` `` at 585.
Would you switch the five occurrences (345, 346, 396, 397, 585) over to
`{@code ...}`?
##########
integrations/vector-stores/elasticsearch/src/main/java/org/apache/flink/agents/integrations/vectorstores/elasticsearch/ElasticsearchVectorStore.java:
##########
@@ -546,7 +554,7 @@ private void deleteDocuments(
if (combined != null) {
builder.query(q -> q.withJson(new StringReader(combined)));
} else {
- // No filter at all → delete every document (match_all).
+ // No filter at all 闂?delete every document (match_all).
Review Comment:
At head this line reads:
```
// No filter at all 闂?delete every document (match_all).
```
On `main` it is `// No filter at all → delete every document (match_all).`
The arrow has turned into `闂` followed by a literal `?` (bytes `e9 97 82 3f`).
Line 736 has the same corruption, where an em dash became the identical
sequence: `Double 闂?null`.
I bisected it to `41c78db8`, so it isn't fallout from the KNN commits.
Neither line is documentation, and both now show up as extra hunks. CI is green
on this, so nothing catches it automatically.
Since restoring them would also bring the diff back to just the three
Javadoc hunks, could you put both lines back to what `main` has?
--
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]
