You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by rvesse <gi...@git.apache.org> on 2018/02/12 14:22:41 UTC
[GitHub] jena pull request #360: Make sure to use datasets transactionally (JENA-1486...
GitHub user rvesse opened a pull request:
https://github.com/apache/jena/pull/360
Make sure to use datasets transactionally (JENA-1486)
textindexer did not use datasets transactionally which could fail for
dataset implementations that require transactional usage. And it is
safer to always use transactions where provided anyway.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rvesse/jena JENA-1486
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/360.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 #360
----
commit 0eeb40db8eb8b64617b232aaf8806769c00df964
Author: Rob Vesse <rv...@...>
Date: 2018-02-12T14:19:01Z
Make sure to use datasets transactionally (JENA-1486)
textindexer did not use datasets transactionally which could fail for
dataset implementations that require transactional usage. And it is
safer to always use transactions where provided anyway.
----
---
[GitHub] jena pull request #360: Make sure to use datasets transactionally (JENA-1486...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/360
---
[GitHub] jena issue #360: Make sure to use datasets transactionally (JENA-1486)
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/360
This is in the code base now.
Can we close it please?
To do it after the event, you can do an empty commit with message "This closes #360."
---
[GitHub] jena pull request #360: Make sure to use datasets transactionally (JENA-1486...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/360#discussion_r167613683
--- Diff: jena-text/src/main/java/jena/textindexer.java ---
@@ -104,39 +105,57 @@ protected String getSummary() {
@Override
protected void exec() {
- Set<Node> properties = getIndexedProperties() ;
-
- // there are various strategies possible here
- // what is implemented is a first cut simple approach
- // currently - for each indexed property
- // list and index triples with that property
- // that way only process triples that will be indexed
- // but each entity may be updated several times
-
- for ( Node property : properties )
- {
- Iterator<Quad> quadIter = dataset.find( Node.ANY, Node.ANY, property, Node.ANY );
- for (; quadIter.hasNext(); )
+ try {
+ // JENA-1486 Make sure to use transactions if supported
+ // The supportsTransactions() check should be strictly unecessary as we should always be using a
+ // DatasetGraphText which is transactional but just for future proofing we check anyway
+ if (dataset.supportsTransactions()) {
--- End diff --
Comment, rather than change request :
https://github.com/apache/jena/blob/master/jena-cmds/src/main/java/arq/query.java#L220
```
Transactional transactional = (dataset != null && dataset.supportsTransactions())
? dataset
: new TransactionalNull() ;
Txn.executeRead(transactional, ()->{
....
});
```
is an idiom that can be useful to (1) ensure the transaction is finished, and (2) avoid repeated `if (dataset.supportsTransactions())`.
---