Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-8902

ReturnFields can return fields that were not requested

    Details

      Description

      It looks like something changed that now returns all fields requested from lucene, not just the ones request from solr.

      This is the difference between 'fields' and 'okFieldNames' in SolrReturnFields.

      The logic here:
      https://github.com/apache/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java#L141

      adds all the 'fields' to 'okFieldName'

      I think that should be removed

      1. SOLR-8902.diff
        4 kB
        Ryan McKinley
      2. SOLR-8902-branch_6_0.patch
        24 kB
        Steve Rowe

        Activity

        Hide
        ryantxu Ryan McKinley added a comment -

        Here is a simple patch with a test

        Show
        ryantxu Ryan McKinley added a comment - Here is a simple patch with a test
        Hide
        mkhludnev Mikhail Khludnev added a comment -

        I walked around ReturnFields recently (while work on SOLR-8208), was a little bit wondered, but considered SOLR-7622 as a feature after all, and now I found SOLR-8902 issue description is a little controversial. Sadly, I don't fully understand what's about and what all this wantsFoo, okFoo means. I just want to make sure that this fix is solid with existing.

        Show
        mkhludnev Mikhail Khludnev added a comment - I walked around ReturnFields recently (while work on SOLR-8208 ), was a little bit wondered, but considered SOLR-7622 as a feature after all, and now I found SOLR-8902 issue description is a little controversial. Sadly, I don't fully understand what's about and what all this wantsFoo , okFoo means. I just want to make sure that this fix is solid with existing.
        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

        Just curious... can this be tickled from the outside (i.e. in stock solr, is it possible to send a request that results in more fields than requested being sent back?)

        Show
        yseeley@gmail.com Yonik Seeley added a comment - Just curious... can this be tickled from the outside (i.e. in stock solr, is it possible to send a request that results in more fields than requested being sent back?)
        Hide
        ryantxu Ryan McKinley added a comment -

        You can get more fields then you request if a DocumentTransformer uses `getExtraRequestFields()` SOLR-7622 – this should tell Lucene to fetch the field, but should not affect the fields returned in the final SolrDocument. An 'off-the-shelf' example is with the new 'geo' transformer:

        from the attached patch
        +    // Don't return 'store_rpt' just because it is required by the transformer
        +    rf = new SolrReturnFields( req("fl", "[geo f=store_rpt]") );
        +    assertFalse( rf.wantsScore() );
        +    assertTrue(rf.wantsField("[geo]"));
        +    assertFalse( rf.wantsField( "store_rpt" ) );
        +    assertFalse(rf.wantsAllFields());
        +    assertNotNull(rf.getTransformer());
        

        I think there are other possible cases, but most of the time, when a transformer renames the field, it removes the SolrDocument so the fact that ReturnFields allows the original name does not matter. For example:

         fl=aaa:bbb
        

        Without this patch,

         returnFields.wantsField( 'bbb' ) == true
        

        Sadly, I don't fully understand what's about and what all this wantsFoo, okFoo means

        The distinction is between what fields are requested from Lucene and put on the SolrDocument passed to the DocumentTransformers vs what fields the end user wants to see in the results.

        Show
        ryantxu Ryan McKinley added a comment - You can get more fields then you request if a DocumentTransformer uses `getExtraRequestFields()` SOLR-7622 – this should tell Lucene to fetch the field, but should not affect the fields returned in the final SolrDocument. An 'off-the-shelf' example is with the new 'geo' transformer: from the attached patch + // Don't return 'store_rpt' just because it is required by the transformer + rf = new SolrReturnFields( req( "fl" , "[geo f=store_rpt]" ) ); + assertFalse( rf.wantsScore() ); + assertTrue(rf.wantsField( "[geo]" )); + assertFalse( rf.wantsField( "store_rpt" ) ); + assertFalse(rf.wantsAllFields()); + assertNotNull(rf.getTransformer()); I think there are other possible cases, but most of the time, when a transformer renames the field, it removes the SolrDocument so the fact that ReturnFields allows the original name does not matter. For example: fl=aaa:bbb Without this patch, returnFields.wantsField( 'bbb' ) == true Sadly, I don't fully understand what's about and what all this wantsFoo, okFoo means The distinction is between what fields are requested from Lucene and put on the SolrDocument passed to the DocumentTransformers vs what fields the end user wants to see in the results.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 8254724bb17fd4cf479ab34c3919d9a862ca4ea5 in lucene-solr's branch refs/heads/branch_6x from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8254724 ]

        SOLR-8902: Make sure ReturnFields only returns the requested fields

        Show
        jira-bot ASF subversion and git services added a comment - Commit 8254724bb17fd4cf479ab34c3919d9a862ca4ea5 in lucene-solr's branch refs/heads/branch_6x from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8254724 ] SOLR-8902 : Make sure ReturnFields only returns the requested fields
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit ffd557b117455cb2a37e1a27cfd0d026314b4137 in lucene-solr's branch refs/heads/master from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ffd557b ]

        SOLR-8902: Make sure ReturnFields only returns the requested fields

        Show
        jira-bot ASF subversion and git services added a comment - Commit ffd557b117455cb2a37e1a27cfd0d026314b4137 in lucene-solr's branch refs/heads/master from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ffd557b ] SOLR-8902 : Make sure ReturnFields only returns the requested fields
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit e26c0b7125903e24e5865b825f0f9e993eb10178 in lucene-solr's branch refs/heads/branch_6x from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e26c0b7 ]

        SOLR-8902: fix glob test (put back the fields.clear())

        Show
        jira-bot ASF subversion and git services added a comment - Commit e26c0b7125903e24e5865b825f0f9e993eb10178 in lucene-solr's branch refs/heads/branch_6x from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e26c0b7 ] SOLR-8902 : fix glob test (put back the fields.clear())
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 6b7030d637e86effe7e38f3610e9475b0b595cf6 in lucene-solr's branch refs/heads/master from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6b7030d ]

        SOLR-8902: fix glob test (put back the fields.clear())

        Show
        jira-bot ASF subversion and git services added a comment - Commit 6b7030d637e86effe7e38f3610e9475b0b595cf6 in lucene-solr's branch refs/heads/master from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6b7030d ] SOLR-8902 : fix glob test (put back the fields.clear())
        Hide
        erickerickson Erick Erickson added a comment -

        Ryan McKinley how confident are you about the testing for this patch (and, BTW, should it be closed?)

        The reason I asked is because SOLR-3191 changes a lot of the guts of SolrReturnFields, so much so that I'm having a really hard time reconciling even the very small amount of code changed from that file in this patch. However, just substituting SolrReturnFields.java entirely to trunk works, as in all the tests in ReturnFieldsTests work including the one from this patch. Do you have an opinion here?

        Show
        erickerickson Erick Erickson added a comment - Ryan McKinley how confident are you about the testing for this patch (and, BTW, should it be closed?) The reason I asked is because SOLR-3191 changes a lot of the guts of SolrReturnFields, so much so that I'm having a really hard time reconciling even the very small amount of code changed from that file in this patch. However, just substituting SolrReturnFields.java entirely to trunk works, as in all the tests in ReturnFieldsTests work including the one from this patch. Do you have an opinion here?
        Hide
        ryantxu Ryan McKinley added a comment -

        Erick Erickson If your patch for SOLR-3191 does not break any tests than we are good. The coverage is reasonable. If there are uncovered expectations that should not be your problem

        Show
        ryantxu Ryan McKinley added a comment - Erick Erickson If your patch for SOLR-3191 does not break any tests than we are good. The coverage is reasonable. If there are uncovered expectations that should not be your problem
        Hide
        erickerickson Erick Erickson added a comment -

        Thanks Ryan!

        Show
        erickerickson Erick Erickson added a comment - Thanks Ryan!
        Hide
        hossman Hoss Man added a comment -

        Manually correcting fixVersion per Step #S5 of LUCENE-7271

        Show
        hossman Hoss Man added a comment - Manually correcting fixVersion per Step #S5 of LUCENE-7271
        Hide
        steve_rowe Steve Rowe added a comment -

        Reopening to backport to 6.0.1.

        Show
        steve_rowe Steve Rowe added a comment - Reopening to backport to 6.0.1.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit e562803b3fe3d3f829d251228648f7c1a4da81bf in lucene-solr's branch refs/heads/branch_6_0 from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e562803 ]

        SOLR-8902: Make sure ReturnFields only returns the requested fields

        Show
        jira-bot ASF subversion and git services added a comment - Commit e562803b3fe3d3f829d251228648f7c1a4da81bf in lucene-solr's branch refs/heads/branch_6_0 from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e562803 ] SOLR-8902 : Make sure ReturnFields only returns the requested fields
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 308b50816a6c89f9f4f474ed16f6358f88229709 in lucene-solr's branch refs/heads/branch_6_0 from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=308b508 ]

        SOLR-8902: fix glob test (put back the fields.clear())

        Show
        jira-bot ASF subversion and git services added a comment - Commit 308b50816a6c89f9f4f474ed16f6358f88229709 in lucene-solr's branch refs/heads/branch_6_0 from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=308b508 ] SOLR-8902 : fix glob test (put back the fields.clear())
        Hide
        steve_rowe Steve Rowe added a comment -

        Reopening to fix a 6.0.1 backporting problem (see test failure here: https://builds.apache.org/job/Lucene-Solr-SmokeRelease-6.0/14/): The [geo] transformer, which is used in a test for this bugfix, was introduced in SOLR-8814, which is landing in 6.1 and won't be backported to 6.0.1.

        Show
        steve_rowe Steve Rowe added a comment - Reopening to fix a 6.0.1 backporting problem (see test failure here: https://builds.apache.org/job/Lucene-Solr-SmokeRelease-6.0/14/ ): The [geo] transformer, which is used in a test for this bugfix, was introduced in SOLR-8814 , which is landing in 6.1 and won't be backported to 6.0.1.
        Hide
        steve_rowe Steve Rowe added a comment -

        Attached patch (branch_6_0 only) switches ReturnFieldsTest.testTransformers() to use the (test-only) [custom] transformer from TestCustomDocTransformer instead of the [geo] transformer. Like GeoTransformerFactory, CustomTransformerFactory overrides DocTransformer.getExtraRequestFields() to request Lucene fields that won't be (directly) returned in the response.

        I reverted the non-test changes introduced in this issue on branch_6_0, and with this patch, the assertion on line 272 fails properly:

        ReturnFieldsTest.testTransformers()
        268:    // Don't return 'text' just because it is required by the transformer
        269:    rf = new SolrReturnFields( req("fl", "[custom extra=text]") );
        270:    assertFalse( rf.wantsScore() );
        271:    assertTrue(rf.wantsField("[custom]"));
        272:    assertFalse( rf.wantsField( "text" ) );
        

        Committing to branch_6_0 shortly.

        Show
        steve_rowe Steve Rowe added a comment - Attached patch (branch_6_0 only) switches ReturnFieldsTest.testTransformers() to use the (test-only) [custom] transformer from TestCustomDocTransformer instead of the [geo] transformer. Like GeoTransformerFactory , CustomTransformerFactory overrides DocTransformer.getExtraRequestFields() to request Lucene fields that won't be (directly) returned in the response. I reverted the non-test changes introduced in this issue on branch_6_0, and with this patch, the assertion on line 272 fails properly: ReturnFieldsTest.testTransformers() 268: // Don't return 'text' just because it is required by the transformer 269: rf = new SolrReturnFields( req( "fl" , "[custom extra=text]" ) ); 270: assertFalse( rf.wantsScore() ); 271: assertTrue(rf.wantsField( "[custom]" )); 272: assertFalse( rf.wantsField( "text" ) ); Committing to branch_6_0 shortly.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit eb9985210ecc72d0bf6669e6002ac4f655e7e3c8 in lucene-solr's branch refs/heads/branch_6_0 from Steve Rowe
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=eb99852 ]

        SOLR-8902: branch_6_0 only: use [custom] transformer instead of [geo] transformer in ReturnFieldsTest, since the [geo] transformer, introduced in SOLR-8814 and landing in 6.1, won't be backported to branch_6_0

        Show
        jira-bot ASF subversion and git services added a comment - Commit eb9985210ecc72d0bf6669e6002ac4f655e7e3c8 in lucene-solr's branch refs/heads/branch_6_0 from Steve Rowe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=eb99852 ] SOLR-8902 : branch_6_0 only: use [custom] transformer instead of [geo] transformer in ReturnFieldsTest, since the [geo] transformer, introduced in SOLR-8814 and landing in 6.1, won't be backported to branch_6_0
        Hide
        steve_rowe Steve Rowe added a comment -

        Bulk close issues released in 6.0.1.

        Show
        steve_rowe Steve Rowe added a comment - Bulk close issues released in 6.0.1.
        Hide
        steve_rowe Steve Rowe added a comment -

        Reopening to backport to 5.6 and 5.5.2.

        Show
        steve_rowe Steve Rowe added a comment - Reopening to backport to 5.6 and 5.5.2.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 252acfebe13c2a9fa60ac81cb2c8e7afd9845876 in lucene-solr's branch refs/heads/branch_5_5 from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=252acfe ]

        SOLR-8902: Make sure ReturnFields only returns the requested fields

        Show
        jira-bot ASF subversion and git services added a comment - Commit 252acfebe13c2a9fa60ac81cb2c8e7afd9845876 in lucene-solr's branch refs/heads/branch_5_5 from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=252acfe ] SOLR-8902 : Make sure ReturnFields only returns the requested fields
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 5589f9b7e705aeefc038e1480b33ede01317b96a in lucene-solr's branch refs/heads/branch_5_5 from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5589f9b ]

        SOLR-8902: fix glob test (put back the fields.clear())

        Show
        jira-bot ASF subversion and git services added a comment - Commit 5589f9b7e705aeefc038e1480b33ede01317b96a in lucene-solr's branch refs/heads/branch_5_5 from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5589f9b ] SOLR-8902 : fix glob test (put back the fields.clear())
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit e9e55d1ef5fd3cccfd80281c5f66ec3486cb98f1 in lucene-solr's branch refs/heads/branch_5_5 from Steve Rowe
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e9e55d1 ]

        SOLR-8902: branch_6_0 only: use [custom] transformer instead of [geo] transformer in ReturnFieldsTest, since the [geo] transformer, introduced in SOLR-8814 and landing in 6.1, won't be backported to branch_6_0

        Show
        jira-bot ASF subversion and git services added a comment - Commit e9e55d1ef5fd3cccfd80281c5f66ec3486cb98f1 in lucene-solr's branch refs/heads/branch_5_5 from Steve Rowe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e9e55d1 ] SOLR-8902 : branch_6_0 only: use [custom] transformer instead of [geo] transformer in ReturnFieldsTest, since the [geo] transformer, introduced in SOLR-8814 and landing in 6.1, won't be backported to branch_6_0
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 489f386b766882d717d1bf5f740b81c179084d0e in lucene-solr's branch refs/heads/branch_5_5 from Steve Rowe
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=489f386 ]

        SOLR-8902: don't try to pull return fields from the ResultContext - this method, added in SOLR-7957, won't be backported to 5.x

        Show
        jira-bot ASF subversion and git services added a comment - Commit 489f386b766882d717d1bf5f740b81c179084d0e in lucene-solr's branch refs/heads/branch_5_5 from Steve Rowe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=489f386 ] SOLR-8902 : don't try to pull return fields from the ResultContext - this method, added in SOLR-7957 , won't be backported to 5.x
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 8a91a9339626a1d1a8136ece2e28d97fbc5a8d77 in lucene-solr's branch refs/heads/branch_5x from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8a91a93 ]

        SOLR-8902: Make sure ReturnFields only returns the requested fields

        Show
        jira-bot ASF subversion and git services added a comment - Commit 8a91a9339626a1d1a8136ece2e28d97fbc5a8d77 in lucene-solr's branch refs/heads/branch_5x from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8a91a93 ] SOLR-8902 : Make sure ReturnFields only returns the requested fields
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit cd8cccd228cbbf4ae807b4fda008a099f8ea8b19 in lucene-solr's branch refs/heads/branch_5x from Ryan McKinley
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=cd8cccd ]

        SOLR-8902: fix glob test (put back the fields.clear())

        Show
        jira-bot ASF subversion and git services added a comment - Commit cd8cccd228cbbf4ae807b4fda008a099f8ea8b19 in lucene-solr's branch refs/heads/branch_5x from Ryan McKinley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=cd8cccd ] SOLR-8902 : fix glob test (put back the fields.clear())
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit a409beecd0fb198466cc7874498446eab165d6fa in lucene-solr's branch refs/heads/branch_5x from Steve Rowe
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a409bee ]

        SOLR-8902: branch_6_0 only: use [custom] transformer instead of [geo] transformer in ReturnFieldsTest, since the [geo] transformer, introduced in SOLR-8814 and landing in 6.1, won't be backported to branch_6_0

        Show
        jira-bot ASF subversion and git services added a comment - Commit a409beecd0fb198466cc7874498446eab165d6fa in lucene-solr's branch refs/heads/branch_5x from Steve Rowe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a409bee ] SOLR-8902 : branch_6_0 only: use [custom] transformer instead of [geo] transformer in ReturnFieldsTest, since the [geo] transformer, introduced in SOLR-8814 and landing in 6.1, won't be backported to branch_6_0
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 0442aa5e5eca3a688fa1aa47b26d2f1aeaa5c994 in lucene-solr's branch refs/heads/branch_5x from Steve Rowe
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=0442aa5 ]

        SOLR-8902: don't try to pull return fields from the ResultContext - this method, added in SOLR-7957, won't be backported to 5.x

        Show
        jira-bot ASF subversion and git services added a comment - Commit 0442aa5e5eca3a688fa1aa47b26d2f1aeaa5c994 in lucene-solr's branch refs/heads/branch_5x from Steve Rowe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=0442aa5 ] SOLR-8902 : don't try to pull return fields from the ResultContext - this method, added in SOLR-7957 , won't be backported to 5.x

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development