You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by moshebla <gi...@git.apache.org> on 2018/05/23 15:25:17 UTC
[GitHub] lucene-solr pull request #382: SOLR-12361
GitHub user moshebla opened a pull request:
https://github.com/apache/lucene-solr/pull/382
SOLR-12361
pass tests but the TestChildDocTransformer.
We have to think of a how we want to change the transformer, which should probably be discussed in another issue.
Currently it does not add the _childDocuments_ field to fl, causing the childDocuments to be ommited from the response.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/moshebla/lucene-solr SOLR-12361
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/382.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #382
----
commit 9a55af35e320e0931b7b670b46828d68177a9075
Author: user <us...@...>
Date: 2018-05-23T14:55:36Z
pass tests but TestChildDocTransformer
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #382: WIP: SOLR-12361
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/382#discussion_r190457184
--- Diff: solr/solrj/src/java/org/apache/solr/common/SolrDocument.java ---
@@ -388,20 +386,47 @@ public void addChildDocuments(Collection<SolrDocument> children) {
}
}
+ @Override
+ public Map<String, Object> getChildDocumentsMap() {
+ Map<String, Object> childDocs = new HashMap<>();
+ for (Entry<String, Object> entry: _fields.entrySet()) {
+ Object value = entry.getValue();
+ if(objIsDocument(value)) {
+ childDocs.put(entry.getKey(), value);
+ }
+ }
+ return childDocs;
+ }
+
/** Returns the list of child documents, or null if none. */
@Override
public List<SolrDocument> getChildDocuments() {
- return _childDocuments;
+ List<SolrDocument> childDocs = new ArrayList<>();
+ Stream<AbstractMap.SimpleEntry<String, SolrDocument>> fields = _fields.entrySet().stream()
+ .filter(value -> value.getValue() instanceof SolrInputDocument)
+ .map(value -> new AbstractMap.SimpleEntry<>(value.getKey(), (SolrDocument) value.getValue()));
+ fields.forEach(e -> childDocs.add(e.getValue()));
+ return childDocs.size() > 0 ? childDocs: null;
}
@Override
public boolean hasChildDocuments() {
--- End diff --
We'd probably deprecate these if we go with this overall approach. It's too much internal cost to call this method.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #382: WIP: SOLR-12361
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/382#discussion_r190386655
--- Diff: solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java ---
@@ -376,20 +376,6 @@ public void writeSolrDocument(String name, SolrDocument doc, ReturnFields return
writeVal(fname, val);
}
}
-
--- End diff --
Good; this can be repeated in GeoJSONResponseWriter too, I think.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #382: WIP: SOLR-12361
Posted by michaelbraun <gi...@git.apache.org>.
Github user michaelbraun commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/382#discussion_r190521566
--- Diff: solr/solrj/src/java/org/apache/solr/common/SolrDocument.java ---
@@ -388,20 +386,47 @@ public void addChildDocuments(Collection<SolrDocument> children) {
}
}
+ @Override
+ public Map<String, Object> getChildDocumentsMap() {
+ Map<String, Object> childDocs = new HashMap<>();
+ for (Entry<String, Object> entry: _fields.entrySet()) {
+ Object value = entry.getValue();
+ if(objIsDocument(value)) {
+ childDocs.put(entry.getKey(), value);
+ }
+ }
+ return childDocs;
+ }
+
/** Returns the list of child documents, or null if none. */
@Override
public List<SolrDocument> getChildDocuments() {
- return _childDocuments;
+ List<SolrDocument> childDocs = new ArrayList<>();
+ Stream<AbstractMap.SimpleEntry<String, SolrDocument>> fields = _fields.entrySet().stream()
+ .filter(value -> value.getValue() instanceof SolrInputDocument)
--- End diff --
Instead of creating an outer List<SolrDocument> childDocs = new ArrayList<>() up front, can this look like List<...> fields = ......collect(Collectors.toList()), with the size check at the end?
Also would be nice if this API could be changed to always return non-null, and if it's empty, it's just empty.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #382: WIP: SOLR-12361
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/382
new pull request #385
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #382: WIP: SOLR-12361
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/382#discussion_r190477830
--- Diff: solr/solrj/src/java/org/apache/solr/common/SolrDocument.java ---
@@ -388,20 +386,47 @@ public void addChildDocuments(Collection<SolrDocument> children) {
}
}
+ @Override
+ public Map<String, Object> getChildDocumentsMap() {
+ Map<String, Object> childDocs = new HashMap<>();
+ for (Entry<String, Object> entry: _fields.entrySet()) {
+ Object value = entry.getValue();
+ if(objIsDocument(value)) {
+ childDocs.put(entry.getKey(), value);
+ }
+ }
+ return childDocs;
+ }
+
/** Returns the list of child documents, or null if none. */
@Override
public List<SolrDocument> getChildDocuments() {
- return _childDocuments;
+ List<SolrDocument> childDocs = new ArrayList<>();
+ Stream<AbstractMap.SimpleEntry<String, SolrDocument>> fields = _fields.entrySet().stream()
+ .filter(value -> value.getValue() instanceof SolrInputDocument)
--- End diff --
yeah you're right, I'll add another commit to fix this
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #382: WIP: SOLR-12361
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/382#discussion_r190546131
--- Diff: solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java ---
@@ -376,20 +376,6 @@ public void writeSolrDocument(String name, SolrDocument doc, ReturnFields return
writeVal(fname, val);
}
}
-
--- End diff --
Removed in GeoJSONResponseWriter too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #382: WIP: SOLR-12361
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla closed the pull request at:
https://github.com/apache/lucene-solr/pull/382
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #382: WIP: SOLR-12361
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/382#discussion_r190457103
--- Diff: solr/solrj/src/java/org/apache/solr/common/SolrDocument.java ---
@@ -388,20 +386,47 @@ public void addChildDocuments(Collection<SolrDocument> children) {
}
}
+ @Override
+ public Map<String, Object> getChildDocumentsMap() {
+ Map<String, Object> childDocs = new HashMap<>();
+ for (Entry<String, Object> entry: _fields.entrySet()) {
+ Object value = entry.getValue();
+ if(objIsDocument(value)) {
+ childDocs.put(entry.getKey(), value);
+ }
+ }
+ return childDocs;
+ }
+
/** Returns the list of child documents, or null if none. */
@Override
public List<SolrDocument> getChildDocuments() {
- return _childDocuments;
+ List<SolrDocument> childDocs = new ArrayList<>();
+ Stream<AbstractMap.SimpleEntry<String, SolrDocument>> fields = _fields.entrySet().stream()
+ .filter(value -> value.getValue() instanceof SolrInputDocument)
--- End diff --
Or the value might be a List of SolrInputDocument, so we have to check for that too; right?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #382: WIP: SOLR-12361
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/382
As discussed in the JIRA issue, perhaps you should update the patch to try and leave the existing anonymous children in place and merely add semantic values via field values? (or start a new PR if you wish). Maybe modify AddBlockUpdateTest to add a test that works with SolrInputDocument directly to add child docs as field values, and index it, and test? You can use EmbeddedSolrServer wrapping h.getCore()) to work with the test harness using SolrJ (SolrInputDocument), thus avoiding the XML/JSON incoming which we have not yet added semantic key support to in formats.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org