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

Separation dot without space and prefixed name #250

Closed
wants to merge 1 commit into from
Closed

Separation dot without space and prefixed name #250

wants to merge 1 commit into from

Conversation

niklas88
Copy link
Member

@niklas88 niklas88 commented Jun 6, 2019

Previously with a prefixed name a final dot was not recognized as
separating triple blocks.

Sadly in SPARQL this is quite messy.

For example the following query works (in Blazegraph):

SELECT ?item WHERE {
   ?item wdt:P31 wd:Q2934.?item wdt:P39 wd:Q41240317
}

Using ^ reversing the following also works

SELECT ?item WHERE {
   ?item wdt:P31 wd:Q2934. wd:Q41240317 ^wdt:P39 ?item
}

However removing the after the . breaks parsing even though it's
not needed at the same position when the ? disambiguates.

So yeah we really should use a proper parser that naturally handles this
weirdness.

Note: This still uses the current hacky parser. #236 is absolutely still
relevant but this case is just a bit too frustrating with a lot of Wikidata queries.

@niklas88
Copy link
Member Author

niklas88 commented Jun 6, 2019

With this change we match Blazegraph behavior for the given examples.

@niklas88 niklas88 mentioned this pull request Jun 6, 2019
Previously with a prefixed name a final dot was not recognized as
separating triple blocks.

Sadly in SPARQL this is quite messy.

For example the following query works (in Blazegraph):

    SELECT ?item WHERE {
       ?item wdt:P31 wd:Q2934.?item wdt:P39 wd:Q41240317
    }

Using `^` reversing the following also works

    SELECT ?item WHERE {
       ?item wdt:P31 wd:Q2934. wd:Q41240317 ^wdt:P39 ?item
    }

However removing the ` ` after the `.` breaks parsing even though it's
not needed at the same position when the `?` disambiguates.

So yeah we really should use a proper parser that naturally handles this
weirdness.
Copy link
Member

@floriankramer floriankramer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@floriankramer
Copy link
Member

@niklas88 I'm currently working on supporting the VALUES statement. As I need to parse the values in the values statement I've slightly restructured the way triples are parsed to make the code more reusable and in doing that have also included the changes in this pr. If this pr is not to pressing it would save me a (potentially annoying) rebase if we could not merge this, but instead wait for the values pr (which should be done sometime next week)

@niklas88
Copy link
Member Author

@floriankramer that sounds good to me, will keep this in my development/test instances until then because this makes rewriting Blazegraph stuff much easier.

@niklas88
Copy link
Member Author

@floriankramer by the way I'm making progress on testing against Blazegraph using the Wikidata Example queries. It's looking pretty good performance wise and maybe you can use some of the uses of VALUES from there for testing. It's also great for getting lists of queries that still fail and especially those that fail even though our ANTLR grammar can parse them.

@niklas88
Copy link
Member Author

niklas88 commented Aug 6, 2019

This has been made obsolete (as discussed) by the recent parser changes

@niklas88 niklas88 closed this Aug 6, 2019
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

2 participants