Matthias Mullie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/196660
Change subject: Remove unique constraints from flow_*_ref
......................................................................
Remove unique constraints from flow_*_ref
The unique constraints are actually on all columns.
We don't really need this:
* it's not a proper way to ensure unique data
* code already inserts only uniques
* doesn't really matter if we were to have duplicates in there,
that data is just a denormalization anyway (the real data can
be extracted from flow_revision.rev_content)
But I've added an extra check just to make sure that duplicates,
which should never even make it in, won't show up.
Bug: T92284
Change-Id: I8fa92f5cf72bf979bc774c7202f55bc8b6253a56
---
M Hooks.php
M container.php
A db_patches/patch-remove_unique_ref_indices.sql
M flow.sql
M includes/Data/Storage/BasicDbStorage.php
M includes/Model/Reference.php
M includes/ReferenceClarifier.php
7 files changed, 48 insertions(+), 7 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/60/196660/1
diff --git a/Hooks.php b/Hooks.php
index a54347a..82e1477 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -156,6 +156,7 @@
$updater->dropExtensionTable( 'flow_definition',
"$dir/db_patches/patch-drop_definition.sql" );
$updater->dropExtensionField( 'flow_workflow',
'workflow_user_ip', "$dir/db_patches/patch-drop_workflow_user.sql" );
$updater->addExtensionField( 'flow_revision',
'rev_content_length', "$dir/db_patches/patch-add-revision-content-length.sql" );
+ $updater->addExtensionIndex( 'flow_ext_ref',
'flow_ext_ref_idx', "$dir/db_patches/patch-remove_unique_ref_indices.sql" );
require_once __DIR__.'/maintenance/FlowUpdateRecentChanges.php';
$updater->addPostDatabaseUpdateMaintenance(
'FlowUpdateRecentChanges' );
diff --git a/container.php b/container.php
index 5f9daf2..772141c 100644
--- a/container.php
+++ b/container.php
@@ -991,7 +991,8 @@
'ref_src_title',
'ref_src_object_id',
'ref_type',
- 'ref_target_namespace', 'ref_target_title'
+ 'ref_target_namespace',
+ 'ref_target_title'
);
$c['storage.wiki_reference.mapper'] = function( $c ) {
return Flow\Data\Mapper\BasicObjectMapper::model(
diff --git a/db_patches/patch-remove_unique_ref_indices.sql
b/db_patches/patch-remove_unique_ref_indices.sql
new file mode 100644
index 0000000..7a0cc31
--- /dev/null
+++ b/db_patches/patch-remove_unique_ref_indices.sql
@@ -0,0 +1,15 @@
+-- drop unique constraint & recreate index
+DROP INDEX /*i*/flow_wiki_ref_pk ON /*_*/flow_wiki_ref;
+DROP INDEX /*i*/flow_wiki_ref_revision ON /*_*/flow_wiki_ref;
+
+CREATE INDEX /*i*/flow_wiki_ref_idx ON /*_*/flow_wiki_ref ( ref_src_namespace,
ref_src_title, ref_type, ref_target_namespace, ref_target_title,
ref_src_object_type, ref_src_object_id );
+CREATE INDEX /*i*/flow_wiki_ref_revision ON /*_*/flow_wiki_ref (
ref_src_namespace, ref_src_title, ref_src_object_type, ref_src_object_id,
ref_type, ref_target_namespace, ref_target_title );
+
+-- drop unique constraint, change url column to blob & recreate index
+DROP INDEX /*i*/flow_ext_ref_pk ON /*_*/flow_ext_ref;
+DROP INDEX /*i*/flow_ext_ref_revision ON /*_*/flow_ext_ref;
+
+ALTER TABLE /*_*/flow_ext_ref CHANGE ref_target ref_target BLOB;
+
+CREATE INDEX /*i*/flow_ext_ref_idx ON /*_*/flow_ext_ref ( ref_src_namespace,
ref_src_title, ref_type, ref_target(255), ref_src_object_type,
ref_src_object_id );
+CREATE INDEX /*i*/flow_ext_ref_revision ON /*_*/flow_ext_ref (
ref_src_namespace, ref_src_title, ref_src_object_type, ref_src_object_id,
ref_type, ref_target(255) );
diff --git a/flow.sql b/flow.sql
index 639d160..a4ff522 100644
--- a/flow.sql
+++ b/flow.sql
@@ -146,10 +146,10 @@
ref_type varbinary(16) not null
) /*$wgDBTableOptions*/;
-CREATE UNIQUE INDEX /*i*/flow_wiki_ref_pk ON /*_*/flow_wiki_ref
+CREATE INDEX /*i*/flow_wiki_ref_idx ON /*_*/flow_wiki_ref
(ref_src_namespace, ref_src_title, ref_type, ref_target_namespace,
ref_target_title, ref_src_object_type, ref_src_object_id);
-CREATE UNIQUE INDEX /*i*/flow_wiki_ref_revision ON /*_*/flow_wiki_ref
+CREATE INDEX /*i*/flow_wiki_ref_revision ON /*_*/flow_wiki_ref
(ref_src_namespace, ref_src_title, ref_src_object_type,
ref_src_object_id, ref_type, ref_target_namespace, ref_target_title);
CREATE TABLE /*_*/flow_ext_ref (
@@ -162,7 +162,7 @@
ref_type varbinary(16) not null
) /*$wgDBTableOptions*/;
-CREATE UNIQUE INDEX /*i*/flow_ext_ref_pk ON /*_*/flow_ext_ref
+CREATE UNIQUE INDEX /*i*/flow_ext_ref_idx ON /*_*/flow_ext_ref
(ref_src_namespace, ref_src_title, ref_type, ref_target,
ref_src_object_type, ref_src_object_id);
CREATE UNIQUE INDEX /*i*/flow_ext_ref_revision ON /*_*/flow_ext_ref
diff --git a/includes/Data/Storage/BasicDbStorage.php
b/includes/Data/Storage/BasicDbStorage.php
index 380fb8a..d1bb3ef 100644
--- a/includes/Data/Storage/BasicDbStorage.php
+++ b/includes/Data/Storage/BasicDbStorage.php
@@ -131,7 +131,7 @@
public function find( array $attributes, array $options = array() ) {
$attributes = $this->preprocessSqlArray( $attributes );
- if ( ! $this->validateOptions( $options ) ) {
+ if ( !$this->validateOptions( $options ) ) {
throw new \MWException( "Validation error in database
options" );
}
diff --git a/includes/Model/Reference.php b/includes/Model/Reference.php
index 00496fa..8dbe7ae 100644
--- a/includes/Model/Reference.php
+++ b/includes/Model/Reference.php
@@ -134,4 +134,18 @@
$this->getObjectId()->getAlphadecimal() . '|' .
$this->getIdentifier();
}
+
+
+ /**
+ * We don't have a real PK (see comment in
+ * ReferenceClarifier::loadReferencesForPage) but I'll do a
array_unique on
+ * multiple Reference objects, just to make sure we have no duplicates.
+ * But to be able to do an array_unique, the objects will be compared as
+ * strings.
+ *
+ * @return string
+ */
+ public function __toString() {
+ return $this->getUniqueIdentifier();
+ }
}
diff --git a/includes/ReferenceClarifier.php b/includes/ReferenceClarifier.php
index 1fe339f..7b5ef7b 100644
--- a/includes/ReferenceClarifier.php
+++ b/includes/ReferenceClarifier.php
@@ -32,7 +32,7 @@
$ids[$id->getAlphadecimal()] = $id;
}
- // Dont need to do anything with them, they are automatically
+ // Don't need to do anything with them, they are automatically
// passed into url generator when loaded.
$this->storage->getMulti( 'Workflow', $ids );
@@ -111,13 +111,23 @@
)
);
if ( $res ) {
+ /*
+ * We're "cheating", we have no PK!
+ * We used to have a unique index (on all
columns), but the size
+ * of the index was too small (urls can be
pretty long...)
+ * We have no data integrity reasons to want to
ensure unique
+ * entries, and the code actually does a good
jon of only
+ * inserting uniques. Still, I'll do a sanity
check and get rid
+ * of duplicates, should there be any...
+ */
+ $res = array_unique( $res );
$allReferences = array_merge( $allReferences,
$res );
}
}
$cache = array();
foreach( $allReferences as $reference ) {
- if ( ! isset( $cache[$reference->getTargetIdentifier()]
) ) {
+ if ( !isset( $cache[$reference->getTargetIdentifier()]
) ) {
$cache[$reference->getTargetIdentifier()] =
array();
}
--
To view, visit https://gerrit.wikimedia.org/r/196660
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8fa92f5cf72bf979bc774c7202f55bc8b6253a56
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
