Skip to content

Commit

Permalink
TextByChangeUpdater change update approach, refs 1481 (#2388)
Browse files Browse the repository at this point in the history
A certain update sequence could allow for a text element to remain in the
index due to the missing information in the diff record about a removed
element.

Instead of using the insert-diff to replace text elements, use the delete-diff
to remove all items from the index and after that insert related data from
the primary data container. This approach ensures the content is in sync with
the SemanticData primary data update.
  • Loading branch information
mwjames committed Apr 2, 2017
1 parent 2a8480e commit b4e0c65
Show file tree
Hide file tree
Showing 12 changed files with 367 additions and 172 deletions.
3 changes: 2 additions & 1 deletion DefaultSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -1268,7 +1268,8 @@
##
'smwgFulltextSearchPropertyExemptionList' => array(
'_ASKFO', '_ASKST', '_ASKPA','_IMPO', '_LCODE', '_UNIT', '_CONV',
'_TYPE', '_ERRT', '_INST', '_ASK', '_INST', '_SOBJ', '_PVAL', '_PVALI'
'_TYPE', '_ERRT', '_INST', '_ASK', '_INST', '_SOBJ', '_PVAL', '_PVALI',
'_REDI'
),
##

Expand Down
13 changes: 9 additions & 4 deletions src/SQLStore/ChangeOp/TableChangeOp.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,24 @@ public function hasChangeOp( $opType ) {
/**
* @since 2.4
*
* @param string $opType
* @param string|null $opType
*
* @return FieldChangeOp[]|[]
*/
public function getFieldChangeOps( $opType ) {
public function getFieldChangeOps( $opType = null ) {

if ( !$this->hasChangeOp( $opType ) ) {
if ( $opType !== null && !$this->hasChangeOp( $opType ) ) {
return array();
}

$fieldOps = array();
$changeOps = $this->changeOps;

if ( $opType !== null ) {
$changeOps = $this->changeOps[$opType];
}

foreach ( $this->changeOps[$opType] as $op ) {
foreach ( $changeOps as $op ) {
$fieldOps[] = new FieldChangeOp( $op );
}

Expand Down
34 changes: 10 additions & 24 deletions src/SQLStore/ChangeOp/TempChangeOpStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public function createSlotFrom( CompositePropertyTableDiffIterator $compositePro

$this->cache->save(
$slot,
$orderedDiffByTable
serialize( $compositePropertyTableDiffIterator )
);

return $slot;
Expand All @@ -115,37 +115,23 @@ public function delete( $slot ) {
}

/**
* @since 2.5
* @since 3.0
*
* @param string $slot
* @param string|null $tableName
*
* @return TableChange[]
* @return CompositePropertyTableDiffIterator|null
*/
public function newTableChangeOpsFrom( $slot, $tableName = null ) {

$start = microtime( true );

$diffByTable = $this->cache->fetch( $slot );
$tableChangeOps = array();

if ( $diffByTable === false || $diffByTable === null ) {
$this->log( __METHOD__ . ' unknown slot :: '. $slot );
return array();
}
public function newCompositePropertyTableDiffIterator( $slot ) {

foreach ( $diffByTable as $tblName => $diff ) {

if ( $tableName !== null && $tableName !== $tblName ) {
continue;
}
$compositePropertyTableDiffIterator = unserialize(
$this->cache->fetch( $slot )
);

$tableChangeOps[] = new TableChangeOp( $tblName, $diff );
if ( $compositePropertyTableDiffIterator === false || $compositePropertyTableDiffIterator === null ) {
return null;
}

$this->log( __METHOD__ . ' procTime (sec): '. round( ( microtime( true ) - $start ), 5 ) );

return $tableChangeOps;
return $compositePropertyTableDiffIterator;
}

private function log( $message, $context = array() ) {
Expand Down
100 changes: 81 additions & 19 deletions src/SQLStore/CompositePropertyTableDiffIterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,27 @@
*/
class CompositePropertyTableDiffIterator implements IteratorAggregate {

/**
* Type of change operations
*/
const TYPE_INSERT = 'insert';
const TYPE_DELETE = 'delete';

/**
* @var array
*/
private $diff = array();

/**
* @var array
*/
private $data = array();

/**
* @var array
*/
private $orderedDiff = array();

/**
* @var DIWikiPage
*/
Expand Down Expand Up @@ -77,20 +93,47 @@ public function getIterator() {
* @return string
*/
public function getHash() {
return md5( $this->hash . ( $this->subject !== null ? $this->subject->getHash() : '' ) );
return md5( $this->hash . ( $this->subject !== null ? $this->subject->asBase()->getHash() : '' ) );
}

/**
* @since 3.0
*
* @param array $data
*/
public function addDataRecord( $hash, array $data ) {
$this->data[$hash] = $data;
}

/**
* @since 3.0
*
* @return TableChangeOp[]
*/
public function getDataChangeOps() {

$dataChangeOps = array();

foreach ( $this->data as $hash => $data ) {
foreach ( $data as $tableName => $d ) {
$dataChangeOps[] = new TableChangeOp( $tableName, $d );
}
}

return $dataChangeOps;
}

/**
* @since 2.3
*
* @param array $insertRecord
* @param array $deleteRecord
* @param array $insertChangeOp
* @param array $deleteChangeOp
*/
public function addTableRowsToCompositeDiff( array $insertRecord, array $deleteRecord ) {
public function addTableDiffChangeOp( array $insertChangeOp, array $deleteChangeOp ) {

$diff = array(
'insert' => $insertRecord,
'delete' => $deleteRecord
'insert' => $insertChangeOp,
'delete' => $deleteChangeOp
);

$this->diff[] = $diff;
Expand Down Expand Up @@ -147,6 +190,10 @@ public function getTableChangeOps( $table = null ) {
*/
public function getOrderedDiffByTable( $table = null ) {

if ( $table === null && $this->orderedDiff !== array() ) {
return $this->orderedDiff;
}

$ordered = array();

foreach ( $this as $diff ) {
Expand Down Expand Up @@ -176,49 +223,64 @@ public function getOrderedDiffByTable( $table = null ) {
}
}

if ( $table === null ) {
$this->orderedDiff = $ordered;
}

return $ordered;
}

/**
* @since 2.3
* @since 3.0
*
* @param string|null $type
*
* @return array
*/
public function getCombinedIdListOfChangedEntities() {
public function getListOfChangedEntityIdsByType( $type = null ) {

$list = array();
$changedEntities = array();

foreach ( $this->getOrderedDiffByTable() as $diff ) {

if ( isset( $diff['insert'] ) ) {
$this->addToIdList( $list, $diff['insert'] );
if ( ( $type === 'insert' || $type === null ) && isset( $diff['insert'] ) ) {
$this->addToIdList( $changedEntities, $diff['insert'] );
}

if ( isset( $diff['delete'] ) ) {
$this->addToIdList( $list, $diff['delete'] );
if ( ( $type === 'delete' || $type === null ) && isset( $diff['delete'] ) ) {
$this->addToIdList( $changedEntities, $diff['delete'] );
}

if ( isset( $diff['property'] ) ) {
$list[$diff['property']['p_id']] = null;
if ( $type === null && isset( $diff['property'] ) ) {
$changedEntities[$diff['property']['p_id']] = true;
}
}

return array_keys( $list );
return $changedEntities;
}

/**
* @since 2.3
*
* @return array
*/
public function getCombinedIdListOfChangedEntities() {
return array_keys( $this->getListOfChangedEntityIdsByType() );
}

private function addToIdList( &$list, $value ) {
foreach ( $value as $element ) {

if ( isset( $element['p_id'] ) ) {
$list[$element['p_id']] = null;
$list[$element['p_id']] = true;
}

if ( isset( $element['s_id'] ) ) {
$list[$element['s_id']] = null;
$list[$element['s_id']] = true;
}

if ( isset( $element['o_id'] ) ) {
$list[$element['o_id']] = null;
$list[$element['o_id']] = true;
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/SQLStore/PropertyTableRowDiffer.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,12 @@ public function computeTableRowDiffFor( $sid, SemanticData $semanticData ) {
}
}

$this->getCompositePropertyTableDiff()->addTableRowsToCompositeDiff(
$this->getCompositePropertyTableDiff()->addDataRecord(
$semanticData->getSubject()->getHash(),
$newData
);

$this->getCompositePropertyTableDiff()->addTableDiffChangeOp(
$tablesInsertRows,
$tablesDeleteRows
);
Expand Down
23 changes: 23 additions & 0 deletions src/SQLStore/QueryEngine/Fulltext/SearchTableUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,29 @@ public function optimize() {
return true;
}

/**
* @since 2.5
*
* @param integer $sid
* @param integer $pid
*
* @return boolean
*/
public function exists( $sid, $pid ) {

$row = $this->connection->selectRow(
$this->searchTable->getTableName(),
array( 's_id' ),
array(
's_id' => (int)$sid,
'p_id' => (int)$pid
),
__METHOD__
);

return $row !== false;
}

/**
* @since 2.5
*
Expand Down

0 comments on commit b4e0c65

Please sign in to comment.