Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JENA-1468: Retain variables in op (table) #345

Merged
merged 2 commits into from Jan 30, 2018
Merged

Conversation

afs
Copy link
Member

@afs afs commented Jan 27, 2018

No description provided.

@afs
Copy link
Member Author

afs commented Jan 27, 2018

Use new operation TableEmpty.isTableEmpty to test whether a data structure is the empty table, not rely on the contents. Thus a table of no rows but with declared variables, keeps the variables in the algebra.

@rvesse
Copy link
Member

rvesse commented Jan 29, 2018

Does

need to change to use the new test?

@afs
Copy link
Member Author

afs commented Jan 29, 2018

It does not need to change. It will behave as before.

The changes here differentiate between a table that is of class TableEmpty, using function TableEmpty.isTableEmpty, and a table that evaluates to empty (no rows) using method Table.isEmpty. Previously, there was only the latter method.

VALUES ?foo {} creates algebra TableN with declared variable ?foo and no rows. It printed (incorrectly) as (table empty) but internally it was a TableN and method isEmpty is true (no rows).

Now it prints as (table (vars ?foo)), which when read in again, creates the same TableN. There is no change to the data structure in the algebra.

The change this PR makes is on conversion to a string, and on .equals (then only if read in again) because it was not round-tripping the data structure, even if it evaluated to the same thing. Declared variables make no difference to the value of a table and that follows the SPARQL 1.1 spec exactly.

As it makes no eval difference, we haven't noticed before.

@RickMoynihan
Copy link

@afs This is fantastic thank you so much!

I've checked out the branch from your repo, built a local version and can confirm it fixes the issue for us.

Do you have any idea when this will be merged and a 3.7.0 release of JENA released?

@afs
Copy link
Member Author

afs commented Jan 29, 2018

@RickMoynihan : thanks for looking at it.

Merge : soon, because you provide independent checking. It will then be in the daily development build but that is not a formal release.

Release : this depends on people-time, and realistically, we can't make solid timelines. We do run "releasable master" means that the master branch is releasable most of the time so the dev builds are realistic.

The general project hope is every 3-4 months, though we had an out-of-step release at 3.6.0. That is not a commitment though!

@RickMoynihan
Copy link

Thanks for the info @afs, was really just checking incase there was a release planned in the next week or so.

Looks like I'll be deploying an internal build to our private artifact bucket then :-)

@rvesse
Copy link
Member

rvesse commented Jan 30, 2018

@afs Thanks for the more detailed explanation

+1 to merge

@asfgit asfgit merged commit 2586abf into apache:master Jan 30, 2018
asfgit pushed a commit that referenced this pull request Jan 30, 2018
@afs afs deleted the empty-table branch January 30, 2018 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants