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.

Sign up for GitHub

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

Jump to bottom

SemanticDataLookup, use DISTINCT for non subject items, refs 2928, 3531 #3617

Merged
merged 1 commit into from
Jan 13, 2019
Merged

SemanticDataLookup, use DISTINCT for non subject items, refs 2928, 3531 #3617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 46 additions & 4 deletions src/SQLStore/EntityStore/SemanticDataLookup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ public function fetchSemanticData( $id, DataItem $dataItem = null, PropertyTable
$query->condition( $query->eq( 'p_id', $extraCondition['p_id'] ) );
}
}
} else {
$requestOptions = new RequestOptions();
}

$valueCount = 0;
Expand All @@ -243,6 +245,39 @@ public function fetchSemanticData( $id, DataItem $dataItem = null, PropertyTable
$fieldname
);

// Don't use DISTINCT for subject related value match but make sure
// (#3531) it is used when requesting other values in order to retrieve
// all available unique values within the range of the limit
if ( !$isSubject ) {
$requestOptions->setOption( 'DISTINCT', true );

// Don't sort, this avoids a SQL `filesort`/`temporary table` usage
// in combination with DISTINCT, values will be listed as-is instead
// of a lexical representation but can be compensated by selecting a
// wider range in case this is used as retrieving "all" values
// for a property

// SELECT DISTINCT o_id AS id0, o0.smw_title AS v0, o0.smw_namespace
// AS v1, o0.smw_iw AS v2, o0.smw_sortkey AS v3, o0.smw_subobject AS
// v4 FROM `smw_di_wikipage` INNER JOIN `smw_object_ids` AS o0 ON
// o_id=o0.smw_id WHERE (p_id='x') LIMIT 51
//
// 8.6281ms
//
// vs.
//
// SELECT DISTINCT o_id AS id0, o0.smw_title AS v0, o0.smw_namespace
// AS v1, o0.smw_iw AS v2, o0.smw_sortkey AS v3, o0.smw_subobject AS
// v4 FROM `smw_di_wikipage` INNER JOIN `smw_object_ids` AS o0 ON
// o_id=o0.smw_id WHERE (p_id='x') ORDER BY o_id LIMIT 51
//
// 24189.0128ms
//
// PS: In case of a `TYPE_WIKIPAGE` entity, sorting by `o_id`
// wouldn't make much sense as it does not guarantee any lexical order
$requestOptions->setOption( 'ORDER BY', false );
}

// Apply sorting/string matching; only with given property
if ( !$isSubject ) {
$conds = $this->store->getSQLConditions(
Expand All @@ -253,14 +288,14 @@ public function fetchSemanticData( $id, DataItem $dataItem = null, PropertyTable
);

$query->condition( $conds );
$query->options( $this->store->getSQLOptions( $requestOptions, $valueField ) + [ 'DISTINCT' ] );
} else {
$valueField = '';

// Don't use DISTINCT for value of one subject:
$query->options( $this->store->getSQLOptions( $requestOptions, $valueField ) );
}

$query->options(
$this->store->getSQLOptions( $requestOptions, $valueField )
);

$res = $connection->query(
$query,
__METHOD__
Expand All @@ -287,6 +322,13 @@ public function fetchSemanticData( $id, DataItem $dataItem = null, PropertyTable

$connection->freeResult( $res );

// Sorting via PHP for an explicit disabled `ORDER BY` to ensure that
// the result set has at least a lexical order applied for the range of
// retrieved values
if ( $requestOptions->getOption( 'ORDER BY' ) === false ) {
sort( $result );
}

return $result;
}

Expand Down
59 changes: 59 additions & 0 deletions tests/phpunit/Integration/JSONScript/TestCases/s-0033.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
{
"description": "Test output from `Special:SearchByProperty` to show all values for a property (#3531)",
"setup": [
{
"namespace": "SMW_NS_PROPERTY",
"page": "Has vtext",
"contents": "[[Has type::Text]]"
},
{
"page": "Test/S0033/1",
"contents": "[[Has vtext::123]] [[Has vtext::456]] [[Has vtext::abc]] [[Has vtext::def]] [[Has vtext::1001]] [[Has vtext::42]]"
},
{
"page": "Test/S0033/2",
"contents": "[[Has vtext::123]] [[Has vtext::456]] [[Has vtext::abc]] [[Has vtext::def]] [[Has vtext::1001]] [[Has vtext::42]]"
},
{
"page": "Test/S0033/3",
"contents": "[[Has vtext::123]] [[Has vtext::456]] [[Has vtext::abc]] [[Has vtext::def]] [[Has vtext::1001]] [[Has vtext::42]]"
},
{
"page": "Test/S0033/4",
"contents": "[[Has vtext::123]] [[Has vtext::456]] [[Has vtext::abc]] [[Has vtext::def]] [[Has vtext::1001]] [[Has vtext::42]]"
}
],
"tests": [
{
"type": "special",
"about": "#0 (find all values for property `Has vtext`)",
"special-page": {
"page": "SearchByProperty",
"query-parameters": "",
"request-parameters": {
"limit": 6,
"property": "Has vtext"
}
},
"assert-output": {
"to-contain": [
"<li>1001.*</li>",
"<li>123.*</li>",
"<li>42.*</li>",
"<li>456.*</li>",
"<li>abc.*</li>",
"<li>def.*</ul>"
]
}
}
],
"settings": {
"wgContLang": "en",
"wgLanguageCode": "en"
},
"meta": {
"version": "2",
"is-incomplete": false,
"debug": false
}
}
48 changes: 48 additions & 0 deletions tests/phpunit/Unit/SQLStore/EntityStore/SemanticDataLookupTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use SMW\DIProperty;
use SMW\DIWikiPage;
use SMWDIBlob as DIBlob;
use SMW\RequestOptions;
use SMW\SQLStore\EntityStore\SemanticDataLookup;
use SMW\Tests\PHPUnitCompat;
Expand Down Expand Up @@ -305,4 +306,51 @@ public function testSemanticData_NoDataItem() {
);
}

public function testFetchSemanticData_NonWikiPageTable_DISTINCT_SELECT() {

$row = new \stdClass;
$row->prop = 'FOO';
$row->v0 = '1001';

$this->dataItemHandler->expects( $this->any() )
->method( 'getFetchFields' )
->will( $this->returnValue( [ 'fooField' => 'fieldType' ] ) );

$propertyTable = $this->getMockBuilder( '\SMW\SQLStore\PropertyTableDefinition' )
->disableOriginalConstructor()
->getMock();

$propertyTable->expects( $this->atLeastOnce() )
->method( 'getDIType' )
->will( $this->returnValue( 'Foo' ) );

$propertyTable->expects( $this->atLeastOnce() )
->method( 'getName' )
->will( $this->returnValue( 'bar_table' ) );

$this->connection->expects( $this->any() )
->method( 'addQuotes' )
->will( $this->returnCallback( function( $value ) { return "'$value'"; } ) );

$this->connection->expects( $this->once() )
->method( 'query' )
->will( $this->returnValue( [ $row ] ) );

$dataItem = new DIBlob( __METHOD__ );

$requestOptions = new RequestOptions();
$requestOptions->setLimit( 4 );

$instance = new SemanticDataLookup(
$this->store
);

$instance->fetchSemanticData( 42, $dataItem, $propertyTable, $requestOptions );

$this->assertEquals(
"SELECT DISTINCT fooField AS v0 FROM bar_table WHERE (p_id='42') LIMIT 4",
$this->query->build()
);
}

}