Peter M. Groen / knowledgetree

Commit e833a8a554014169f6ea8e878189bad1778b987f

Authored by Paul Barrett
1 parent 21b83452

KTS-4214. Refactored the fix to use a more generic solution as suggested by obse… 

…rvations of similar searches

Fixed

Search does not return any results if more than one metadata type is included in the search expression with an AND statement.

Committed by: Paul Barrett

Reviewed by: Megan Watson
Inline Side-by-side
Showing 2 changed files with 53 additions and 51 deletions
search2/search/expr.inc.php
  View file @e833a8a
... ... @@ -271,10 +271,6 @@ class Expr
271 271 return $this instanceof MetadataField;
272 272 }
273 273  
274   -
275   -
276   -
277   -
278 274 public function toViz(&$str, $phase)
279 275 {
280 276 throw new Exception('To be implemented' . get_class($this));
... ... @@ -454,7 +450,6 @@ class DBFieldExpr extends FieldExpr
454 450 protected $matchfield;
455 451 protected $quotedvalue;
456 452  
457   -
458 453 /**
459 454 * Constructor for the database field
460 455 *
... ... @@ -507,7 +502,6 @@ class DBFieldExpr extends FieldExpr
507 502 return $value;
508 503 }
509 504  
510   -
511 505 public function getJoinTable() { return $this->jointable; }
512 506 public function getJoinField() { return $this->joinfield; }
513 507 public function getMatchingField() { return $this->matchfield; }
... ... @@ -629,9 +623,6 @@ class ValueExpr extends Expr
629 623 }
630 624 }
631 625  
632   -
633   -
634   -
635 626 public function getSQL($field, $fieldname, $op, $not=false)
636 627 {
637 628 $val = $this->getValue();
... ... @@ -714,7 +705,7 @@ class ValueExpr extends Expr
714 705 default:
715 706 throw new Exception(sprintf(_kt('Unknown op: %s'), $op));
716 707 }
717   -
  708 +
718 709 return $sql;
719 710 }
720 711  
... ... @@ -968,7 +959,7 @@ class TextQueryBuilder implements QueryBuilder
968 959 else
969 960 $query .= "$not$fieldname:$value";
970 961 }
971   -
  962 +
972 963 return $query;
973 964 }
974 965  
... ... @@ -1066,7 +1057,7 @@ class SQLQueryBuilder implements QueryBuilder
1066 1057  
1067 1058 private function exploreExprs($expr, $parent=null)
1068 1059 {
1069   - if ($expr->isMetadataField())
  1060 + if ($expr->isMetadataField())
1070 1061 {
1071 1062 $this->metadata[] = & $parent;
1072 1063 }
... ... @@ -1079,7 +1070,7 @@ class SQLQueryBuilder implements QueryBuilder
1079 1070 if (array_key_exists($tablename, $this->used_tables))
1080 1071 {
1081 1072 $this->used_tables[$tablename]++;
1082   - if (isset($expr->references)) ++$expr->references;
  1073 +// if (isset($expr->references)) ++$expr->references;
1083 1074 }
1084 1075 }
1085 1076 }
... ... @@ -1144,6 +1135,7 @@ class SQLQueryBuilder implements QueryBuilder
1144 1135  
1145 1136 private function getSQLEvalExpr($expr)
1146 1137 {
  1138 +
1147 1139 $left = $expr->left();
1148 1140 $right = $expr->right();
1149 1141 $isNot = $expr->not();
... ... @@ -1182,6 +1174,7 @@ class SQLQueryBuilder implements QueryBuilder
1182 1174 $query = $right->getSQL($left, $left->modifyName($fieldname), $expr->op(), $isNot);
1183 1175 }
1184 1176 }
  1177 +
1185 1178 return $query;
1186 1179 }
1187 1180  
... ... @@ -1243,21 +1236,26 @@ class SQLQueryBuilder implements QueryBuilder
1243 1236 {
1244 1237 $sql .= ' INNER JOIN document_content_version dcv ON dmv.content_version_id=dcv.id ' . "\n";
1245 1238 }
1246   - if ($this->used_tables['document_fields_link'] > 0)
1247   - {
1248   - // NOTE this is a bit of a hack, maybe? or maybe it really is the best way?
1249   - // ...what about using the loop below which checks the expression
1250   - // for a join table rather than this up here? - otherwise this only
1251   - // affects this particular query type - then again maybe that's what we want...
1252   - for ($i = 0; $i < $this->used_tables['document_fields_link']; ++$i)
1253   - {
1254   - if ($i > 0) $counter = $i;
1255   - else $counter = '';
1256   -
1257   - $sql .= ' LEFT JOIN document_fields_link pdfl' . $counter . ' '
1258   - . 'ON dmv.id=pdfl' . $counter . '.metadata_version_id ' . "\n";
1259   - }
1260   - }
  1239 + // NOTE this was the old method of dealing with joining on the document_fields_link table;
  1240 + // the new method incorporates previously set up joining code which was not completely understood
  1241 + // at the time I modified this code; it allows everything to be more encapsulated within the classes and
  1242 + // done in a more generic way which does not require a special case code snippet such as the one commented
  1243 + // out below.
  1244 +// if ($this->used_tables['document_fields_link'] > 0)
  1245 +// {
  1246 +// // NOTE this is a bit of a hack, maybe? or maybe it really is the best way?
  1247 +// // ...what about using the loop below which checks the expression
  1248 +// // for a join table rather than this up here? - otherwise this only
  1249 +// // affects this particular query type - then again maybe that's what we want...
  1250 +// for ($i = 0; $i < $this->used_tables['document_fields_link']; ++$i)
  1251 +// {
  1252 +// if ($i > 0) $counter = $i;
  1253 +// else $counter = '';
  1254 +//
  1255 +// $sql .= ' LEFT JOIN document_fields_link pdfl' . $counter . ' '
  1256 +// . 'ON dmv.id=pdfl' . $counter . '.metadata_version_id ' . "\n";
  1257 +// }
  1258 +// }
1261 1259  
1262 1260 if ($this->used_tables['tag_words'] > 0)
1263 1261 {
... ... @@ -1325,6 +1323,7 @@ class SQLQueryBuilder implements QueryBuilder
1325 1323 $sql .= "f.linked_folder_id is null";
1326 1324 }
1327 1325 $sql .= ' AND ';
  1326 +
1328 1327 return $sql;
1329 1328 }
1330 1329  
... ... @@ -1380,7 +1379,7 @@ class SQLQueryBuilder implements QueryBuilder
1380 1379 $query = 'false';
1381 1380 }
1382 1381 }
1383   -
  1382 +
1384 1383 return $query;
1385 1384 }
1386 1385  
... ... @@ -1402,7 +1401,7 @@ class SQLQueryBuilder implements QueryBuilder
1402 1401  
1403 1402 $sql .= " limit $maxSqlResults";
1404 1403  
1405   - return $sql;
  1404 + return $sql;
1406 1405 }
1407 1406  
1408 1407 public function buildSimpleQuery($op, $group)
... ...
search2/search/fields/AnyMetadataField.inc.php
  View file @e833a8a
... ... @@ -40,12 +40,15 @@
40 40 class AnyMetadataField extends DBFieldExpr
41 41 {
42 42 public $general_op = ExprOp::CONTAINS;
43   - public $references = 0;
  43 +// public $references = 0;
44 44  
45 45 public function __construct()
46 46 {
47   - parent::__construct('value', 'document_fields_link', _kt('Any Metadata'));
  47 +// parent::__construct('value', 'document_fields_link', _kt('Any Metadata'));
  48 + parent::__construct('id', 'document_metadata_version', _kt('Any Metadata'));
48 49 $this->setAlias('Metadata');
  50 + $this->joinTo('document_fields_link', 'metadata_version_id');
  51 + $this->matchField('value');
49 52 }
50 53  
51 54 /*
... ... @@ -56,25 +59,25 @@ class AnyMetadataField extends DBFieldExpr
56 59 * I don't like this and think we should look for a way to make the table joining more generic
57 60 * such that it can be controlled via these classes and thereby contained as a unit.
58 61 */
59   - public function modifyName($name)
60   - {
61   - if ($this->references > 0)
62   - {
63   - static $count = 0;
64   - if ($count >= $this->references)
65   - {
66   - $count = 0;
67   - }
68   -
69   - if ((($pos = strpos($name, '.')) !== false) && ($count != 0))
70   - {
71   - $name = substr($name, 0, $pos) . $count . substr($name, $pos);
72   - }
73   - ++$count;
74   - }
75   -
76   - return $name;
77   - }
  62 +// public function modifyName($name)
  63 +// {
  64 +// if ($this->references > 0)
  65 +// {
  66 +// static $count = 0;
  67 +// if ($count >= $this->references)
  68 +// {
  69 +// $count = 0;
  70 +// }
  71 +//
  72 +// if ((($pos = strpos($name, '.')) !== false) && ($count != 0))
  73 +// {
  74 +// $name = substr($name, 0, $pos) . $count . substr($name, $pos);
  75 +// }
  76 +// ++$count;
  77 +// }
  78 +//
  79 +// return $name;
  80 +// }
78 81  
79 82 public function getInputRequirements()
80 83 {
... ...