You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by afs <gi...@git.apache.org> on 2018/06/10 20:20:20 UTC

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

GitHub user afs opened a pull request:

    https://github.com/apache/jena/pull/434

    JENA-1562: Fix for Graph.size() for TDB2 graphs

    

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

    $ git pull https://github.com/afs/jena tdb2-graph-size

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

    https://github.com/apache/jena/pull/434.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 #434
    
----
commit de34b1cd77333faf5b84765e626a065d313aec90
Author: Andy Seaborne <an...@...>
Date:   2018-06-10T08:52:18Z

    JENA-1562: Fix for Graph.size() for TDB2 graphs

----


---

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

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

    https://github.com/apache/jena/pull/434#discussion_r194269450
  
    --- Diff: jena-db/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/GraphViewSwitchable.java ---
    @@ -79,16 +83,34 @@ public TransactionHandler getTransactionHandler() {
         /** Return the {@code DatasetGraphSwitchable} we are viewing. */
         @Override
         public DatasetGraphSwitchable getDataset() {
    -        return getx() ;
    +        return getx();
         }
         
         /** Return the {@code Graph} from the underlying switchable.
          *  Do not hold onto this reference across switches. 
          */
         public Graph getGraph() {
    -        return getx().getGraph(getGraphName()) ;
    +        return getx().getGraph(getGraphName());
         }
     
    +    // Super uses find. Override to call GraphTDB.size()
    +    @Override
    +    protected int graphBaseSize() {
    +        if ( isDefaultGraph() )
    +            return getDSG().getDefaultGraphTDB().size();
    +        return getDSG().getGraphTDB(getGraphName()).size();
    +    }
    +
    +    private DatasetGraphTDB getDSG() {
    +        return ((DatasetGraphTDB)(getx().get()));    
    +    }
    +    
    +    private static Function<Tuple<NodeId>, Tuple<NodeId>> project4TupleTo3Tuple = item -> {
    --- End diff --
    
    This seems kind of out of place...


---

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

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

    https://github.com/apache/jena/pull/434#discussion_r194395266
  
    --- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/GraphView.java ---
    @@ -165,6 +165,9 @@ public void performDelete( Triple t ) {
             Node o = t.getObject() ;
             dsg.delete(g, s, p, o) ;
         }
    +    
    +    // Subclasses may wish to provide graphBaseSize otherwise GraphBase uses find()  
    --- End diff --
    
    Done.


---

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

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

    https://github.com/apache/jena/pull/434


---

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

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

    https://github.com/apache/jena/pull/434#discussion_r194408181
  
    --- Diff: jena-db/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/GraphViewSwitchable.java ---
    @@ -79,16 +83,34 @@ public TransactionHandler getTransactionHandler() {
         /** Return the {@code DatasetGraphSwitchable} we are viewing. */
         @Override
         public DatasetGraphSwitchable getDataset() {
    -        return getx() ;
    +        return getx();
         }
         
         /** Return the {@code Graph} from the underlying switchable.
          *  Do not hold onto this reference across switches. 
          */
         public Graph getGraph() {
    -        return getx().getGraph(getGraphName()) ;
    +        return getx().getGraph(getGraphName());
         }
     
    +    // Super uses find. Override to call GraphTDB.size()
    +    @Override
    +    protected int graphBaseSize() {
    +        if ( isDefaultGraph() )
    +            return getDSG().getDefaultGraphTDB().size();
    +        return getDSG().getGraphTDB(getGraphName()).size();
    +    }
    +
    +    private DatasetGraphTDB getDSG() {
    +        return ((DatasetGraphTDB)(getx().get()));    
    +    }
    +    
    +    private static Function<Tuple<NodeId>, Tuple<NodeId>> project4TupleTo3Tuple = item -> {
    --- End diff --
    
    OK - got it - it's a stray from GraphTDB. This one can be cleaned out.


---

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

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

    https://github.com/apache/jena/pull/434#discussion_r194269455
  
    --- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/GraphView.java ---
    @@ -165,6 +165,9 @@ public void performDelete( Triple t ) {
             Node o = t.getObject() ;
             dsg.delete(g, s, p, o) ;
         }
    +    
    +    // Subclasses may wish to provide graphBaseSize otherwise GraphBase uses find()  
    --- End diff --
    
    Maybe this is just where I like to find things, but I'd rather `@Override protected int graphBaseSize() { return super.graphBaseSize(); }` with this as a Javadoc comment. 


---

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

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

    https://github.com/apache/jena/pull/434#discussion_r194398578
  
    --- Diff: jena-db/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/GraphViewSwitchable.java ---
    @@ -79,16 +83,34 @@ public TransactionHandler getTransactionHandler() {
         /** Return the {@code DatasetGraphSwitchable} we are viewing. */
         @Override
         public DatasetGraphSwitchable getDataset() {
    -        return getx() ;
    +        return getx();
         }
         
         /** Return the {@code Graph} from the underlying switchable.
          *  Do not hold onto this reference across switches. 
          */
         public Graph getGraph() {
    -        return getx().getGraph(getGraphName()) ;
    +        return getx().getGraph(getGraphName());
         }
     
    +    // Super uses find. Override to call GraphTDB.size()
    +    @Override
    +    protected int graphBaseSize() {
    +        if ( isDefaultGraph() )
    +            return getDSG().getDefaultGraphTDB().size();
    +        return getDSG().getGraphTDB(getGraphName()).size();
    +    }
    +
    +    private DatasetGraphTDB getDSG() {
    +        return ((DatasetGraphTDB)(getx().get()));    
    +    }
    +    
    +    private static Function<Tuple<NodeId>, Tuple<NodeId>> project4TupleTo3Tuple = item -> {
    --- End diff --
    
    I'm totally confused. The method above reads:
    ```
    private DatasetGraphTDB getDSG() {
            return ((DatasetGraphTDB)(getx().get()));    
    }
    ```
    Where is `project4TupleTo3Tuple` used there?


---

[GitHub] jena pull request #434: JENA-1562: Fix for Graph.size() for TDB2 graphs

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

    https://github.com/apache/jena/pull/434#discussion_r194396180
  
    --- Diff: jena-db/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/GraphViewSwitchable.java ---
    @@ -79,16 +83,34 @@ public TransactionHandler getTransactionHandler() {
         /** Return the {@code DatasetGraphSwitchable} we are viewing. */
         @Override
         public DatasetGraphSwitchable getDataset() {
    -        return getx() ;
    +        return getx();
         }
         
         /** Return the {@code Graph} from the underlying switchable.
          *  Do not hold onto this reference across switches. 
          */
         public Graph getGraph() {
    -        return getx().getGraph(getGraphName()) ;
    +        return getx().getGraph(getGraphName());
         }
     
    +    // Super uses find. Override to call GraphTDB.size()
    +    @Override
    +    protected int graphBaseSize() {
    +        if ( isDefaultGraph() )
    +            return getDSG().getDefaultGraphTDB().size();
    +        return getDSG().getGraphTDB(getGraphName()).size();
    +    }
    +
    +    private DatasetGraphTDB getDSG() {
    +        return ((DatasetGraphTDB)(getx().get()));    
    +    }
    +    
    +    private static Function<Tuple<NodeId>, Tuple<NodeId>> project4TupleTo3Tuple = item -> {
    --- End diff --
    
    Don't understand.  It is used in the method above and pulled out for clarity. It makes the handling of union graph clearer IMO with a named function at that point, not inline code.


---