Github user osma commented on the pull request:
https://github.com/apache/jena/pull/53#issuecomment-106801397
@amiara514 What's the status of this? Obviously there are now some merge
conflicts that need to be resolved, but other than that, do you think it's
ready for merging? I for one would like to see (a version of) this merged,
because it would help a lot in making efficient text index queries if you could
rely on the index not containing stale entries.
Looking at the diffs, I see there are some changes to jena-arq that don't
seem related to the main purpose of this code. Some of that is just cosmetic
changes like adjusting whitespace. Then there is some QueryVisitor code, and
some changes to ARQ tests. I doubt these patches are related to jena-text?
A couple of notes on the jena-text part:
1. You seem to always use the "uid" field to store the SHA-1 checksum.
However, all other field names in jena-text, including entityField to store the
URI, have so far been configurable using the assembler or EntityDefinition.
Should this field name be made configurable as well? Not having it set could
then mean "backwards compatible mode" where checksums are not stored and old
entries are thus not deleted.
2. You are using DigestUtils.shaHex() to calculate the checksum, but this
seems to be a deprecated method nowadays. I suggest using sha1Hex() or one of
the other more explicit variants instead.
3. I'm not sure TextQueryFuncs.getLiteralLanguage is a very useful method
(at least in its current form, with no error handling for null values etc), as
one could just do node.getLiteral().getLanguage() directly. But this is a minor
style issue.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
