You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@rya.apache.org by kchilton2 <gi...@git.apache.org> on 2016/08/10 22:52:32 UTC

[GitHub] incubator-rya pull request #65: RYA-148 Fixed a bug where Joins would write ...

GitHub user kchilton2 opened a pull request:

    https://github.com/apache/incubator-rya/pull/65

    RYA-148 Fixed a bug where Joins would write the same binding set with\u2026

    \u2026 different visibility orders depending on which child node the new result appeared on.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kchilton2/incubator-rya RYA-148

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-rya/pull/65.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #65
    
----
commit b7d3f80ab0ffa0a39eba110da7011613aa7be862
Author: Kevin Chilton <ke...@parsons.com>
Date:   2016-08-10T22:47:35Z

    RYA-148 Fixed a bug where Joins would write the same binding set with different visibility orders depending on which child node the new result appeared on.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rya pull request #65: RYA-148 Fixed a bug where Joins would write ...

Posted by kchilton2 <gi...@git.apache.org>.
Github user kchilton2 commented on a diff in the pull request:

    https://github.com/apache/incubator-rya/pull/65#discussion_r74608490
  
    --- Diff: extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java ---
    @@ -420,10 +423,20 @@ public VisibilityBindingSet next() {
                     bs.addBinding(binding);
                 }
     
    +            // We want to make sure the visibilities are always written the same way,
    +            // so figure out which are on the left side and which are on the right side.
    +            final String leftVisi;
    +            final String rightVisi;
    +            if(newResultSide == Side.LEFT) {
    +                leftVisi = newResult.getVisibility();
    +                rightVisi = joinResult.getVisibility();
    +            } else {
    +                leftVisi = joinResult.getVisibility();
    +                rightVisi = newResult.getVisibility();
    +            }
    +
                 String visibility = "";
                 final Joiner join = Joiner.on(")&(");
    -            final String leftVisi = newResult.getVisibility();
    -            final String rightVisi = joinResult.getVisibility();
    --- End diff --
    
    Using flatten makes a bunch of unit tests fail, so I'd rather address that in a different ticket.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rya issue #65: RYA-148 Fixed a bug where Joins would write the sam...

Posted by amihalik <gi...@git.apache.org>.
Github user amihalik commented on the issue:

    https://github.com/apache/incubator-rya/pull/65
  
    Merged.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rya pull request #65: RYA-148 Fixed a bug where Joins would write ...

Posted by amihalik <gi...@git.apache.org>.
Github user amihalik commented on a diff in the pull request:

    https://github.com/apache/incubator-rya/pull/65#discussion_r74498673
  
    --- Diff: extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java ---
    @@ -420,10 +423,20 @@ public VisibilityBindingSet next() {
                     bs.addBinding(binding);
                 }
     
    +            // We want to make sure the visibilities are always written the same way,
    +            // so figure out which are on the left side and which are on the right side.
    +            final String leftVisi;
    +            final String rightVisi;
    +            if(newResultSide == Side.LEFT) {
    +                leftVisi = newResult.getVisibility();
    +                rightVisi = joinResult.getVisibility();
    +            } else {
    +                leftVisi = joinResult.getVisibility();
    +                rightVisi = newResult.getVisibility();
    +            }
    +
                 String visibility = "";
                 final Joiner join = Joiner.on(")&(");
    -            final String leftVisi = newResult.getVisibility();
    -            final String rightVisi = joinResult.getVisibility();
    --- End diff --
    
    Consider adding ColumnVisibility.flatten() to simplify the visibility expression.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rya pull request #65: RYA-148 Fixed a bug where Joins would write ...

Posted by kchilton2 <gi...@git.apache.org>.
Github user kchilton2 commented on a diff in the pull request:

    https://github.com/apache/incubator-rya/pull/65#discussion_r74602355
  
    --- Diff: extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java ---
    @@ -420,10 +423,20 @@ public VisibilityBindingSet next() {
                     bs.addBinding(binding);
                 }
     
    +            // We want to make sure the visibilities are always written the same way,
    +            // so figure out which are on the left side and which are on the right side.
    +            final String leftVisi;
    +            final String rightVisi;
    +            if(newResultSide == Side.LEFT) {
    +                leftVisi = newResult.getVisibility();
    +                rightVisi = joinResult.getVisibility();
    +            } else {
    +                leftVisi = joinResult.getVisibility();
    +                rightVisi = newResult.getVisibility();
    +            }
    +
                 String visibility = "";
                 final Joiner join = Joiner.on(")&(");
    -            final String leftVisi = newResult.getVisibility();
    -            final String rightVisi = joinResult.getVisibility();
    --- End diff --
    
    Will do.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rya pull request #65: RYA-148 Fixed a bug where Joins would write ...

Posted by kchilton2 <gi...@git.apache.org>.
Github user kchilton2 closed the pull request at:

    https://github.com/apache/incubator-rya/pull/65


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rya issue #65: RYA-148 Fixed a bug where Joins would write the sam...

Posted by isper3at <gi...@git.apache.org>.
Github user isper3at commented on the issue:

    https://github.com/apache/incubator-rya/pull/65
  
    :+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rya pull request #65: RYA-148 Fixed a bug where Joins would write ...

Posted by isper3at <gi...@git.apache.org>.
Github user isper3at commented on a diff in the pull request:

    https://github.com/apache/incubator-rya/pull/65#discussion_r74458400
  
    --- Diff: extras/rya.pcj.fluo/pcj.fluo.app/src/test/java/org/apache/rya/indexing/pcj/fluo/app/IterativeJoinTest.java ---
    @@ -0,0 +1,88 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.rya.indexing.pcj.fluo.app;
    +
    +import static org.junit.Assert.assertEquals;
    +
    +import java.util.Arrays;
    +import java.util.Collection;
    +import java.util.Collections;
    +import java.util.Iterator;
    +
    +import org.apache.rya.indexing.pcj.fluo.app.JoinResultUpdater.IterativeJoin;
    +import org.apache.rya.indexing.pcj.fluo.app.JoinResultUpdater.LeftOuterJoin;
    +import org.apache.rya.indexing.pcj.fluo.app.JoinResultUpdater.NaturalJoin;
    +import org.apache.rya.indexing.pcj.storage.accumulo.VisibilityBindingSet;
    +import org.junit.Test;
    +import org.junit.runner.RunWith;
    +import org.junit.runners.Parameterized;
    +import org.junit.runners.Parameterized.Parameter;
    +import org.junit.runners.Parameterized.Parameters;
    +import org.openrdf.model.ValueFactory;
    +import org.openrdf.model.impl.ValueFactoryImpl;
    +import org.openrdf.query.impl.MapBindingSet;
    +
    +/**
    + * Tests the methods of {@link IterativeJoin}.
    + */
    +@RunWith(Parameterized.class)
    +public class IterativeJoinTest {
    +
    +    @Parameters
    +    public static Collection<Object[]> data() {
    +        return Arrays.asList(new Object[][] {
    --- End diff --
    
    this just feels.....odd


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---