You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by xristy <gi...@git.apache.org> on 2017/11/29 15:30:41 UTC

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

GitHub user xristy opened a pull request:

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

    JENA-1438 rdf:langString args

    JENA-1438 allow rdf:langString arguments to text:query

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

    $ git pull https://github.com/BuddhistDigitalResourceCenter/jena JENA-1438-rdflangString

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

    https://github.com/apache/jena/pull/320.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 #320
    
----
commit eff7e533c3ee48a7413bd1f5c248f9dfea09ac78
Author: Chris Tomlinson <ct...@moonvine.org>
Date:   2017-11-29T15:28:05Z

    JENA-1438 rdf:langString args
    
    JENA-1438 allow rdf:langString arguments to text:query

----


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r154109219
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -319,13 +311,13 @@ private StrMatch objectToStruct(PropFuncArg argObject, boolean executionTime) {
     
                 String lang = o.getLiteralLanguage();
                 RDFDatatype dt = o.getLiteralDatatype() ;
    -            if (noLang(lang)) {
    +            if (lang.isEmpty()) {
                     if (dt != null && dt != XSDDatatype.XSDstring) {
                         log.warn("Object to text query is not a string") ;
                         return null ;
                     }
                 }
    -            lang = fixLang(lang);
    +            lang = lang.isEmpty() ? null : lang;
    --- End diff --
    
    I like [`emptyToNull`](https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Strings.html#emptyToNull(java.lang.String)) for this, but this isn't a problem or anything.


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r154325977
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -309,14 +310,18 @@ private StrMatch objectToStruct(PropFuncArg argObject, boolean executionTime) {
                     return null ;
                 }
     
    +            String lang = o.getLiteralLanguage();
                 RDFDatatype dt = o.getLiteralDatatype() ;
    -            if (dt != null && dt != XSDDatatype.XSDstring) {
    -                log.warn("Object to text query is not a string") ;
    -                return null ;
    +            if (lang.isEmpty()) {
    --- End diff --
    
    Comment: Testing for datatype `RDF.dtLangString` is also possible.


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r154028330
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -295,6 +295,14 @@ private String chooseGraphURI(ExecutionContext execCxt) {
             return results;
         }
         
    +    private boolean noLang(String lang) {
    +        return (lang == null || lang.isEmpty() || " ".equals(lang));
    --- End diff --
    
    Yes - no lang tag is a return of empty string.  If you get `<SPC>`, there is a bug somewhere.


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r153824555
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -295,6 +295,14 @@ private String chooseGraphURI(ExecutionContext execCxt) {
             return results;
         }
         
    +    private boolean noLang(String lang) {
    +        return (lang == null || lang.isEmpty() || " ".equals(lang));
    --- End diff --
    
    I don't know much about the text indexing, but this single space-- does that have some special meaning, or is it just a simple check for whitespace? I.e., could we see more than one space char here?


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r153872049
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -295,6 +295,14 @@ private String chooseGraphURI(ExecutionContext execCxt) {
             return results;
         }
         
    +    private boolean noLang(String lang) {
    +        return (lang == null || lang.isEmpty() || " ".equals(lang));
    --- End diff --
    
    The single space is the response from `o.getLiteralLanguage()` where `Node o` and `o.isLiteral()` at lines 344 and 386 of `TextQueryPF.java`. It appears to be the response when the literal has no explicit language tag. I thought the response was supposed to be an empty string, but the single space is what I was getting.


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r154112909
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -319,13 +311,13 @@ private StrMatch objectToStruct(PropFuncArg argObject, boolean executionTime) {
     
                 String lang = o.getLiteralLanguage();
                 RDFDatatype dt = o.getLiteralDatatype() ;
    -            if (noLang(lang)) {
    +            if (lang.isEmpty()) {
                     if (dt != null && dt != XSDDatatype.XSDstring) {
                         log.warn("Object to text query is not a string") ;
                         return null ;
                     }
                 }
    -            lang = fixLang(lang);
    +            lang = lang.isEmpty() ? null : lang;
    --- End diff --
    
    Agreed and changed. Thanks


---

[GitHub] jena issue #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320
  
    Thank you for approving. I'll be traveling this coming week and won't be able to add test cases until week after at the earliest. I'd like to fold that into a future issue if that works.


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r154031380
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -295,6 +295,14 @@ private String chooseGraphURI(ExecutionContext execCxt) {
             return results;
         }
         
    +    private boolean noLang(String lang) {
    +        return (lang == null || lang.isEmpty() || " ".equals(lang));
    --- End diff --
    
    Other than this issue (" " for lang tag), the rest looks good.


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r154326766
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -347,11 +352,15 @@ private StrMatch objectToStruct(PropFuncArg argObject, boolean executionTime) {
                     log.warn("Text query string is not a literal " + list) ;
                 return null ;
             }
    -        
    -        if (x.getLiteralDatatype() != null && !x.getLiteralDatatype().equals(XSDDatatype.XSDstring)) {
    --- End diff --
    
    Comment:
    In RDF 1.1, the data type of a literal is never null.
    
    There are functions like `Util.isLangString` (in RDF 1.1: rdf:langString) and `Util.isSimpleString` (in RDF 1.1, xsd:string) that mask the RDF1.0/RDF 1.1 mode setting (not that Jena claims to support RDF 1.0 any more).


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

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


---

[GitHub] jena pull request #320: JENA-1438 rdf:langString args

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

    https://github.com/apache/jena/pull/320#discussion_r154380929
  
    --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
    @@ -347,11 +352,15 @@ private StrMatch objectToStruct(PropFuncArg argObject, boolean executionTime) {
                     log.warn("Text query string is not a literal " + list) ;
                 return null ;
             }
    -        
    -        if (x.getLiteralDatatype() != null && !x.getLiteralDatatype().equals(XSDDatatype.XSDstring)) {
    --- End diff --
    
    Thank you for the tips. I tried to avoid changing code that was present. I plan to raise another issue or two in jena-text and will be glad to visit some of these cases to try and use RDF functionality rather than bare Java functionality. It certainly will make the code clearer.


---