You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jena.apache.org by GitBox <gi...@apache.org> on 2022/08/29 16:02:56 UTC

[GitHub] [jena] Aklakan opened a new issue, #1498: Prologue not compatible with PrefixMapping2

Aklakan opened a new issue, #1498:
URL: https://github.com/apache/jena/issues/1498

   ### Version
   
   4.7.0-SNAPSHOT
   
   ### What happened?
   
   Preface: As mentioned e.g. at https://issues.apache.org/jira/browse/JENA-2309 we are using a prefix cc dump (~2.9K prefixes)
   Also, with the [LinkedSparqlQuery](http://lsq.aksw.org/), we are parsing logs of SPARQL queries. Often servers have predefined prefixes which do not appear in the logs so query parsing needs a "fragmented" approach with query string being parsed against a preconfigured set of prefixes. Because we want to parse million of queries we do not want to spend time uselessly coping billions of prefixes (all thousands prefixes for every query) - hence, an approach based on e.g. PrefixMapping2 is just what we want. I am mentioning this in order to avoid @afs  from performing a quick fix that seals off the APIs I need by e.g. changing all methods to always do the copying :)
   
   Unfortunately, the following breaks because Prologue setPrefix calls removeNsPrefix:
   
   ```java
   PrefixMapping pm = new PrefixMapping2(PrefixMapping.Extended);
   Query query = new Query();
   query.setPrefixMapping(pm);
   QueryFactory.parse(query, "PREFIX rdf: <http://foo.bar/baz/> SELECT * {}", null, Syntax.syntaxARQ);
   ```
   
   ### Relevant output and stacktrace
   
   ```shell
   Caused by: java.lang.UnsupportedOperationException: PrefixMapping2: prefix 'rdf' in the immutable map
   	at org.apache.jena.sparql.util.PrefixMapping2.removeNsPrefix(PrefixMapping2.java:68)
   	at org.apache.jena.sparql.core.Prologue.setPrefix(Prologue.java:122)
   	at org.apache.jena.sparql.lang.QueryParserBase.setPrefix(QueryParserBase.java:117)
   ```
   
   
   ### Are you interested in making a pull request?
   
   Maybe


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org
For additional commands, e-mail: issues-help@jena.apache.org


[GitHub] [jena] afs commented on issue #1498: Prologue not compatible with PrefixMapping2

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1498:
URL: https://github.com/apache/jena/issues/1498#issuecomment-1231686298

   > Hm, the advantage of PrefixMapping2 is that the internal 'added' map is again a PrefixMapping and the lookup inherts its performance characteristics.
   
   There is a `BufferingPrefixMapping` as well.
   
   > a solution that can deliver O(log(n)) is appreciated over a O(n) one
   
   I don't understand that.
   
   `PrefixMapBuffering`, `PrefixMapStd` and `BufferingPrefixMapping` are O(1) prefix lookup.
   
   > iri-to-prefix
   
   Parsing involves prefix-to-iri.
   
   > it's not exhilarating having to clutter my own projects with such rather basic stuff
   
   After its many years of evolution, sure Jena does not meet your ideals or the requirements of your day-job, and you can see better ways of of doing things.
   
   But Jena is what it is. Open source.
   
   Code has to be maintained over years. Code does not get written then remain unchanging.
   Maintenance work, questions from users, reviewing PRs, releases, security alerts ... rather basic stuff.
   
   Just recently, a lot of time has gone into the not-exhilarating work of cleaning up javadoc. Java changed. The javadoc was valid, and now (Java11) it isn't.
   
   Jena is not a dumping ground. No volunteer open source project can be.
   
   > the problem is not even related to PrefixMapping2 
   
   Quite. Did you try the fix?
   
   > to avoid @afs from performing a quick fix that seals off the APIs I need
   
   Don't make this personal.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org
For additional commands, e-mail: issues-help@jena.apache.org


[GitHub] [jena] Aklakan commented on issue #1498: Prologue not compatible with PrefixMapping2

Posted by GitBox <gi...@apache.org>.
Aklakan commented on issue #1498:
URL: https://github.com/apache/jena/issues/1498#issuecomment-1231588129

   Hm, the advantage of PrefixMapping2 is that the internal 'added' map is again a PrefixMapping and the lookup inherts its performance characteristics.
   
   BufferingPrefixMap only uses a plain HashMap for the additions which means iri-to-prefix lookups fall back to O(n)
   I guess in the presence of deletions there is not really a better way.
   
   So BufferingPrefixMap is not a replacement from a performance perspective. 
   
   > One-impl-does-everything can be hard to maintain.
   Yes, but
   - most importantly, I'd expect that in general use a solution that can deliver O(log(n)) is appreciated over a O(n) one and
   - implementations are not *that* complex and
   -  the existence of PrefixMapping2 (and the removed FastAbbreviatingPrefixMap) testifies that at least at some point these use case were relevant (and they *still* are for me).
   
   I would also be fine with doing a port of PrefixMapping2 to e.g. PrefixMap2. But it's not exhilarating having to clutter my own projects with such rather basic stuff (especially when it already existed).
   
   And actually the problem is not even related to PrefixMapping2 but a needless call to removeNsPrefix in Prologue that would break any PrefixMapping2-like approach:
   
   ```java
   // class Prologue:
               String oldExpansion = prefixMap.getNsPrefixURI(prefix) ;
               if ( Objects.equals(oldExpansion, expansion) )
                   return ;
               if ( oldExpansion != null ) // <- remove
                   prefixMap.removeNsPrefix(prefix) ; // <- remove
   
               prefixMap.setNsPrefix(prefix, expansion) ; // Isn't the contract of PrefixMap anyway that this should replace a prior mapping?
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org
For additional commands, e-mail: issues-help@jena.apache.org


[GitHub] [jena] afs commented on issue #1498: Prologue not compatible with PrefixMapping2

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1498:
URL: https://github.com/apache/jena/issues/1498#issuecomment-1231307271

   No code calls the PrefixMNapping2 constructors so we can be delete it.
   
   It says "Updates go to the local (second) copy only". It is (was) for locally extending prefixes mapping. In particular, it does not alter the global pmap which would be a concurrency problem.
   
   And if you'd let the code through, you'd find PrefixMapping.Extended is immutable. 
   
   
   The general solution is a buffering prefix map (records diffs). [BufferingPrefixMap](https://github.com/afs/jena-workspace/blob/main/src/main/java/dsg/buffering/BufferingPrefixMap.java).
   Sounds like what you want is the "add only" temporary extensions version of that.
   
   NB `PrefixMap` vs `PrefixMapping`.
     
   As a general point on [JENA-2309](https://issues.apache.org/jira/browse/JENA-2309) -- Jena has many interfaces. Tweaking implementations for one use case needs to be balanced with having clear code for general use (the majority of Jena users). One-impl-does-everything can be hard to maintain. Not everything is a bug.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org
For additional commands, e-mail: issues-help@jena.apache.org


[GitHub] [jena] afs closed issue #1498: Prologue not compatible with PrefixMapping2

Posted by GitBox <gi...@apache.org>.
afs closed issue #1498: Prologue not compatible with PrefixMapping2
URL: https://github.com/apache/jena/issues/1498


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org
For additional commands, e-mail: issues-help@jena.apache.org


[GitHub] [jena] Aklakan commented on issue #1498: Prologue not compatible with PrefixMapping2

Posted by GitBox <gi...@apache.org>.
Aklakan commented on issue #1498:
URL: https://github.com/apache/jena/issues/1498#issuecomment-1257213489

   > Don't make this personal.
   
   Apologies, that was inappropriate.
   
   > Quite. Did you try the fix?
   
   The PrefixMapping2 approach works with the PR. The change to Prologue by this PR should be valid anyway. If that class gets replaced with something more future proof I am of course fine with it.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org
For additional commands, e-mail: issues-help@jena.apache.org