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

hashJoin does not work when "on" maps fields with "="

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 6.0
    • Fix Version/s: 6.0.1, 6.1, 7.0
    • Component/s: None
    • Labels:
      None

      Description

      hashJoin does not work when "on" maps fields with "="
      eg.
      hashJoin(
      ...
      on="field1=field2"
      )

      See link for fix.

        Activity

        Hide
        joel.bernstein Joel Bernstein added a comment - - edited

        Thanks. I just checked and there isn't a test case for this scenario.

        Show
        joel.bernstein Joel Bernstein added a comment - - edited Thanks. I just checked and there isn't a test case for this scenario.
        Hide
        joel.bernstein Joel Bernstein added a comment - - edited

        Dennis Gove, you want to take this one? Or do you want me to take a look?

        Show
        joel.bernstein Joel Bernstein added a comment - - edited Dennis Gove , you want to take this one? Or do you want me to take a look?
        Hide
        dpgove Dennis Gove added a comment -

        I'll take this. I like the fix but want to see if there's an easy way to do the splitting once (during init) instead of on each read - we'll get slightly better performance with that.

        Show
        dpgove Dennis Gove added a comment - I'll take this. I like the fix but want to see if there's an easy way to do the splitting once (during init) instead of on each read - we'll get slightly better performance with that.
        Hide
        dpgove Dennis Gove added a comment -

        Note - need to check if this fix is also necessary for outerHash. I suspect it is.

        Show
        dpgove Dennis Gove added a comment - Note - need to check if this fix is also necessary for outerHash. I suspect it is.
        Hide
        Osthold Stephan Osthold added a comment -

        In regard to splitting only once, if you don't mind changing the parameters for the constructors, I'd do it like this:

        https://github.com/StephanOsthold/lucene-solr/commit/c0749f0aa6587d70db3bdbb2ad6a86d2901ed72f

        The lists are private, because I need to make sure they have the same size.

        Show
        Osthold Stephan Osthold added a comment - In regard to splitting only once, if you don't mind changing the parameters for the constructors, I'd do it like this: https://github.com/StephanOsthold/lucene-solr/commit/c0749f0aa6587d70db3bdbb2ad6a86d2901ed72f The lists are private, because I need to make sure they have the same size.
        Hide
        Osthold Stephan Osthold added a comment -

        OuterHashJoin inherits from HashJoin. I did the required changes and the tests pass, however I only did a very brief manual test.

        Show
        Osthold Stephan Osthold added a comment - OuterHashJoin inherits from HashJoin. I did the required changes and the tests pass, however I only did a very brief manual test.
        Hide
        dpgove Dennis Gove added a comment - - edited

        This patch fixes up both hashJoin and outerHashJoin and adds tests for the scenario to both.

        The approach taken is to have a left and right list of fields to hash on. During construction it checks for an = in the field and if found will split into a left and right side field name. If not found then it uses that single field name for both the left and right side. It then uses those lists when reading tuples from the stream.

        Stephan Osthold, good catch on this one and thank you for the test showing the failure (I swiped that and included it in the test case).

        Show
        dpgove Dennis Gove added a comment - - edited This patch fixes up both hashJoin and outerHashJoin and adds tests for the scenario to both. The approach taken is to have a left and right list of fields to hash on. During construction it checks for an = in the field and if found will split into a left and right side field name. If not found then it uses that single field name for both the left and right side. It then uses those lists when reading tuples from the stream. Stephan Osthold , good catch on this one and thank you for the test showing the failure (I swiped that and included it in the test case).
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit c7929f8b851dd12d3ae1b9834058428394821790 in lucene-solr's branch refs/heads/master from Dennis Gove
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c7929f8 ]

        SOLR-9058: Makes HashJoinStream and OuterHashJoinStream support different field names in the incoming streams, eg. fieldA=fieldB

        Show
        jira-bot ASF subversion and git services added a comment - Commit c7929f8b851dd12d3ae1b9834058428394821790 in lucene-solr's branch refs/heads/master from Dennis Gove [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c7929f8 ] SOLR-9058 : Makes HashJoinStream and OuterHashJoinStream support different field names in the incoming streams, eg. fieldA=fieldB
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit d95a91a9cca341d7633d339bf56b08ecd59d1c2a in lucene-solr's branch refs/heads/branch_6x from Dennis Gove
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d95a91a ]

        SOLR-9058: Makes HashJoinStream and OuterHashJoinStream support different field names in the incoming streams, eg. fieldA=fieldB

        Show
        jira-bot ASF subversion and git services added a comment - Commit d95a91a9cca341d7633d339bf56b08ecd59d1c2a in lucene-solr's branch refs/heads/branch_6x from Dennis Gove [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d95a91a ] SOLR-9058 : Makes HashJoinStream and OuterHashJoinStream support different field names in the incoming streams, eg. fieldA=fieldB
        Hide
        hossman Hoss Man added a comment -

        Dennis Gove - should this be resolved?

        Show
        hossman Hoss Man added a comment - Dennis Gove - should this be resolved?
        Hide
        dpgove Dennis Gove added a comment -

        Yeah, it can be. Have a question about the fix versions though. This has been committed to master and branch_6x. My understanding is that means it's going to hit 6.1. Is that right? Is there another branch I should backport it to?

        Show
        dpgove Dennis Gove added a comment - Yeah, it can be. Have a question about the fix versions though. This has been committed to master and branch_6x. My understanding is that means it's going to hit 6.1. Is that right? Is there another branch I should backport it to?
        Hide
        joel.bernstein Joel Bernstein added a comment -

        Yes if it's in 6x it will be in 6.1.

        If you wanted to add this to 6.0.1 you'd have to back port to branch_6_0.

        Show
        joel.bernstein Joel Bernstein added a comment - Yes if it's in 6x it will be in 6.1. If you wanted to add this to 6.0.1 you'd have to back port to branch_6_0.
        Hide
        erickerickson Erick Erickson added a comment -

        I changed the fix versions to reflect the actual pushes. If you want it to go into 6.0.1, feel free to backport to 6_0 and update the fix versions....

        Whether you want to backport depends, usually that's reserved for those bugs you consider critical to the release. And there's no guarantee at all that there will ever be a 6.0.1. Up to you....

        Show
        erickerickson Erick Erickson added a comment - I changed the fix versions to reflect the actual pushes. If you want it to go into 6.0.1, feel free to backport to 6_0 and update the fix versions.... Whether you want to backport depends, usually that's reserved for those bugs you consider critical to the release. And there's no guarantee at all that there will ever be a 6.0.1. Up to you....
        Hide
        dpgove Dennis Gove added a comment -

        Closing this. The fix will appear in 6.1. Thanks everyone for the help.

        Show
        dpgove Dennis Gove added a comment - Closing this. The fix will appear in 6.1. Thanks everyone for the help.
        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 fbf7d5446d3cdfda0c3906c274ac2c444b419a82 in lucene-solr's branch refs/heads/branch_6_0 from Dennis Gove
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fbf7d54 ]

        SOLR-9058: Makes HashJoinStream and OuterHashJoinStream support different field names in the incoming streams, eg. fieldA=fieldB

        Show
        jira-bot ASF subversion and git services added a comment - Commit fbf7d5446d3cdfda0c3906c274ac2c444b419a82 in lucene-solr's branch refs/heads/branch_6_0 from Dennis Gove [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fbf7d54 ] SOLR-9058 : Makes HashJoinStream and OuterHashJoinStream support different field names in the incoming streams, eg. fieldA=fieldB
        Hide
        steve_rowe Steve Rowe added a comment -

        Bulk close issues included in the 6.0.1 release.

        Show
        steve_rowe Steve Rowe added a comment - Bulk close issues included in the 6.0.1 release.

          People

          • Assignee:
            dpgove Dennis Gove
            Reporter:
            Osthold Stephan Osthold
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development