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

Support dates and booleans in /export handler, support boolean DocValues fields

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.2, 7.0
    • Component/s: None
    • Labels:
      None

      Description

      Since these can be DV fields it seems like it should. The first-level problem is that SortingResponseWriter doesn't check for date types as a legal export value. Whether there are repercussions elsewhere I don't know yet.

      1. SOLR-9187.patch
        69 kB
        Erick Erickson
      2. SOLR-9187.patch
        67 kB
        Erick Erickson
      3. SOLR-9187.patch
        63 kB
        Erick Erickson

        Issue Links

          Activity

          Hide
          hossman Hoss Man added a comment -

          Since this change has already been released, commenting here on bugs it may have introduced isn't the best idea – too easy to get lost, and any fix has to be tracked in a new issue anyway.

          I've created SOLR-9490 to track these new reports from Colvin Cowie and Pavan Shetty

          Show
          hossman Hoss Man added a comment - Since this change has already been released, commenting here on bugs it may have introduced isn't the best idea – too easy to get lost, and any fix has to be tracked in a new issue anyway. I've created SOLR-9490 to track these new reports from Colvin Cowie and Pavan Shetty
          Hide
          pavan_shetty Pavan Shetty added a comment -

          As suggested in devel mailing list i am updating this here...

          Binary Response Writer Issue in solr version 6.2.0 :

          I downloaded solr version 6.2.0 (6.2.0 764d0f19151dbff6f5fcd9fc4b2682cf934590c5 - mike - 2016-08-20 05:41:37) and installed my core.

          In my schema.xml i have an field like following :

          <field name="stock_status" type="boolean" indexed="true" stored="true" multiValued="false"/>

          Now i am accessing this field using SolrJ (6.1.0). But i am always getting false value for above field even though it contains true boolean value. This is happening for all boolean fields.

          http://localhost:8983/solr...wt=javabin&version=2 HTTP/1.1

          It is working fine in other response writer.

          If i change the solr version to 6.1.0, with same SolrJ, it starts working. So clearly this is a bug in version 6.2.0.

          This is my first mail and issue report, so please correct me.

          Thanks,

          Pavan

          Show
          pavan_shetty Pavan Shetty added a comment - As suggested in devel mailing list i am updating this here... Binary Response Writer Issue in solr version 6.2.0 : I downloaded solr version 6.2.0 (6.2.0 764d0f19151dbff6f5fcd9fc4b2682cf934590c5 - mike - 2016-08-20 05:41:37) and installed my core. In my schema.xml i have an field like following : <field name="stock_status" type="boolean" indexed="true" stored="true" multiValued="false"/> Now i am accessing this field using SolrJ (6.1.0). But i am always getting false value for above field even though it contains true boolean value. This is happening for all boolean fields. http://localhost:8983/solr...wt=javabin&version=2 HTTP/1.1 It is working fine in other response writer. If i change the solr version to 6.1.0, with same SolrJ, it starts working. So clearly this is a bug in version 6.2.0. This is my first mail and issue report, so please correct me. Thanks, Pavan
          Hide
          cjcowie Colvin Cowie added a comment -

          Hi, I've just picked up 6.2.0. It seems that the change to toExternal() in BoolField now means that booleans without DocValues return null, which then turns into Boolean.FALSE in toObject() regardless of whether the value is true or false.

          e.g. with this schema, facet counts are correct, the returned values are wrong.

          <field name="f_EVE64" type="boolean" indexed="true" stored="true" required="false" multiValued="false"/>
          <fieldType name="boolean" class="solr.BoolField" sortMissingLast="true"/>
          
          "response":{"numFound":2,"start":0,"maxScore":1.0,"docs":[
                {
                  "id":"124",
                  "f_EVE64":false,
                  "_version_":1544828487600177152},
                {
                  "id":"123",
                  "f_EVE64":false,
                  "_version_":1544828492458229760}]
            },
            "facet_counts":{
              "facet_queries":{},
              "facet_fields":{
                "f_EVE64":[
                  "false",1,
                  "true",1]},
          

          Could toExternal() perhaps fallback to how it originally behaved? e.g.

          if (f.binaryValue() == null) {
                return indexedToReadable(f.stringValue());
          }
          
          Show
          cjcowie Colvin Cowie added a comment - Hi, I've just picked up 6.2.0. It seems that the change to toExternal() in BoolField now means that booleans without DocValues return null, which then turns into Boolean.FALSE in toObject() regardless of whether the value is true or false. e.g. with this schema, facet counts are correct, the returned values are wrong. <field name="f_EVE64" type="boolean" indexed="true" stored="true" required="false" multiValued="false"/> <fieldType name="boolean" class="solr.BoolField" sortMissingLast="true"/> "response":{"numFound":2,"start":0,"maxScore":1.0,"docs":[ { "id":"124", "f_EVE64":false, "_version_":1544828487600177152}, { "id":"123", "f_EVE64":false, "_version_":1544828492458229760}] }, "facet_counts":{ "facet_queries":{}, "facet_fields":{ "f_EVE64":[ "false",1, "true",1]}, Could toExternal() perhaps fallback to how it originally behaved? e.g. if (f.binaryValue() == null) { return indexedToReadable(f.stringValue()); }
          Hide
          mikemccand Michael McCandless added a comment -

          Bulk close resolved issues after 6.2.0 release.

          Show
          mikemccand Michael McCandless added a comment - Bulk close resolved issues after 6.2.0 release.
          Hide
          erickerickson Erick Erickson added a comment -

          Also fixes SOLR-7264

          Show
          erickerickson Erick Erickson added a comment - Also fixes SOLR-7264
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit bc86c1015c96a1764ae0909bac256ee2cd498dd0 in lucene-solr's branch refs/heads/branch_6x from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=bc86c10 ]

          SOLR-9187: Support dates and booleans in /export handler, support boolean DocValues fields

          Show
          jira-bot ASF subversion and git services added a comment - Commit bc86c1015c96a1764ae0909bac256ee2cd498dd0 in lucene-solr's branch refs/heads/branch_6x from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=bc86c10 ] SOLR-9187 : Support dates and booleans in /export handler, support boolean DocValues fields
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 844ca4a348e282b5f857aa7ce4de6f9781766ef9 in lucene-solr's branch refs/heads/master from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=844ca4a ]

          SOLR-9187: Support dates and booleans in /export handler, support boolean DocValues fields

          Show
          jira-bot ASF subversion and git services added a comment - Commit 844ca4a348e282b5f857aa7ce4de6f9781766ef9 in lucene-solr's branch refs/heads/master from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=844ca4a ] SOLR-9187 : Support dates and booleans in /export handler, support boolean DocValues fields
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit f1c74bb250fb285b9fb2fe1e96a361fd0ed92ab2 in lucene-solr's branch refs/heads/branch_6x from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f1c74bb ]

          SOLR-9187: Support dates and booleans in /export handler, support boolean DocValues fields

          Show
          jira-bot ASF subversion and git services added a comment - Commit f1c74bb250fb285b9fb2fe1e96a361fd0ed92ab2 in lucene-solr's branch refs/heads/branch_6x from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f1c74bb ] SOLR-9187 : Support dates and booleans in /export handler, support boolean DocValues fields
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 79d1b8c22768b6c902a4e880698733277796820c in lucene-solr's branch refs/heads/master from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=79d1b8c ]

          SOLR-9187: Support dates and booleans in /export handler, support boolean DocValues fields

          Show
          jira-bot ASF subversion and git services added a comment - Commit 79d1b8c22768b6c902a4e880698733277796820c in lucene-solr's branch refs/heads/master from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=79d1b8c ] SOLR-9187 : Support dates and booleans in /export handler, support boolean DocValues fields
          Hide
          romseygeek Alan Woodward added a comment -

          I think that code was always there, it's just been moved into its own method. Your solution sounds reasonable though.

          Show
          romseygeek Alan Woodward added a comment - I think that code was always there, it's just been moved into its own method. Your solution sounds reasonable though.
          Hide
          erickerickson Erick Erickson added a comment -

          All tests pass now, I'll be checking it in sometime this afteroon

          Show
          erickerickson Erick Erickson added a comment - All tests pass now, I'll be checking it in sometime this afteroon
          Hide
          erickerickson Erick Erickson added a comment -

          Alan Woodward: I'm about to commit this patch and running afoul of SOLR-9176.

          The root is this line in SimpleFacets:
          /* Always use filters for booleans... we know the number of values is very small. */
          if (type instanceof BoolField)

          For a DV-only field it defaults to FacetMethod.ENUM which doesn't work.

          I can get around it by something like this (still testing):
          if (type instanceof BoolField && (field.indexed() == true || field.hasDocValues() == false))

          I'm always suspicious of increasingly complex tests like this. So my question is whether this optimization should be removed? Or just keep it with additional clauses? I'm coming down on the side of leaving it in, but wanted to get your thoughts (or anyone else who wants to chime in of course).

          The other thing I wanted to check is whether it's better to compute facets from the indexed values or the DV fields when both are set. My supposition is that the ENUM method on the indexed fields is the better choice since it'll then cache the results.

          Show
          erickerickson Erick Erickson added a comment - Alan Woodward : I'm about to commit this patch and running afoul of SOLR-9176 . The root is this line in SimpleFacets: /* Always use filters for booleans... we know the number of values is very small. */ if (type instanceof BoolField) For a DV-only field it defaults to FacetMethod.ENUM which doesn't work. I can get around it by something like this (still testing): if (type instanceof BoolField && (field.indexed() == true || field.hasDocValues() == false)) I'm always suspicious of increasingly complex tests like this. So my question is whether this optimization should be removed? Or just keep it with additional clauses? I'm coming down on the side of leaving it in, but wanted to get your thoughts (or anyone else who wants to chime in of course). The other thing I wanted to check is whether it's better to compute facets from the indexed values or the DV fields when both are set. My supposition is that the ENUM method on the indexed fields is the better choice since it'll then cache the results.
          Hide
          erickerickson Erick Erickson added a comment -

          There was a problem with the previous patch, this one fixes it.

          I'm probably going to let this settle for a day or so then check it in unless there are objections.

          Show
          erickerickson Erick Erickson added a comment - There was a problem with the previous patch, this one fixes it. I'm probably going to let this settle for a day or so then check it in unless there are objections.
          Hide
          erickerickson Erick Erickson added a comment -

          Running through precommit and test now, I think it's ready if I get through those.

          Show
          erickerickson Erick Erickson added a comment - Running through precommit and test now, I think it's ready if I get through those.
          Hide
          erickerickson Erick Erickson added a comment -

          While looking at this I didn't see any reason the booleans shouldn't be exportable too, so while I was in the code for Dates I added Booleans.

          Show
          erickerickson Erick Erickson added a comment - While looking at this I didn't see any reason the booleans shouldn't be exportable too, so while I was in the code for Dates I added Booleans.
          Hide
          erickerickson Erick Erickson added a comment -

          Assigned to myself, but if anyone else wants to pick it up feel free.

          Show
          erickerickson Erick Erickson added a comment - Assigned to myself, but if anyone else wants to pick it up feel free.

            People

            • Assignee:
              erickerickson Erick Erickson
              Reporter:
              erickerickson Erick Erickson
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development