Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-7609

Refactor expressions module to use DoubleValuesSource

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.4
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      With DoubleValuesSource in core, we can refactor the expressions module to use these instead of ValueSource, and remove the dependency of expressions on the queries module in master.

      1. LUCENE-7609.patch
        52 kB
        Alan Woodward
      2. LUCENE-7609.patch
        54 kB
        Alan Woodward

        Activity

        Hide
        romseygeek Alan Woodward added a comment -

        Patch. The dependency on queries is kept for the moment, and the ValueSource versions of methods deprecated; these will be removed in a subsequent patch.

        Show
        romseygeek Alan Woodward added a comment - Patch. The dependency on queries is kept for the moment, and the ValueSource versions of methods deprecated; these will be removed in a subsequent patch.
        Hide
        jpountz Adrien Grand added a comment -

        The dependency on queries is kept for the moment, and the ValueSource versions of methods deprecated; these will be removed in a subsequent patch.

        Why didn't you remove them in this patch? This change is breaking anyway, so maybe we should make things clean right away? For instance, I'm looking at the deprecated SimpleBindings.getValueSource which is not expected to be called by users of the expression module, so maybe we should remove it now?

        Show
        jpountz Adrien Grand added a comment - The dependency on queries is kept for the moment, and the ValueSource versions of methods deprecated; these will be removed in a subsequent patch. Why didn't you remove them in this patch? This change is breaking anyway, so maybe we should make things clean right away? For instance, I'm looking at the deprecated SimpleBindings.getValueSource which is not expected to be called by users of the expression module, so maybe we should remove it now?
        Hide
        jpountz Adrien Grand added a comment -

        Also this change seems to be changing expression values to not have a value if any of the sub sources does not have a value while in the past it looks like all documents would have a value and we would treat sub sources that do not have a value as zero. I think we should either preserve this behaviour or document the new one?

        Show
        jpountz Adrien Grand added a comment - Also this change seems to be changing expression values to not have a value if any of the sub sources does not have a value while in the past it looks like all documents would have a value and we would treat sub sources that do not have a value as zero. I think we should either preserve this behaviour or document the new one?
        Hide
        romseygeek Alan Woodward added a comment -

        Updated patch:

        • Removes the queries/ValueSource dependency entirely - I had thought that we could keep this backwards-compatible, but there's a breaking change in the Bindings public signature, so we may as well go all in.
        • Removes the ValueSource.asDoubleValuesSource() helper method - will put this in separately with LUCENE-7610 as it's no longer tested by exercising the expressions
        • Changes subsources to return 0 if they have no value. I tried making this configurable but couldn't find a nice way of doing it - something for a later ticket maybe.
        Show
        romseygeek Alan Woodward added a comment - Updated patch: Removes the queries/ValueSource dependency entirely - I had thought that we could keep this backwards-compatible, but there's a breaking change in the Bindings public signature, so we may as well go all in. Removes the ValueSource.asDoubleValuesSource() helper method - will put this in separately with LUCENE-7610 as it's no longer tested by exercising the expressions Changes subsources to return 0 if they have no value. I tried making this configurable but couldn't find a nice way of doing it - something for a later ticket maybe.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 776087eef48dbeba639b94b574f806b7265a7ffe in lucene-solr's branch refs/heads/branch_6x from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=776087e ]

        LUCENE-7609: Refactor expressions module to use DoubleValuesSource

        Show
        jira-bot ASF subversion and git services added a comment - Commit 776087eef48dbeba639b94b574f806b7265a7ffe in lucene-solr's branch refs/heads/branch_6x from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=776087e ] LUCENE-7609 : Refactor expressions module to use DoubleValuesSource
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 8b055382d6c88acaed9fe472a038c7ee6b35c016 in lucene-solr's branch refs/heads/master from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8b05538 ]

        LUCENE-7609: Refactor expressions module to use DoubleValuesSource

        Show
        jira-bot ASF subversion and git services added a comment - Commit 8b055382d6c88acaed9fe472a038c7ee6b35c016 in lucene-solr's branch refs/heads/master from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8b05538 ] LUCENE-7609 : Refactor expressions module to use DoubleValuesSource
        Hide
        romseygeek Alan Woodward added a comment -

        Thanks for the reviews Adrien!

        Show
        romseygeek Alan Woodward added a comment - Thanks for the reviews Adrien!
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1e55c5effee994856033ce115daabb2e12c91459 in lucene-solr's branch refs/heads/branch_6x from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1e55c5e ]

        LUCENE-7609: Add explicit dependency on queries to suggester

        This was previously included transitively through expressions, but expressions
        no longer depends on queries

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1e55c5effee994856033ce115daabb2e12c91459 in lucene-solr's branch refs/heads/branch_6x from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1e55c5e ] LUCENE-7609 : Add explicit dependency on queries to suggester This was previously included transitively through expressions, but expressions no longer depends on queries

          People

          • Assignee:
            romseygeek Alan Woodward
            Reporter:
            romseygeek Alan Woodward
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development