Solr
  1. Solr
  2. SOLR-7622

Allow DocumentTransformer to request fields that are not displayed

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.3
    • Component/s: None
    • Labels:
      None

      Description

      Currently, a DocumentTransformer can only inspect fields that appear in the final SolrDocument. But we already have infrastructure to request fields from the IndexSearcher that do not get displayed.

      This will let DocumentTransformers ask for more fields that are not requested in the fl parameter

      1. SOLR-7622-extra-request-fields.patch
        10 kB
        Ryan McKinley
      2. SOLR-7622-extra-request-fields.patch
        3 kB
        Ryan McKinley
      3. SOLR-7662.patch
        23 kB
        Noble Paul

        Activity

        Hide
        Ryan McKinley added a comment -

        The key addition is:

        
        +  /**
        +   * When a transformer needs access to fields that are not automaticaly derived from the
        +   * input fields names, this option lets us explicitly say the field names that we hope
        +   * will be in the SolrDocument.  These fields will be requestd from the 
        +   * {@link SolrIndexSearcher} but may or may not be returned in the final
        +   * {@link QueryResponseWriter}
        +   * 
        +   * @return a list of extra lucene fields
        +   */
        +  public String[] getExtraRequestFields() {
        +    return null;
        +  }
        +  
        
        Show
        Ryan McKinley added a comment - The key addition is: + /** + * When a transformer needs access to fields that are not automaticaly derived from the + * input fields names, this option lets us explicitly say the field names that we hope + * will be in the SolrDocument. These fields will be requestd from the + * {@link SolrIndexSearcher} but may or may not be returned in the final + * {@link QueryResponseWriter} + * + * @ return a list of extra lucene fields + */ + public String [] getExtraRequestFields() { + return null ; + } +
        Hide
        Ryan McKinley added a comment -

        now with tests and README

        Show
        Ryan McKinley added a comment - now with tests and README
        Hide
        ASF subversion and git services added a comment -

        Commit 1683134 from Ryan McKinley in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1683134 ]

        SOLR-7622: let DocTransformers request extra fields

        Show
        ASF subversion and git services added a comment - Commit 1683134 from Ryan McKinley in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1683134 ] SOLR-7622 : let DocTransformers request extra fields
        Hide
        ASF subversion and git services added a comment -

        Commit 1683162 from Ryan McKinley in branch 'dev/trunk'
        [ https://svn.apache.org/r1683162 ]

        Merged revision(s) 1683134 from lucene/dev/branches/branch_5x:
        SOLR-7622: let DocTransformers request extra fields
        ........

        Show
        ASF subversion and git services added a comment - Commit 1683162 from Ryan McKinley in branch 'dev/trunk' [ https://svn.apache.org/r1683162 ] Merged revision(s) 1683134 from lucene/dev/branches/branch_5x: SOLR-7622 : let DocTransformers request extra fields ........
        Hide
        ASF subversion and git services added a comment -

        Commit 1683170 from Ryan McKinley in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1683170 ]

        SOLR-7622: add eol style

        Show
        ASF subversion and git services added a comment - Commit 1683170 from Ryan McKinley in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1683170 ] SOLR-7622 : add eol style
        Hide
        ASF subversion and git services added a comment -

        Commit 1683171 from Ryan McKinley in branch 'dev/trunk'
        [ https://svn.apache.org/r1683171 ]

        Merged revision(s) 1683170 from lucene/dev/branches/branch_5x:
        SOLR-7622: add eol style

        ........

        Show
        ASF subversion and git services added a comment - Commit 1683171 from Ryan McKinley in branch 'dev/trunk' [ https://svn.apache.org/r1683171 ] Merged revision(s) 1683170 from lucene/dev/branches/branch_5x: SOLR-7622 : add eol style ........
        Hide
        Noble Paul added a comment -

        This does not really solve the problem . You are not addressing the Binary responsewriter
        I'm working on SOLR-7662 which will deal with this in wt=javabin as well

        Show
        Noble Paul added a comment - This does not really solve the problem . You are not addressing the Binary responsewriter I'm working on SOLR-7662 which will deal with this in wt=javabin as well
        Hide
        Noble Paul added a comment -

        please change the signature from

        
          public String[] getExtraRequestFields() {
            return null;
          }  
        

        to

          public Set<String> getExtraRequestFields() {
            return null;
          }  
        

        array is a bad choice for an API because they are mutable
        I'm making this change

        Show
        Noble Paul added a comment - please change the signature from public String [] getExtraRequestFields() { return null ; } to public Set< String > getExtraRequestFields() { return null ; } array is a bad choice for an API because they are mutable I'm making this change
        Hide
        ASF subversion and git services added a comment -

        Commit 1687307 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1687307 ]

        SOLR-7622: changed return type from array to Set

        Show
        ASF subversion and git services added a comment - Commit 1687307 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1687307 ] SOLR-7622 : changed return type from array to Set
        Hide
        ASF subversion and git services added a comment -

        Commit 1687330 from Noble Paul in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1687330 ]

        SOLR-7622: changed return type from array to Set

        Show
        ASF subversion and git services added a comment - Commit 1687330 from Noble Paul in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1687330 ] SOLR-7622 : changed return type from array to Set
        Hide
        Hoss Man added a comment -

        can someone please clarify what exactly the status of this issue is?


        it was backported to 5x by ryan, and subsequent tweaks to the API were then made to both trunk & 5x by noble, but the issue is still open and not listed anywhere in CHANGES – which is a really dangerous state to leave an issue in. Feature is commited it should ALWAYS be in CHANGES since you never know when someone will create a release branch – and if folks didn't think this was ready to be released, it definitely shouldn't have been backported to 5x.


        In addition - the commits made in the last 24 hours seem to suspiciously corrolate to 4 jenkins failures on the 5x branch that have happened since then in the test that was modified...

        https://builds.apache.org/job/Lucene-Solr-Tests-5.x-Java7/3259/
        https://builds.apache.org/job/Lucene-Solr-Tests-5.x-Java7/3258/
        http://jenkins.thetaphi.de/job/Lucene-Solr-5.x-Linux/13023/
        http://jenkins.thetaphi.de/job/Lucene-Solr-5.x-Windows/4837/

        This seed doesn't fail reliably for me, but beasting it failed on the second iteration...

        ant beast -Dbeast.iters=10  -Dtestcase=TestCustomDocTransformer -Dtests.method=testCustomTransformer -Dtests.seed=C062F72E9252DCA8 -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=hr -Dtests.timezone=America/Montserrat -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
        

        If i revert r1687330, that beast command passes – and still passes even if i bump up the iters and simplify the args so that we cover more random variable permutations (to allow for more possible changes the way entropy is used between the two versions of the test)

        ant beast -Dbeast.iters=50  -Dtestcase=TestCustomDocTransformer -Dtests.method=testCustomTransformer -Dtests.multiplier=2 -Dtests.slow=true
        
        Show
        Hoss Man added a comment - can someone please clarify what exactly the status of this issue is? it was backported to 5x by ryan, and subsequent tweaks to the API were then made to both trunk & 5x by noble, but the issue is still open and not listed anywhere in CHANGES – which is a really dangerous state to leave an issue in. Feature is commited it should ALWAYS be in CHANGES since you never know when someone will create a release branch – and if folks didn't think this was ready to be released, it definitely shouldn't have been backported to 5x. In addition - the commits made in the last 24 hours seem to suspiciously corrolate to 4 jenkins failures on the 5x branch that have happened since then in the test that was modified... https://builds.apache.org/job/Lucene-Solr-Tests-5.x-Java7/3259/ https://builds.apache.org/job/Lucene-Solr-Tests-5.x-Java7/3258/ http://jenkins.thetaphi.de/job/Lucene-Solr-5.x-Linux/13023/ http://jenkins.thetaphi.de/job/Lucene-Solr-5.x-Windows/4837/ This seed doesn't fail reliably for me, but beasting it failed on the second iteration... ant beast -Dbeast.iters=10 -Dtestcase=TestCustomDocTransformer -Dtests.method=testCustomTransformer -Dtests.seed=C062F72E9252DCA8 -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=hr -Dtests.timezone=America/Montserrat -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1 If i revert r1687330, that beast command passes – and still passes even if i bump up the iters and simplify the args so that we cover more random variable permutations (to allow for more possible changes the way entropy is used between the two versions of the test) ant beast -Dbeast.iters=50 -Dtestcase=TestCustomDocTransformer -Dtests.method=testCustomTransformer -Dtests.multiplier=2 -Dtests.slow=true
        Hide
        Noble Paul added a comment -

        it was backported to 5x by ryan, and subsequent tweaks to the API were then made to both trunk

        I assumed that Ryan McKinley would have added the entry into CHANGES.txt. I'm not sure if he plans to do more work on this

        Show
        Noble Paul added a comment - it was backported to 5x by ryan, and subsequent tweaks to the API were then made to both trunk I assumed that Ryan McKinley would have added the entry into CHANGES.txt. I'm not sure if he plans to do more work on this
        Hide
        Noble Paul added a comment - - edited

        beasting for 100 iters did not fail it for me . I shall revert the changes for now and see if it repeats

        Show
        Noble Paul added a comment - - edited beasting for 100 iters did not fail it for me . I shall revert the changes for now and see if it repeats
        Hide
        ASF subversion and git services added a comment -

        Commit 1687689 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1687689 ]

        SOLR-7622: reverting the previos change

        Show
        ASF subversion and git services added a comment - Commit 1687689 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1687689 ] SOLR-7622 : reverting the previos change
        Hide
        ASF subversion and git services added a comment -

        Commit 1687692 from Noble Paul in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1687692 ]

        SOLR-7622: reverting the previos change

        Show
        ASF subversion and git services added a comment - Commit 1687692 from Noble Paul in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1687692 ] SOLR-7622 : reverting the previos change
        Hide
        Alexandre Rafalovitch added a comment -

        Is this actually in 5.3 as the release notes claim? It's unresolved and untargeted.

        Show
        Alexandre Rafalovitch added a comment - Is this actually in 5.3 as the release notes claim? It's unresolved and untargeted.
        Hide
        David Smiley added a comment -

        Thanks for bringing this to our attention, Alexandre. I double-checked with the code and examined the commits. Indeed this got into 5.3. Noble's commits that say "revert" was a revert of exactly what the API was that switched back & forth (Set<String> vs String[]) – the functionality remains. I Updated the issue for 5.3 and will mark as resolved.

        Another oddity in the process is that I see this was committed to 5x and ported to trunk vs the other way around.

        Show
        David Smiley added a comment - Thanks for bringing this to our attention, Alexandre. I double-checked with the code and examined the commits. Indeed this got into 5.3. Noble's commits that say "revert" was a revert of exactly what the API was that switched back & forth (Set<String> vs String[]) – the functionality remains. I Updated the issue for 5.3 and will mark as resolved. Another oddity in the process is that I see this was committed to 5x and ported to trunk vs the other way around.

          People

          • Assignee:
            Ryan McKinley
            Reporter:
            Ryan McKinley
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development