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

Support subquery negation operator in SemanticMediaWiki queries. #20

Closed

Conversation

vitalif
Copy link

@vitalif vitalif commented Nov 14, 2013

The syntax is {{#ask: [[prop::val]] !...subquery... }}
or just {{#ask: [[prop::val]] ![[prop2::val2]] }}.

Change-Id: I107d7f6905860379684a011ba1bf0c44a150c575

The syntax is {{#ask: [[prop::val]] !<q>...subquery...</q> }}
or just {{#ask: [[prop::val]] ![[prop2::val2]] }}.

Change-Id: I107d7f6905860379684a011ba1bf0c44a150c575
*
* @ingroup SMWQuery
*/
class SMWNegation extends SMWDescription {
Copy link
Member

Choose a reason for hiding this comment

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

Heh. I was actually thinking of doing this as the topic of my dev workshop at the previous SMWCon.

@JeroenDeDauw
Copy link
Member

Thanks for the patch! This is a quite useful feature.

There are a number of complexity issues that need to be resolved before this can be merged. Some tests would also be much appreciated. I'll be answering the question of how to write tests soonish.

@mwjames
Copy link
Contributor

mwjames commented Nov 15, 2013

This patch introduces behavioral changes together with modifications to core classes therefore unit tests should be provided in order to verify that existing behaviour is not altered and new functionality has sufficient coverage especially in cases where NPath complexity is increased or introduced.

@vitalif
Copy link
Author

vitalif commented Nov 15, 2013

By the way, I've also submitted this change to gerrit before. What should I do with it there?

@JeroenDeDauw
Copy link
Member

I abandoned it there, given review here has already happened

@ganqqwerty
Copy link

hey everyone! Am I right that the only blocker to get this awesome feature merged is unit tests?

@mwjames
Copy link
Contributor

mwjames commented Feb 10, 2014

Am I right that the only blocker to get this awesome feature merged is unit tests?

As I pointed out earlier, I'd like to see that amendments of SMW_SQLStore3_Queries + SMW_QueryParser to go into a separate class (SMW\Query\Parser\Negation or SMW\Query\Parser\Complement + SMW\SQLStore\SqlQueryBuilder) making it possible to cause less harm to the original core class (because last time a small change was made in a core class it had some serious repercussions due to insufficient test coverage) .

Description

Amendments to the Description classes need appropriate unit test additions. (needs discussion whether it should be called SMWNegation or SMWComplement)

SqlQueryBuilder

In case of SMWSQLStore3Query::Q_NEGATION, use the new SMW\SQLStore\SqlQueryBuilder to build the sql without the need to alter SMW_SQLStore3_Queries except the invocation of SqlQueryBuilder and the returning query object.

Since core classes are involved and any change can have unpredictable results it is vital that new functionality provides "healthy" unit tests (if possible regression tests would be appreciated as well).

While that may sound like a lot of work, at the current state of the Store we need to start to sort things out independently otherwise the maintenance of the Store implementation remains a nightmare.

@mwjames
Copy link
Contributor

mwjames commented Jul 14, 2018

The PR has several issues (as outlined) that got not resolved and with classes so different from the time the PR was created it seems difficult to expect that it will be resolved for SMW 3.0 and any other upcoming release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature A new, or altered behaviour of an existing functionality that fundamentally impacts behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants