[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

classic Classic list List threaded Threaded
44 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
Github user moshebla commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194690146
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -249,14 +251,27 @@ public void handle(Map<String, Object> record, String path) {
         private SolrInputDocument buildDoc(Map<String, Object> m) {
           SolrInputDocument result = new SolrInputDocument();
           for (Map.Entry<String, Object> e : m.entrySet()) {
    -        if (e.getKey() == null) {// special case. JsonRecordReader emits child docs with null key
    +        if (entryIsChildDoc(e.getValue())) {// special case. JsonRecordReader emits child docs with null key
               if (e.getValue() instanceof List) {
                 List value = (List) e.getValue();
                 for (Object o : value) {
    -              if (o instanceof Map) result.addChildDocument(buildDoc((Map) o));
    +              if (o instanceof Map) {
    +                if (anonChildDocFlag) {
    +                  result.addChildDocument(buildDoc((Map) o));
    +                } else {
    +                  if(!result.containsKey(e.getKey())) {
    +                    result.setField(e.getKey(), new ArrayList<>(1));
    --- End diff --
   
    if we wanted to build the original JSON document as it was indexed, we would have to store it as a single valued array if that was the case. I'm afraid I did not quite get what you meant by using virtual fields, what for?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194754268
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -249,14 +251,27 @@ public void handle(Map<String, Object> record, String path) {
         private SolrInputDocument buildDoc(Map<String, Object> m) {
           SolrInputDocument result = new SolrInputDocument();
           for (Map.Entry<String, Object> e : m.entrySet()) {
    -        if (e.getKey() == null) {// special case. JsonRecordReader emits child docs with null key
    +        if (entryIsChildDoc(e.getValue())) {// special case. JsonRecordReader emits child docs with null key
               if (e.getValue() instanceof List) {
                 List value = (List) e.getValue();
                 for (Object o : value) {
    -              if (o instanceof Map) result.addChildDocument(buildDoc((Map) o));
    +              if (o instanceof Map) {
    +                if (anonChildDocFlag) {
    +                  result.addChildDocument(buildDoc((Map) o));
    +                } else {
    +                  if(!result.containsKey(e.getKey())) {
    +                    result.setField(e.getKey(), new ArrayList<>(1));
    --- End diff --
   
    By "virtual field" I'm referring to my earlier idea about having a special field in the schema that designates a child document with this relation.  it would have a name, multiValued option, and probably that's it.  https://issues.apache.org/jira/browse/SOLR-12298?focusedCommentId=16467523&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16467523


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194755084
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -249,14 +251,27 @@ public void handle(Map<String, Object> record, String path) {
         private SolrInputDocument buildDoc(Map<String, Object> m) {
           SolrInputDocument result = new SolrInputDocument();
           for (Map.Entry<String, Object> e : m.entrySet()) {
    -        if (e.getKey() == null) {// special case. JsonRecordReader emits child docs with null key
    +        if (entryIsChildDoc(e.getValue())) {// special case. JsonRecordReader emits child docs with null key
               if (e.getValue() instanceof List) {
                 List value = (List) e.getValue();
                 for (Object o : value) {
    -              if (o instanceof Map) result.addChildDocument(buildDoc((Map) o));
    +              if (o instanceof Map) {
    +                if (anonChildDocFlag) {
    +                  result.addChildDocument(buildDoc((Map) o));
    +                } else {
    +                  if(!result.containsKey(e.getKey())) {
    +                    result.setField(e.getKey(), new ArrayList<>(1));
    --- End diff --
   
    Any way; lets not increase the scope of this particular issue with my idea to add this info to the schema. It seems adequate to force the use of an internal array for a single value in SolrInputField that we know originated from an array.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194760654
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -556,82 +572,71 @@ private void parseFieldValue(SolrInputField sif) throws IOException {
           if (ev == JSONParser.OBJECT_START) {
             parseExtendedFieldValue(sif, ev);
           } else {
    -        Object val = parseNormalFieldValue(ev, sif.getName());
    +        Object val = parseNormalFieldValue(ev, sif);
             sif.setValue(val);
           }
         }
     
         private void parseExtendedFieldValue(SolrInputField sif, int ev) throws IOException {
    --- End diff --
   
    Lets put "ev" as first arg to be more consistent


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194766692
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -664,8 +672,38 @@ private Object parseSingleFieldValue(int ev, String fieldName) throws IOExceptio
             if (ev == JSONParser.ARRAY_END) {
               return lst;
             }
    -        Object val = parseSingleFieldValue(ev, fieldName);
    +        Object val = parseSingleFieldValue(ev, sif);
             lst.add(val);
    +        sif.setValue(null);
    +      }
    +    }
    +
    +    private boolean isChildDoc(SolrInputDocument extendedMap) {
    --- End diff --
   
    rename extendedMap to extendedFieldValue


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194765026
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -664,8 +672,38 @@ private Object parseSingleFieldValue(int ev, String fieldName) throws IOExceptio
             if (ev == JSONParser.ARRAY_END) {
               return lst;
             }
    -        Object val = parseSingleFieldValue(ev, fieldName);
    +        Object val = parseSingleFieldValue(ev, sif);
             lst.add(val);
    +        sif.setValue(null);
    +      }
    +    }
    +
    +    private boolean isChildDoc(SolrInputDocument extendedMap) {
    +      return extendedMap.containsKey(req.getSchema().getUniqueKeyField().getName());
    +    }
    +
    +    private boolean entryIsChildDoc(Object val) {
    +      if(val instanceof List) {
    +        List listVal = (List) val;
    +        if (listVal.size() == 0) return false;
    +        return  listVal.get(0) instanceof Map;
    +      }
    +      return val instanceof Map;
    +    }
    +
    +    private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
    --- End diff --
   
    the name seems off -- 'generate' isn't bad but this method is parsing and plenty of other methods here parse and are named as such so I think this should be `parseExtendedValueAsDoc` or better `parseExtendedFieldValueAsDoc` since it is only called by `parseExtendedFieldValue`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194761077
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -556,82 +572,71 @@ private void parseFieldValue(SolrInputField sif) throws IOException {
           if (ev == JSONParser.OBJECT_START) {
             parseExtendedFieldValue(sif, ev);
           } else {
    -        Object val = parseNormalFieldValue(ev, sif.getName());
    +        Object val = parseNormalFieldValue(ev, sif);
             sif.setValue(val);
           }
         }
     
         private void parseExtendedFieldValue(SolrInputField sif, int ev) throws IOException {
           assert ev == JSONParser.OBJECT_START;
     
    -      Object normalFieldValue = null;
    -      Map<String, Object> extendedInfo = null;
    +      SolrInputDocument extendedSolrDocument = generateExtendedValueDoc(ev);
     
    -      for (; ; ) {
    -        ev = parser.nextEvent();
    -        switch (ev) {
    -          case JSONParser.STRING:
    -            String label = parser.getString();
    -            if ("boost".equals(label)) {
    -              ev = parser.nextEvent();
    -              if (ev != JSONParser.NUMBER &&
    -                  ev != JSONParser.LONG &&
    -                  ev != JSONParser.BIGNUMBER) {
    -                throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Boost should have number. "
    -                    + "Unexpected " + JSONParser.getEventString(ev) + " at [" + parser.getPosition() + "], field=" + sif.getName());
    -              }
    +      if (isChildDoc(extendedSolrDocument)) {
    +        SolrInputDocument cDoc = new SolrInputDocument();
    +        for (Map.Entry<String, SolrInputField> extendedEntry: extendedSolrDocument.entrySet()) {
    +          cDoc.setField(extendedEntry.getKey(), extendedEntry.getValue().getValue());
    --- End diff --
   
    Why can't you simply do sif.addValue(extendedSolrDocument)?  Thus avoiding the rebuilding of a SolrInputDocument that seems to have no purpose.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194761432
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -664,8 +672,38 @@ private Object parseSingleFieldValue(int ev, String fieldName) throws IOExceptio
             if (ev == JSONParser.ARRAY_END) {
               return lst;
             }
    -        Object val = parseSingleFieldValue(ev, fieldName);
    +        Object val = parseSingleFieldValue(ev, sif);
             lst.add(val);
    +        sif.setValue(null);
    +      }
    +    }
    +
    +    private boolean isChildDoc(SolrInputDocument extendedMap) {
    +      return extendedMap.containsKey(req.getSchema().getUniqueKeyField().getName());
    +    }
    +
    +    private boolean entryIsChildDoc(Object val) {
    +      if(val instanceof List) {
    +        List listVal = (List) val;
    +        if (listVal.size() == 0) return false;
    +        return  listVal.get(0) instanceof Map;
    +      }
    +      return val instanceof Map;
    +    }
    +
    +    private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
    +      assert ev == JSONParser.OBJECT_START;
    +      SolrInputDocument extendedInfo = new SolrInputDocument();
    +
    +      while(true) {
    +        ev = parser.nextEvent();
    +        if (ev == JSONParser.OBJECT_END) {
    +          return extendedInfo;
    +        }
    +        String label = parser.getString();
    +        SolrInputField sif = new SolrInputField(label);
    +        parseFieldValue(sif);
    +        extendedInfo.addField(label, sif.getValue());
    --- End diff --
   
    I think you can use extendedInfo.putField(key,SolrInputField) here?  Oh I see comments elsewhere in this file explaining why addField is used; maybe copy-paste those comments?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r194768590
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -556,82 +572,71 @@ private void parseFieldValue(SolrInputField sif) throws IOException {
           if (ev == JSONParser.OBJECT_START) {
             parseExtendedFieldValue(sif, ev);
           } else {
    -        Object val = parseNormalFieldValue(ev, sif.getName());
    +        Object val = parseNormalFieldValue(ev, sif);
             sif.setValue(val);
           }
         }
     
         private void parseExtendedFieldValue(SolrInputField sif, int ev) throws IOException {
           assert ev == JSONParser.OBJECT_START;
     
    -      Object normalFieldValue = null;
    -      Map<String, Object> extendedInfo = null;
    +      SolrInputDocument extendedSolrDocument = generateExtendedValueDoc(ev);
     
    -      for (; ; ) {
    -        ev = parser.nextEvent();
    -        switch (ev) {
    -          case JSONParser.STRING:
    -            String label = parser.getString();
    -            if ("boost".equals(label)) {
    -              ev = parser.nextEvent();
    -              if (ev != JSONParser.NUMBER &&
    -                  ev != JSONParser.LONG &&
    -                  ev != JSONParser.BIGNUMBER) {
    -                throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Boost should have number. "
    -                    + "Unexpected " + JSONParser.getEventString(ev) + " at [" + parser.getPosition() + "], field=" + sif.getName());
    -              }
    +      if (isChildDoc(extendedSolrDocument)) {
    +        SolrInputDocument cDoc = new SolrInputDocument();
    +        for (Map.Entry<String, SolrInputField> extendedEntry: extendedSolrDocument.entrySet()) {
    +          cDoc.setField(extendedEntry.getKey(), extendedEntry.getValue().getValue());
    +        }
    +        sif.addValue(cDoc);
    +        return;
    +      }
     
    -              String message = "Ignoring field boost: " + parser.getDouble() + " as index-time boosts are not supported anymore";
    -              if (WARNED_ABOUT_INDEX_TIME_BOOSTS.compareAndSet(false, true)) {
    -                log.warn(message);
    -              } else {
    -                log.debug(message);
    -              }
    -            } else if ("value".equals(label)) {
    -              normalFieldValue = parseNormalFieldValue(parser.nextEvent(), sif.getName());
    -            } else {
    -              // If we encounter other unknown map keys, then use a map
    -              if (extendedInfo == null) {
    -                extendedInfo = new HashMap<>(2);
    -              }
    -              // for now, the only extended info will be field values
    -              // we could either store this as an Object or a SolrInputField
    -              Object val = parseNormalFieldValue(parser.nextEvent(), sif.getName());
    -              extendedInfo.put(label, val);
    -            }
    -            break;
    +      Object normalFieldValue = null;
    +      Map<String, Object> extendedInfo = null;
     
    -          case JSONParser.OBJECT_END:
    -            if (extendedInfo != null) {
    -              if (normalFieldValue != null) {
    -                extendedInfo.put("value", normalFieldValue);
    -              }
    -              sif.setValue(extendedInfo);
    -            } else {
    -              sif.setValue(normalFieldValue);
    -            }
    -            return;
    +      for (String label: extendedSolrDocument.keySet() ) {
    --- End diff --
   
    Whenever I see looping over keys from a map-like thing, I often see a line of code that then fetches the value, as seen here.  This internally results in a bunch of lookups that aren't necessary.  Instead,  note that SolrInputDocument is Iterable<SolrInputField> (which itself has the key and value), so just loop over that.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr issue #395: WIP SOLR-12362: add tests for working relational doc...

janhoy
In reply to this post by janhoy
Github user moshebla commented on the issue:

    https://github.com/apache/lucene-solr/pull/395
 
    Tried tackling the javadoc references, as I'm fairly new this I hope I didn't mess it up too badly.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195071571
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -570,37 +573,34 @@ private SolrInputDocument parseDoc(int ev) throws IOException {
         private void parseFieldValue(SolrInputField sif) throws IOException {
           int ev = parser.nextEvent();
           if (ev == JSONParser.OBJECT_START) {
    -        parseExtendedFieldValue(sif, ev);
    +        parseExtendedFieldValue(ev, sif);
           } else {
             Object val = parseNormalFieldValue(ev, sif);
             sif.setValue(val);
           }
         }
     
    -    private void parseExtendedFieldValue(SolrInputField sif, int ev) throws IOException {
    +    private void parseExtendedFieldValue(int ev, SolrInputField sif) throws IOException {
           assert ev == JSONParser.OBJECT_START;
     
    -      SolrInputDocument extendedSolrDocument = generateExtendedValueDoc(ev);
    +      SolrInputDocument extendedSolrDocument = parseExtendedValueAsDoc(ev);
     
           if (isChildDoc(extendedSolrDocument)) {
    -        SolrInputDocument cDoc = new SolrInputDocument();
    -        for (Map.Entry<String, SolrInputField> extendedEntry: extendedSolrDocument.entrySet()) {
    -          cDoc.setField(extendedEntry.getKey(), extendedEntry.getValue().getValue());
    -        }
    -        sif.addValue(cDoc);
    +        sif.addValue(extendedSolrDocument);
             return;
           }
     
           Object normalFieldValue = null;
           Map<String, Object> extendedInfo = null;
     
    -      for (String label: extendedSolrDocument.keySet() ) {
    -        Object fieldVal = extendedSolrDocument.get(label).getValue();
    -        if ("boost".equals(label)) {
    +      for (SolrInputField field: extendedSolrDocument) {
    +        Object fieldVal = field.getValue();
    +        String fieldName = field.getName();
    --- End diff --
   
    I can understand why you chose the names `fieldName` and `fieldVal` because you are getting this from a SolrInputField.  But recognize what's going on here is very unusual ... we are at this point treating this SolrInputDocument as if it were a map and not as a document.  Lets add comments about this very explicitly before we loop because this is so strange.  Maybe the `field` var could be named `entry`.  The _former_ name `label` (or perhaps `extLabel`) var name is much more appropriate as it is _not_ a treated as *field* label/name.  Likewise, just choose `val` or `value` or `extValue`.
   
    Lets also add a bit of docs to parseExtendedFieldValue explain what this is for, since it's very non-obvious, I think.  Essentially, this is a JSON object that is _either_ a child doc, or it's a Map (used for atomic updates), or it's an outdated mechanism to specify the boost which is no longer in Lucene/Solr.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195082392
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -703,6 +703,10 @@ private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
             String label = parser.getString();
             SolrInputField sif = new SolrInputField(label);
             parseFieldValue(sif);
    +        // pulling out the pieces may seem weird, but it's because
    --- End diff --
   
    Good.  The duplication of comment now has me wondering about this method overall -- why does it seem to be duplicating existing logic -- buildDoc?  Would things "just work" if we replaced generateExtendedValueDoc with buildDoc()?  I think it would?  Less look-a-like code to maintain.  I suppose the only problem with that is it would permit weird things like anonymous child docs on a labelled child doc.  I just wouldn't worry about that now though; we catch that later in AddUpdateCommand.flatten().
   
    BTW I'm sure you realize now that I don't have everything figured out from the start; it's an iterative journey for you and me/the-reviewer.  :-)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user moshebla commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195093742
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -703,6 +703,10 @@ private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
             String label = parser.getString();
             SolrInputField sif = new SolrInputField(label);
             parseFieldValue(sif);
    +        // pulling out the pieces may seem weird, but it's because
    --- End diff --
   
    It's actually quite refreshing to get another person's opinion.
    I don't think it would work flawlessly without changing a lot of the code, since buildDoc expects a Map as an input, so we would have to parse the map beforehand.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user moshebla commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195098134
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -570,37 +573,34 @@ private SolrInputDocument parseDoc(int ev) throws IOException {
         private void parseFieldValue(SolrInputField sif) throws IOException {
           int ev = parser.nextEvent();
           if (ev == JSONParser.OBJECT_START) {
    -        parseExtendedFieldValue(sif, ev);
    +        parseExtendedFieldValue(ev, sif);
           } else {
             Object val = parseNormalFieldValue(ev, sif);
             sif.setValue(val);
           }
         }
     
    -    private void parseExtendedFieldValue(SolrInputField sif, int ev) throws IOException {
    +    private void parseExtendedFieldValue(int ev, SolrInputField sif) throws IOException {
           assert ev == JSONParser.OBJECT_START;
     
    -      SolrInputDocument extendedSolrDocument = generateExtendedValueDoc(ev);
    +      SolrInputDocument extendedSolrDocument = parseExtendedValueAsDoc(ev);
     
           if (isChildDoc(extendedSolrDocument)) {
    -        SolrInputDocument cDoc = new SolrInputDocument();
    -        for (Map.Entry<String, SolrInputField> extendedEntry: extendedSolrDocument.entrySet()) {
    -          cDoc.setField(extendedEntry.getKey(), extendedEntry.getValue().getValue());
    -        }
    -        sif.addValue(cDoc);
    +        sif.addValue(extendedSolrDocument);
             return;
           }
     
           Object normalFieldValue = null;
           Map<String, Object> extendedInfo = null;
     
    -      for (String label: extendedSolrDocument.keySet() ) {
    -        Object fieldVal = extendedSolrDocument.get(label).getValue();
    -        if ("boost".equals(label)) {
    +      for (SolrInputField field: extendedSolrDocument) {
    +        Object fieldVal = field.getValue();
    +        String fieldName = field.getName();
    --- End diff --
   
    Just tackled this one.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195116663
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -593,14 +602,14 @@ private void parseExtendedFieldValue(int ev, SolrInputField sif) throws IOExcept
           Object normalFieldValue = null;
           Map<String, Object> extendedInfo = null;
     
    -      for (SolrInputField field: extendedSolrDocument) {
    -        Object fieldVal = field.getValue();
    -        String fieldName = field.getName();
    -        if ("boost".equals(fieldName)) {
    -          Object boostVal = fieldVal;
    +      for (SolrInputField entry: extendedSolrDocument) {
    +        Object label = entry.getValue();
    --- End diff --
   
    LOL you have these mixed up :-)  entry.getName should be the label.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user moshebla commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195118056
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -703,6 +703,10 @@ private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
             String label = parser.getString();
             SolrInputField sif = new SolrInputField(label);
             parseFieldValue(sif);
    +        // pulling out the pieces may seem weird, but it's because
    --- End diff --
   
    Skimming through the code on master, it looks as if the map parsing was done here before only partially. Currently it seems like we either duplicate the map parsing part, or the document parsing part. If we go with the map part, we would build an intermediate map and then parse it to a document instead of streaming it using the parser. I'm not quite sure if using buildDoc would be as beneficial, since in either case, we have to duplicate some part.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user moshebla commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195118329
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -593,14 +602,14 @@ private void parseExtendedFieldValue(int ev, SolrInputField sif) throws IOExcept
           Object normalFieldValue = null;
           Map<String, Object> extendedInfo = null;
     
    -      for (SolrInputField field: extendedSolrDocument) {
    -        Object fieldVal = field.getValue();
    -        String fieldName = field.getName();
    -        if ("boost".equals(fieldName)) {
    -          Object boostVal = fieldVal;
    +      for (SolrInputField entry: extendedSolrDocument) {
    +        Object label = entry.getValue();
    --- End diff --
   
    oops


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user dsmiley commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195119625
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -703,6 +703,10 @@ private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
             String label = parser.getString();
             SolrInputField sif = new SolrInputField(label);
             parseFieldValue(sif);
    +        // pulling out the pieces may seem weird, but it's because
    --- End diff --
   
    Sorry I meant `parseDoc`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user moshebla commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195122269
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -703,6 +703,10 @@ private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
             String label = parser.getString();
             SolrInputField sif = new SolrInputField(label);
             parseFieldValue(sif);
    +        // pulling out the pieces may seem weird, but it's because
    --- End diff --
   
    True, didn't quite notice it, I'll upload a commit soon. :smile:


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #395: WIP SOLR-12362: add tests for working relatio...

janhoy
In reply to this post by janhoy
Github user moshebla commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/395#discussion_r195131919
 
    --- Diff: solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---
    @@ -703,6 +703,10 @@ private SolrInputDocument generateExtendedValueDoc(int ev) throws IOException {
             String label = parser.getString();
             SolrInputField sif = new SolrInputField(label);
             parseFieldValue(sif);
    +        // pulling out the pieces may seem weird, but it's because
    --- End diff --
   
    It worked.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

123