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/08/30 07:24:58 UTC
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
GitHub user moshebla opened a pull request:
https://github.com/apache/lucene-solr/pull/443
SOLR-12722: add fl param to ChildDocTransformer
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/moshebla/lucene-solr SOLR-12722
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/443.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 #443
----
commit 0eb9cbc0084415706240cb3b81d14eac31d7a206
Author: Moshe <mo...@...>
Date: 2018-08-30T07:23:20Z
SOLR-12722: add fl param to ChildDocTransformer
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214044533
--- Diff: solr/solr-ref-guide/src/transforming-result-documents.adoc ---
@@ -137,6 +137,7 @@ When using this transformer, the `parentFilter` parameter must be specified, and
* `childFilter` - query to filter which child documents should be included, this can be particularly useful when you have multiple levels of hierarchical documents (default: all children)
* `limit` - the maximum number of child documents to be returned per parent document (default: 10)
+* `fl` - the field list which the transformer is to return.
--- End diff --
... "There is a further limitation in which the fields here should be a subset of those specified by the top level fl param." And say defaults to the top level "fl" ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #443: SOLR-12722: add fl param to ChildDocTransformer
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/443
Oh right; we'd get an infinite loop as the ChildDocTransformerFactory is already being called by SolrReturnFields! Ouch!
Sorry but I don't like your solution of trying to parse/strip the child transformer "fl"; it's very error-prone to do such things.
Lets define a ThreadLocal<Boolean> that allows us to know if we're being asked to create this transformer from within a recursive call to SolrReturnFields. When we see this happening, we can return a no-op transformer (see RawValueTransformerFactory.NoopFieldTransformer which could be made public and moved to an inner class of DocTransformer). This thread local would be set & cleared in a try-finally, and this logic would be in some utility method to keep it from being distracting to the caller. In general I avoid ThreadLocals as it's a singleton design pattern which is usually bad, but it can be used sparingly/judiciously.
BTW it appears that doc transformers will not be executed on the child docs. The transformer is not invoking them, and I suspect they won't be invoked on the way out to the client. Can you check/test if this is true? If I'm correct, this is a pre-existing bug/limitation that would be good to mark as a TODO; or you can try to address here since it's related (since you before couldn't specify what 'fl' was therefore it wasn't possible to specify a transformer until now).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214018618
--- Diff: solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java ---
@@ -297,6 +297,35 @@ public void testNoChildren() throws Exception {
"/response/docs/[0]/type_s==cake");
}
+ @Test
+ public void testChildReturnFields() throws Exception {
+ indexSampleData(numberOfDocsPerNestedTest);
+
+ assertJQ(req("q", "test_s:testing",
+ "sort", "id asc",
+ "fl", "*, [child childFilter='lonely/lonelyGrandChild/test2_s:secondTest' fl='*, _nest_path_']",
--- End diff --
Lets leave `_nest_path_` as internal and not mention it since testing "fl" has nothing to do with this field or nested documents. So instead use some other field, foo_s or whatever. And can you move this test to TestChildDocTransformer.java as it's not related to hierarchy?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #443: SOLR-12722: add fl param to ChildDocTransformer
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/443
Oops. There's a problem with this commit.
I'm on it!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214542262
--- Diff: solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java ---
@@ -114,4 +114,27 @@ public void transform(SolrDocument doc, int docid, float score) throws IOExcepti
public String toString() {
return getName();
}
+
+ /**
+ * Trivial Impl that ensure that the specified field is requested as an "extra" field,
+ * but then does nothing during the transformation phase.
+ */
+ public static final class NoopFieldTransformer extends DocTransformer {
--- End diff --
Oh woops; I recommended you do this but overlooked it's not *just* a no-op; it's doing this extra field requesting task. Well I suppose it's okay; not a big deal. Alternatively, this could changed to be *just* a no-op, and then the former impl in RawValueTransformerFactory.java could exist as a subclass that is a bit simpler.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214048597
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java ---
@@ -106,9 +107,12 @@ public DocTransformer create(String field, SolrParams params, SolrQueryRequest r
}
}
+ String childReturnFields = params.get("fl");
+ SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(): new SolrReturnFields(childReturnFields ,req);
--- End diff --
That makes sense,
just pushed a commit :).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214542721
--- Diff: solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java ---
@@ -1831,7 +1831,7 @@ public void testChildDoctransformer() throws IOException, SolrServerException {
q = new SolrQuery("q", "*:*", "indent", "true");
q.setFilterQueries(parentFilter);
- q.setFields("id,[child parentFilter=\"" + parentFilter +
+ q.setFields("id, level_i, [child parentFilter=\"" + parentFilter +
--- End diff --
I'm confused; this change suggests there is a backwards-compatibility change; is there? I think you said that the child "fl" is effectively constrained by whatever the top "fl" is; which if totally true means (I think) we needn't worry by defaulting to the top "fl". If this isn't totally correct (hence your change) then we need to be careful about changing the default behavior in a minor release; we'd have to tweak the default choice (absence of specific "fl") using the luceneMatchVersion.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214541721
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java ---
@@ -54,24 +54,26 @@
private final DocSet childDocSet;
private final int limit;
private final boolean isNestedSchema;
+ private final SolrReturnFields childReturnFields;
- //TODO ought to be provided/configurable
- private final SolrReturnFields childReturnFields = new SolrReturnFields();
-
- ChildDocTransformer(String name, BitSetProducer parentsFilter,
- DocSet childDocSet, boolean isNestedSchema, int limit) {
+ ChildDocTransformer(String name, BitSetProducer parentsFilter, DocSet childDocSet,
+ SolrReturnFields returnFields, boolean isNestedSchema, int limit) {
this.name = name;
this.parentsFilter = parentsFilter;
this.childDocSet = childDocSet;
this.limit = limit;
this.isNestedSchema = isNestedSchema;
+ this.childReturnFields = returnFields!=null? returnFields: new SolrReturnFields();
}
@Override
public String getName() {
return name;
}
+ @Override
+ public boolean needsSolrIndexSearcher() { return true; }
--- End diff --
+1 nice catch!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214041147
--- Diff: solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java ---
@@ -297,6 +297,35 @@ public void testNoChildren() throws Exception {
"/response/docs/[0]/type_s==cake");
}
+ @Test
+ public void testChildReturnFields() throws Exception {
+ indexSampleData(numberOfDocsPerNestedTest);
+
+ assertJQ(req("q", "test_s:testing",
+ "sort", "id asc",
+ "fl", "*, [child childFilter='lonely/lonelyGrandChild/test2_s:secondTest' fl='*, _nest_path_']",
--- End diff --
Sure thing,
just pushed a new commit :).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214542325
--- Diff: solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java ---
@@ -91,10 +98,10 @@ private void testChildDoctransformerXML() throws Exception {
"fl", "*,[child parentFilter=\"subject:parentDocument\"]"), test1);
assertQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ",
- "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2);
+ "fl", "id, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2);
--- End diff --
nitpick: remove that extra space after the comma here and in the other lines you modified.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #443: SOLR-12722: add fl param to ChildDocTransformer
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/443
BTW, I was skimming through DocTransformer, and noticed DocTransformer#needsSolrIndexSearcher.
Shouldn't this return true for ChildDocTransformer, since it searches the docId provided in the underlying index, and will fail if given a negative value?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214542110
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java ---
@@ -57,12 +58,34 @@
static final char PATH_SEP_CHAR = '/';
static final char NUM_SEP_CHAR = '#';
+ private static final ThreadLocal<Boolean> transformerInitialized = new ThreadLocal<Boolean>(){
+ @Override
+ protected Boolean initialValue() {
+ this.set(false);
+ return false;
+ }
+ };
private static final BooleanQuery rootFilter = new BooleanQuery.Builder()
.add(new BooleanClause(new MatchAllDocsQuery(), BooleanClause.Occur.MUST))
.add(new BooleanClause(new DocValuesFieldExistsQuery(NEST_PATH_FIELD_NAME), BooleanClause.Occur.MUST_NOT)).build();
@Override
public DocTransformer create(String field, SolrParams params, SolrQueryRequest req) {
+ if(transformerInitialized.get()) {
+ // this is a recursive call by SolrReturnFields see #createChildDocTransformer
+ return new DocTransformer.NoopFieldTransformer();
+ } else {
+ try {
+ // transformer is yet to be initialized in this thread, create it
+ transformerInitialized.set(true);
+ return createChildDocTransformer(field, params, req);
+ } finally {
+ transformerInitialized.remove();
--- End diff --
Hmm; maybe clearer to set(false)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214573620
--- Diff: solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java ---
@@ -1831,7 +1831,7 @@ public void testChildDoctransformer() throws IOException, SolrServerException {
q = new SolrQuery("q", "*:*", "indent", "true");
q.setFilterQueries(parentFilter);
- q.setFields("id,[child parentFilter=\"" + parentFilter +
+ q.setFields("id, level_i, [child parentFilter=\"" + parentFilter +
--- End diff --
These tests fail since the recent changes made in this branch result the child doc transformer defaulting to the top fl. Beforehand, the default child fl was "*", which does not filter "level_i" from child docs. Should we default to "*" in lucene version < 8, and change this behaviour only once Solr 8 arrives?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla closed the pull request at:
https://github.com/apache/lucene-solr/pull/443
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214773746
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java ---
@@ -132,6 +134,12 @@ public void transform(SolrDocument rootDoc, int rootDocId) {
// load the doc
SolrDocument doc = searcher.getDocFetcher().solrDoc(docId, childReturnFields);
+ if(childReturnFields.getTransformer() != null) {
+ if(childReturnFields.getTransformer().context != this.context) {
--- End diff --
maybe this line should only check if context is null? By checking if the context is different, it's suggestive there may already be a context but if that were true then we ought to reinstate the former context when done. I suspect it's always going to be null. Can you look?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214541692
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java ---
@@ -131,6 +131,12 @@ public void transform(SolrDocument rootDoc, int rootDocId) {
// load the doc
SolrDocument doc = searcher.getDocFetcher().solrDoc(docId, childReturnFields);
--- End diff --
I was thinking the same thing! But yeah; out of scope for this PR. CC @ErickErickson
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214541964
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java ---
@@ -57,12 +58,34 @@
static final char PATH_SEP_CHAR = '/';
static final char NUM_SEP_CHAR = '#';
+ private static final ThreadLocal<Boolean> transformerInitialized = new ThreadLocal<Boolean>(){
--- End diff --
This is the classic but more verbose way to declare a ThreadLocal. In Java 8 this can be a one-liner:
```ThreadLocal<Boolean> recursionCheckThreadLocal = ThreadLocal.withInitial(() -> Boolean.FALSE);```
And please consider another field name, not "transformerInitialized"? I suggest "recursionCheckThreadLocal" since it declares it's purpose (to detect recursion) and it's type (it's a ThreadLocal). "initialized" is dubious to me because it suggests an instance field of a class but this one is global.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214044911
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java ---
@@ -106,9 +107,12 @@ public DocTransformer create(String field, SolrParams params, SolrQueryRequest r
}
}
+ String childReturnFields = params.get("fl");
+ SolrReturnFields childSolrReturnFields = childReturnFields==null? new SolrReturnFields(): new SolrReturnFields(childReturnFields ,req);
--- End diff --
Shouldn't this default to the top level "fl"? I think so. Even if it didn't do this before, it in-effect behaves this way and it makes sense (to me) that it would.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #443: SOLR-12722: add fl param to ChildDocTransformer
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/443
I tried bypassing the stack overflow error from the original commit.
Hope this is good enough.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214773781
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformerFactory.java ---
@@ -106,9 +123,20 @@ public DocTransformer create(String field, SolrParams params, SolrQueryRequest r
}
}
+ String childReturnFields = params.get("fl");
+ SolrReturnFields childSolrReturnFields;
+ if(childReturnFields != null) {
+ childSolrReturnFields = new SolrReturnFields(childReturnFields, req);
+ } else if(req.getSchema().getDefaultLuceneMatchVersion().major < 8) {
--- End diff --
Nice
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214569199
--- Diff: solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java ---
@@ -1831,7 +1831,7 @@ public void testChildDoctransformer() throws IOException, SolrServerException {
q = new SolrQuery("q", "*:*", "indent", "true");
q.setFilterQueries(parentFilter);
- q.setFields("id,[child parentFilter=\"" + parentFilter +
+ q.setFields("id, level_i, [child parentFilter=\"" + parentFilter +
--- End diff --
I changed these tests before we defaulted to the top level "fl" parameter.
I shall investigate further.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #443: SOLR-12722: add fl param to ChildDocTransformer
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/443
Pushed a commit ensuring the context is set for child transformers only if it was not set previously.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214542527
--- Diff: solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java ---
@@ -243,11 +250,39 @@ private void testChildDocNonStoredDVFields() throws Exception {
"fl", "*,[child parentFilter=\"subject:parentDocument\"]"), test1);
assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ",
- "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2);
+ "fl", "intDvoDefault, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:foo\"]"), test2);
assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ",
- "fl", "subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=2]"), test3);
+ "fl", "intDvoDefault, subject,[child parentFilter=\"subject:parentDocument\" childFilter=\"title:bar\" limit=2]"), test3);
+
+ }
+
+ private void testChildReturnFields() throws Exception {
+ assertJQ(req("q", "*:*", "fq", "subject:\"parentDocument\" ",
+ "fl", "*,[child parentFilter=\"subject:parentDocument\" fl=\"intDvoDefault,child_fl:[value v='child_fl_test']\"]"),
+ "/response/docs/[0]/intDefault==42",
+ "/response/docs/[0]/_childDocuments_/[0]/intDvoDefault==42",
+ "/response/docs/[0]/_childDocuments_/[0]/child_fl=='child_fl_test'");
+
+ try(SolrQueryRequest req = req("q", "*:*", "fq", "subject:\"parentDocument\" ",
--- End diff --
Do you know XPath? assertQ takes xpath expressions that is powerful enough to do everything you are asserting here in less code. (assertQ is used in a *ton* of tests; plenty of examples to learn from). The assertJQ thing is less powerful. You don't *have* to rewrite it but it's at least worth being aware for future assertions.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #443: SOLR-12722: add fl param to ChildDocTransformer
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/443
Closing this PR as it was committed to master in commit e4f256be15ca44f12a4aecb32c13d1ab2617cc00
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214892512
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java ---
@@ -132,6 +134,12 @@ public void transform(SolrDocument rootDoc, int rootDocId) {
// load the doc
SolrDocument doc = searcher.getDocFetcher().solrDoc(docId, childReturnFields);
+ if(childReturnFields.getTransformer() != null) {
+ if(childReturnFields.getTransformer().context != this.context) {
--- End diff --
Seems like your suspicion was correct.
I will push a commit asap!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214531907
--- Diff: solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java ---
@@ -131,6 +131,12 @@ public void transform(SolrDocument rootDoc, int rootDocId) {
// load the doc
SolrDocument doc = searcher.getDocFetcher().solrDoc(docId, childReturnFields);
--- End diff --
Perhaps this should be add to DocFetcher#solrDoc in the future?
This is just a thought.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #443: SOLR-12722: add fl param to ChildDocTransform...
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/443#discussion_r214569119
--- Diff: solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java ---
@@ -114,4 +114,27 @@ public void transform(SolrDocument doc, int docid, float score) throws IOExcepti
public String toString() {
return getName();
}
+
+ /**
+ * Trivial Impl that ensure that the specified field is requested as an "extra" field,
+ * but then does nothing during the transformation phase.
+ */
+ public static final class NoopFieldTransformer extends DocTransformer {
--- End diff --
Yeah I added a default constructor to and changed another bit so it works either way.
I would prefer if this was left here, where it is easily accessible, preventing duplication of code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org