From d39cf66286e94a5051296234b279354da7541b56 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 22:12:04 +0000
Subject: [PATCH 01/27] Added support for parsing join configurations for
 aggregations

---
 helper/config.php     |  2 +-
 meta/ConfigParser.php | 27 +++++++++++++++++++++------
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/helper/config.php b/helper/config.php
index 642330e7..e5971dc3 100644
--- a/helper/config.php
+++ b/helper/config.php
@@ -60,7 +60,7 @@ protected function parseFilter($val)
         $comps = implode('|', $comps);
 
         if (!preg_match('/^(.*?)(' . $comps . ')(.*)$/', $val, $match)) {
-            throw new StructException('Invalid search filter %s', hsc($val));
+            throw new StructException('Invalid conditional expression %s', hsc($val));
         }
         array_shift($match); // we don't need the zeroth match
         $match[0] = trim($match[0]);
diff --git a/meta/ConfigParser.php b/meta/ConfigParser.php
index e91caf20..19db9629 100644
--- a/meta/ConfigParser.php
+++ b/meta/ConfigParser.php
@@ -43,7 +43,7 @@ class ConfigParser
     public function __construct($lines)
     {
         /** @var \helper_plugin_struct_config $helper */
-        $helper = plugin_load('helper', 'struct_config');
+        $this->helper = plugin_load('helper', 'struct_config');
         // parse info
         foreach ($lines as $line) {
             [$key, $val] = $this->splitLine($line);
@@ -88,7 +88,7 @@ public function __construct($lines)
                 case 'order':
                 case 'sort':
                     $sorts = $this->parseValues($val);
-                    $sorts = array_map([$helper, 'parseSort'], $sorts);
+                    $sorts = array_map([$this->helper, 'parseSort'], $sorts);
                     $this->config['sort'] = array_merge($this->config['sort'], $sorts);
                     break;
                 case 'where':
@@ -99,7 +99,7 @@ public function __construct($lines)
                     $logic = 'AND';
                 case 'filteror':
                 case 'or':
-                    $flt = $helper->parseFilterLine($logic, $val);
+                    $flt = $this->helper->parseFilterLine($logic, $val);
                     if ($flt) {
                         $this->config['filter'][] = $flt;
                     }
@@ -196,13 +196,28 @@ protected function parseSchema($val)
     {
         $schemas = [];
         $parts = explode(',', $val);
+        $firsttable = null;
         foreach ($parts as $part) {
-            [$table, $alias] = sexplode(' ', trim($part), 2, '');
+            $words = array_pad(explode(' ', trim($part)), 2, '');
+            [$table, $alias, $on] = $words;
             $table = trim($table);
             $alias = trim($alias);
             if (!$table) continue;
-
-            $schemas[] = [$table, $alias];
+            if (is_null($firsttable)) $firsttable = $alias ? $alias : $table;
+            if ($on == 'ON') {
+                if (count($schemas) == 0) {
+                    throw new StructException('Can not specify JOIN ON for first schema'); 
+                }
+                $condition = $this->helper->parseFilter(
+                    implode(' ', array_slice($words, 3))
+                );
+                if ($condition[1] != '=') {
+                    throw new StructException('Only equality comparison is supported for JOIN conditions'); 
+                }
+            } else {
+                $condition = [];
+            }
+            $schemas[] = [$table, $alias, $condition];
         }
         return $schemas;
     }

From c5f5f2008a32d05782dea5222f8d45056cc12667 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 8 Apr 2023 12:17:32 +0100
Subject: [PATCH 02/27] Collect data on how to join schemas

---
 meta/Search.php       | 33 ++++++++++++++++++++++++---------
 meta/SearchConfig.php |  2 +-
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/meta/Search.php b/meta/Search.php
index fe679061..10d2acdc 100755
--- a/meta/Search.php
+++ b/meta/Search.php
@@ -28,6 +28,9 @@ class Search
     /** @var  \helper_plugin_sqlite */
     protected $sqlite;
 
+    /** @var string first schema added to the search */
+    protected $firstschema;
+
     /** @var Schema[] list of schemas to query */
     protected $schemas = [];
 
@@ -46,6 +49,9 @@ class Search
     /** @var array list of aliases tables can be referenced by */
     protected $aliases = [];
 
+    /** @var array list of conditionals to be used when joining tables */
+    protected $joins = array();
+
     /** @var  int begin results from here */
     protected $range_begin = 0;
 
@@ -86,8 +92,9 @@ public function getDb()
      *
      * @param string $table
      * @param string $alias
+     * @param string $joinon
      */
-    public function addSchema($table, $alias = '')
+    public function addSchema($table, $alias = '', $joinon = array())
     {
         $schema = new Schema($table);
         if (!$schema->getId()) {
@@ -96,6 +103,13 @@ public function addSchema($table, $alias = '')
 
         $this->schemas[$schema->getTable()] = $schema;
         if ($alias) $this->aliases[$alias] = $schema->getTable();
+        if (is_null($this->firstschema)) $this->firstschema = $scheam->getTable();
+        if (count($joinon) == 0) {
+            $joinon = array(
+                "{$schema->getTable()}.%pageid%", '=', "{$this->firstschema}.%pageid%"
+            );
+        }
+        $this->joins[$schema->getTable()] = $joinon;
     }
 
     /**
@@ -619,31 +633,32 @@ public function findColumn($colname, $strict = false)
         if (!$this->schemas) throw new StructException('noschemas');
         $schema_list = array_keys($this->schemas);
 
+        [$colname, $table] = $this->resolveColumn($colname);
+        $table_or_first = $table !== null ? $table : $schema_list[0];
+
         // add "fake" column for special col
         if ($colname == '%pageid%') {
-            return new PageColumn(0, new Page(), $schema_list[0]);
+            return new PageColumn(0, new Page(), $table_or_first);
         }
         if ($colname == '%title%') {
-            return new PageColumn(0, new Page(['usetitles' => true]), $schema_list[0]);
+            return new PageColumn(0, new Page(['usetitles' => true]), $table_or_first);
         }
         if ($colname == '%lastupdate%') {
-            return new RevisionColumn(0, new DateTime(), $schema_list[0]);
+            return new RevisionColumn(0, new DateTime(), $table_or_first);
         }
         if ($colname == '%lasteditor%') {
-            return new UserColumn(0, new User(), $schema_list[0]);
+            return new UserColumn(0, new User(), $table_or_first);
         }
         if ($colname == '%lastsummary%') {
-            return new SummaryColumn(0, new AutoSummary(), $schema_list[0]);
+            return new SummaryColumn(0, new AutoSummary(), $table_or_first);
         }
         if ($colname == '%rowid%') {
-            return new RowColumn(0, new Decimal(), $schema_list[0]);
+            return new RowColumn(0, new Decimal(), $table_or_first);
         }
         if ($colname == '%published%') {
             return new PublishedColumn(0, new Decimal(), $schema_list[0]);
         }
 
-        [$colname, $table] = $this->resolveColumn($colname);
-
         /*
          * If table name is given search only that, otherwise if no strict behavior
          * is requested by the caller, try all assigned schemas for matching the
diff --git a/meta/SearchConfig.php b/meta/SearchConfig.php
index 07f8c79f..64910c80 100644
--- a/meta/SearchConfig.php
+++ b/meta/SearchConfig.php
@@ -44,7 +44,7 @@ public function __construct($config, $dynamic = true)
 
         // setup schemas and columns
         if (!empty($config['schemas'])) foreach ($config['schemas'] as $schema) {
-            $this->addSchema($schema[0], $schema[1]);
+            $this->addSchema($schema[0], $schema[1], $scheam[2]);
         }
         if (!empty($config['cols'])) foreach ($config['cols'] as $col) {
             $this->addColumn($col);

From d224d1cf665bacbcf9e97c9907556fd0cbc782cc Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 22:16:06 +0000
Subject: [PATCH 03/27] Updated tests to include JOIN ON logic

This included adding some arguments to old tests and writing a few new
ones to test custom join logic.
---
 _test/ConfigParserTest.php          |  5 ++-
 _test/InlineConfigParserTest.php    |  8 ++--
 _test/SearchConfigParameterTest.php | 14 +++---
 _test/SearchConfigTest.php          |  5 ++-
 _test/SearchTest.php                | 66 +++++++++++++++++++++++++++++
 _test/mock/Search.php               |  2 +
 6 files changed, 86 insertions(+), 14 deletions(-)

diff --git a/_test/ConfigParserTest.php b/_test/ConfigParserTest.php
index d1096ad8..55768fbe 100644
--- a/_test/ConfigParserTest.php
+++ b/_test/ConfigParserTest.php
@@ -17,7 +17,7 @@ class ConfigParserTest extends StructTest
     public function test_simple()
     {
         $lines = [
-            "schema    : testtable, another, foo bar",
+            "schema    : testtable, another ON field = %pageid%, foo bar",
             "cols      : %pageid%, count",
             "sort      : ^count",
             "sort      : %pageid%, ^bam",
@@ -49,16 +49,19 @@ public function test_simple()
                         [
                             0 => 'testtable',
                             1 => '',
+                            2 => [],
                         ],
                     1 =>
                         [
                             0 => 'another',
                             1 => '',
+                            2 => ['field', '=', '%pageid%'],
                         ],
                     2 =>
                         [
                             0 => 'foo',
                             1 => 'bar',
+                            2 => [],
                         ],
                 ],
             'cols' =>
diff --git a/_test/InlineConfigParserTest.php b/_test/InlineConfigParserTest.php
index a8883903..9bd0d8cb 100644
--- a/_test/InlineConfigParserTest.php
+++ b/_test/InlineConfigParserTest.php
@@ -17,7 +17,7 @@ class InlineConfigParserTest extends StructTest
     public function test_simple()
     {
         // Same initial setup as ConfigParser.test
-        $inline = '"testtable, another, foo bar"."%pageid%, count" ';
+        $inline = '"testtable, another ON another.a = testtable.b, foo bar ON bar.a = testtable.a"."%pageid%, count" ';
         $inline .= '?sort: ^count sort: "%pageid%, ^bam" align: "r,l,center,foo"';
         // Add InlineConfigParser-specific tests:
         $inline .= ' & "%pageid% != start" | "count = 1"';
@@ -38,9 +38,9 @@ public function test_simple()
             'limit' => 0,
             'rownumbers' => false,
             'schemas' => [
-                ['testtable', ''],
-                ['another', ''],
-                ['foo', 'bar'],
+                ['testtable', '', []],
+                ['another', '', ['another.a', '=', 'testtable.b']],
+                ['foo', 'bar', ['bar.a', '=', 'testtable.a']],
             ],
             'sepbyheaders' => false,
             'sort' => [
diff --git a/_test/SearchConfigParameterTest.php b/_test/SearchConfigParameterTest.php
index 345dfdd2..1c70e961 100644
--- a/_test/SearchConfigParameterTest.php
+++ b/_test/SearchConfigParameterTest.php
@@ -77,8 +77,8 @@ public function test_constructor()
 
         $data = [
             'schemas' => [
-                ['schema1', 'alias1'],
-                ['schema2', 'alias2'],
+                ['schema1', 'alias1', []],
+                ['schema2', 'alias2', []],
             ],
             'cols' => [
                 '%pageid%',
@@ -142,8 +142,8 @@ public function test_filter()
     {
         $data = [
             'schemas' => [
-                ['schema1', 'alias1'],
-                ['schema2', 'alias2'],
+                ['schema1', 'alias1', []],
+                ['schema2', 'alias2', []],
             ],
             'cols' => [
                 '%pageid%',
@@ -194,8 +194,8 @@ public function test_sort()
     {
         $data = [
             'schemas' => [
-                ['schema1', 'alias1'],
-                ['schema2', 'alias2'],
+                ['schema1', 'alias1', []],
+                ['schema2', 'alias2', []],
             ],
             'cols' => [
                 '%pageid%',
@@ -228,7 +228,7 @@ public function test_pagination()
 
         $data = [
             'schemas' => [
-                ['schema2', 'alias2'],
+                ['schema2', 'alias2', []],
             ],
             'cols' => [
                 'afirst'
diff --git a/_test/SearchConfigTest.php b/_test/SearchConfigTest.php
index c5732c77..39de0ee8 100644
--- a/_test/SearchConfigTest.php
+++ b/_test/SearchConfigTest.php
@@ -49,7 +49,8 @@ public function test_filtervars_struct()
             ]
         );
 
-        $searchConfig = new SearchConfig(['schemas' => [['schema1', 'alias']]]);
+        $searchConfig = new SearchConfig(['schemas' => [['schema1', 'alias', []]]]);
+
         $this->assertEquals('test', $searchConfig->applyFilterVars('$STRUCT.first$'));
         $this->assertEquals('test', $searchConfig->applyFilterVars('$STRUCT.alias.first$'));
         $this->assertEquals('test', $searchConfig->applyFilterVars('$STRUCT.schema1.first$'));
@@ -94,7 +95,7 @@ public function test_filtervars_struct_other()
             ]
         );
 
-        $searchConfig = new SearchConfig(['schemas' => [['schema3', 'alias']]]);
+        $searchConfig = new SearchConfig(['schemas' => [['schema3', 'alias', []]]]);
         $this->assertEquals('', $searchConfig->applyFilterVars('$STRUCT.afirst$'));
         $this->assertEquals('test', $searchConfig->applyFilterVars('$STRUCT.schema2.afirst$'));
 
diff --git a/_test/SearchTest.php b/_test/SearchTest.php
index 0cb2f755..dd74076e 100644
--- a/_test/SearchTest.php
+++ b/_test/SearchTest.php
@@ -230,6 +230,14 @@ public function test_search()
         $search->addSchema('schema2', 'foo');
         $this->assertCount(2, $search->schemas);
 
+        $this->assertEquals(1, count($search->joins));
+        $joincols = $search->joins['schema2'];
+        $this->assertEquals(2, count($joincols));
+        $this->assertInstanceOf(meta\PageColumn::class, $joincols[0]);
+        $this->assertInstanceOf(meta\PageColumn::class, $joincols[1]);
+        $this->assertEquals('schema2', $joincols[0]->getTable());
+        $this->assertEquals('schema1', $joincols[1]->getTable());
+
         $search->addColumn('first');
         $this->assertEquals('schema1', $search->columns[0]->getTable());
         $this->assertEquals(1, $search->columns[0]->getColref());
@@ -440,4 +448,62 @@ public function test_filterValueList()
             $this->callInaccessibleMethod($search, 'parseFilterValueList', array('(18.7, 10e5, -100)')));
 
     }
+
+    public function test_join()
+    {
+        $search = new mock\Search();
+
+        $search->addSchema('schema2', 'foo');
+        $search->addSchema('schema1', '', array('foo.athird', '=', 'third'));
+        $this->assertEquals(2, count($search->schemas));
+
+        $this->assertEquals(1, count($search->joins));
+        $joincols = $search->joins['schema1'];
+        $this->assertEquals(2, count($joincols));
+        $this->assertEquals('schema2', $joincols[0]->getTable());
+        $this->assertEquals('athird', $joincols[0]->getLabel());
+        $this->assertEquals('schema1', $joincols[1]->getTable());
+        $this->assertEquals('third', $joincols[1]->getLabel());
+ 
+        $search->addColumn('%pageid%');
+        $search->addColumn('first');
+        $search->addFilter('afourth', 'fourth data', '=');
+
+        $result = $search->execute();
+        $count = $search->getCount();
+
+        // check result dimensions
+        $this->assertEquals(2, $count, 'result count'); // full result set
+        $this->assertEquals(2, count($result), 'result rows'); // wanted result set
+
+        // check the values
+        $this->assertEquals('page01', $result[0][0]->getValue());
+        $this->assertEquals('test:document', $result[1][0]->getValue());
+        $this->assertEquals('first data', $result[0][1]->getValue());
+        $this->assertEquals('first data', $result[1][1]->getValue());
+    }
+
+
+    public function invalidJoins_testdata() {
+        return array(
+            array('first', '<>', 'afirst'),
+            array('first', '=', 'third'),
+            array('foo.athird', '=', 'afirst'),
+            array('notaschema.first', '=', 'schema1.first'),
+            array('notacolumn', '=', 'schema1.first'),
+            array('foo.athird', '=', 'schema1.notacolumn'),
+        );
+    }
+    
+    /**
+     * @dataProvider invalidJoins_testdata
+     *
+     */
+    public function test_invalid_joins($lhs, $comp, $rhs)
+    {
+        $search = new mock\Search();
+        $search->addSchema('schema1');
+        $this->setExpectedException(meta\StructException::class);
+        $search->addSchema('schema2', 'foo', array($lhs, $comp, $rhs));
+    }
 }
diff --git a/_test/mock/Search.php b/_test/mock/Search.php
index 49a52d94..4c8dc34e 100644
--- a/_test/mock/Search.php
+++ b/_test/mock/Search.php
@@ -16,6 +16,8 @@ class Search extends meta\Search
 
     public $dynamicFilter = array();
 
+    public $joins = array();
+
     /**
      * Register a dummy function that always returns false
      */

From e3fd32bf8e7f485e7ebc9d29f2d3d4f02a8a39ee Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 22:34:16 +0000
Subject: [PATCH 04/27] Complete processing of JOIN configurations

Now just need to implement the joins when performing searches...
---
 helper/config.php         |  2 +-
 meta/ConfigParser.php     | 17 ++++-----------
 meta/Search.php           | 45 +++++++++++++++++++++++++++++++++------
 meta/SearchConfig.php     |  2 +-
 meta/SearchSQLBuilder.php |  1 +
 5 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/helper/config.php b/helper/config.php
index e5971dc3..7c62b13c 100644
--- a/helper/config.php
+++ b/helper/config.php
@@ -50,7 +50,7 @@ public function parseFilterLine($logic, $val)
      * @return array ($col, $comp, $value)
      * @throws StructException
      */
-    protected function parseFilter($val)
+    public function parseFilter($val)
     {
 
         $comps = Search::$COMPARATORS;
diff --git a/meta/ConfigParser.php b/meta/ConfigParser.php
index 19db9629..b68fc105 100644
--- a/meta/ConfigParser.php
+++ b/meta/ConfigParser.php
@@ -198,22 +198,13 @@ protected function parseSchema($val)
         $parts = explode(',', $val);
         $firsttable = null;
         foreach ($parts as $part) {
-            $words = array_pad(explode(' ', trim($part)), 2, '');
-            [$table, $alias, $on] = $words;
+            $segments = explode('ON', trim($part));
+            [$table, $alias] =  array_pad(explode(' ', trim($segments[0])), 2, '');
             $table = trim($table);
             $alias = trim($alias);
             if (!$table) continue;
-            if (is_null($firsttable)) $firsttable = $alias ? $alias : $table;
-            if ($on == 'ON') {
-                if (count($schemas) == 0) {
-                    throw new StructException('Can not specify JOIN ON for first schema'); 
-                }
-                $condition = $this->helper->parseFilter(
-                    implode(' ', array_slice($words, 3))
-                );
-                if ($condition[1] != '=') {
-                    throw new StructException('Only equality comparison is supported for JOIN conditions'); 
-                }
+            if (count($segments) > 1) {
+                $condition = $this->helper->parseFilter($segments[1]);
             } else {
                 $condition = [];
             }
diff --git a/meta/Search.php b/meta/Search.php
index 10d2acdc..b7194093 100755
--- a/meta/Search.php
+++ b/meta/Search.php
@@ -103,13 +103,46 @@ public function addSchema($table, $alias = '', $joinon = array())
 
         $this->schemas[$schema->getTable()] = $schema;
         if ($alias) $this->aliases[$alias] = $schema->getTable();
-        if (is_null($this->firstschema)) $this->firstschema = $scheam->getTable();
-        if (count($joinon) == 0) {
-            $joinon = array(
-                "{$schema->getTable()}.%pageid%", '=', "{$this->firstschema}.%pageid%"
-            );
+        if (is_null($this->firstschema)) {
+            $this->firstschema = $schema->getTable();
+            if (count($joinon) > 0) {
+                throw new StructException('JOIN condition not supported for first schema');
+            }
+        } else {
+            if (count($joinon) == 0) {
+                $joinon = array(
+                    "{$schema->getTable()}.%pageid%", '=', "{$this->firstschema}.%pageid%"
+                );
+            }
+            if ($joinon[1] != '=') {
+                throw new StructException('Only equality comparison is supported for JOIN conditions'); 
+            }
+            $this->joins[$schema->getTable()] = $this->getJoinColumns($schema, $joinon[0], $joinon[2]);
+        }
+    }
+
+    /**
+     * Returns the columns being matched against for a JOIN ... ON expression.
+     * 
+     * @param Schema $schema The schema being JOINed to the query
+     * @param string $left The LHS of the JOIN ON comparison
+     * @param string $right the RHS of the JOIN ON comparison
+     * @return array The first element is the LHS column object and second is the RHS
+     */
+    protected function getJoinColumns($schema, $left, $right) {
+        $lcol = $this->findColumn($left);
+        if ($lcol === false) {
+            throw new StructException('Unrecognoside field ' . $left);
+        }
+        $rcol = $this->findColumn($right);
+        if ($rcol === false) {
+            throw new StructException('Unrecognoside field ' . $right);
+        }
+        $table = $schema->getTable();
+        if (($lcol->getTable() != $table) == ($rcol->getTable() != $table)) {
+            throw new StructException("Exactly one side of ON condition $left = $right must be a column of $table" );
         }
-        $this->joins[$schema->getTable()] = $joinon;
+        return array($lcol, $rcol);
     }
 
     /**
diff --git a/meta/SearchConfig.php b/meta/SearchConfig.php
index 64910c80..8acad472 100644
--- a/meta/SearchConfig.php
+++ b/meta/SearchConfig.php
@@ -44,7 +44,7 @@ public function __construct($config, $dynamic = true)
 
         // setup schemas and columns
         if (!empty($config['schemas'])) foreach ($config['schemas'] as $schema) {
-            $this->addSchema($schema[0], $schema[1], $scheam[2]);
+            $this->addSchema($schema[0], $schema[1], $schema[2]);
         }
         if (!empty($config['cols'])) foreach ($config['cols'] as $col) {
             $this->addColumn($col);
diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index 977cdcfc..ac214300 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -36,6 +36,7 @@ public function addSchemas($schemas)
             $datatable = 'data_' . $schema->getTable();
             if ($first_table) {
                 // follow up tables
+                // TODO: Think I'm going to need to create a joinOn method for columns
                 $this->qb->addLeftJoin($first_table, $datatable, $datatable, "$first_table.pid = $datatable.pid");
             } else {
                 // first table

From 70ec5d2d2640703f8a0529effd0b9a149bf24a9d Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sun, 9 Apr 2023 16:05:54 +0100
Subject: [PATCH 05/27] Add (empty) join data to schema data

---
 _test/action/LookupAjaxTest.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/_test/action/LookupAjaxTest.php b/_test/action/LookupAjaxTest.php
index 5be00722..b0d7ba3c 100644
--- a/_test/action/LookupAjaxTest.php
+++ b/_test/action/LookupAjaxTest.php
@@ -36,10 +36,10 @@ public function testSaveGlobalDataEvent()
     {
         $testLabel = 'testcontent';
         global $INPUT;
-        $INPUT->post->set('schema', 'wikilookup');
+        $INPUT->post->set('schema', 'wikilookup', );
         $INPUT->post->set('entry', ['FirstFieldText' => $testLabel]);
         $INPUT->post->set('searchconf', json_encode([
-            'schemas' => [['wikilookup', '']],
+            'schemas' => [['wikilookup', '', []]],
             'cols' => ['*']
         ]));
         $call = 'plugin_struct_aggregationeditor_save';

From 8df37996801f2e751345b897fb01a39cc6bf9984 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 22:37:37 +0000
Subject: [PATCH 06/27] Initial implementation of custom JOINs for simple types

This supports types that need no special processing of the column data
returned from the SQLite database and which are single-valued. Further
work will be needed to support multi-valued columns. I also need to
implement some more sophisticated logic for some of the more complex
data types. Basically, any type which overrides the `filter` method
will also need an override for the purposes of JOINing.
---
 _test/SearchTest.php       |  4 ++--
 meta/Search.php            | 13 ++++++++++---
 meta/SearchSQLBuilder.php  | 11 +++++++++--
 types/AbstractBaseType.php | 37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/_test/SearchTest.php b/_test/SearchTest.php
index dd74076e..e6396105 100644
--- a/_test/SearchTest.php
+++ b/_test/SearchTest.php
@@ -235,8 +235,8 @@ public function test_search()
         $this->assertEquals(2, count($joincols));
         $this->assertInstanceOf(meta\PageColumn::class, $joincols[0]);
         $this->assertInstanceOf(meta\PageColumn::class, $joincols[1]);
-        $this->assertEquals('schema2', $joincols[0]->getTable());
-        $this->assertEquals('schema1', $joincols[1]->getTable());
+        $this->assertEquals('schema1', $joincols[0]->getTable());
+        $this->assertEquals('schema2', $joincols[1]->getTable());
 
         $search->addColumn('first');
         $this->assertEquals('schema1', $search->columns[0]->getTable());
diff --git a/meta/Search.php b/meta/Search.php
index b7194093..13bd0698 100755
--- a/meta/Search.php
+++ b/meta/Search.php
@@ -122,7 +122,9 @@ public function addSchema($table, $alias = '', $joinon = array())
     }
 
     /**
-     * Returns the columns being matched against for a JOIN ... ON expression.
+     * Returns the columns being matched against for a JOIN ... ON
+     * expression. The result will be ordered such that the first
+     * column is the one from a previously-joined schema.
      * 
      * @param Schema $schema The schema being JOINed to the query
      * @param string $left The LHS of the JOIN ON comparison
@@ -139,10 +141,15 @@ protected function getJoinColumns($schema, $left, $right) {
             throw new StructException('Unrecognoside field ' . $right);
         }
         $table = $schema->getTable();
-        if (($lcol->getTable() != $table) == ($rcol->getTable() != $table)) {
+        $left_is_old_table = $lcol->getTable() != $table;
+        if ($left_is_old_table == ($rcol->getTable() != $table)) {
             throw new StructException("Exactly one side of ON condition $left = $right must be a column of $table" );
         }
-        return array($lcol, $rcol);
+        if ($left_is_old_table) {
+            return array($lcol, $rcol);
+        } else {
+            return array($rcol, $lcol);
+        }
     }
 
     /**
diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index ac214300..d2b98e6e 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -36,8 +36,15 @@ public function addSchemas($schemas)
             $datatable = 'data_' . $schema->getTable();
             if ($first_table) {
                 // follow up tables
-                // TODO: Think I'm going to need to create a joinOn method for columns
-                $this->qb->addLeftJoin($first_table, $datatable, $datatable, "$first_table.pid = $datatable.pid");
+                [$lcol, $rcol] = $this->joins[$schema->getTable()];
+                $lefttable = 'data_' . $lcol->getTable();
+                $righttable = 'data_' . $rcol->getTable();
+                $add = new QueryBuilderWhere($QB);
+                $lcol->getType()->joinCondition(
+                    $add, $lefttable, $lcol->getColName(),
+                    $righttable, $rcol->getColName(), $rcol->getType()
+                );
+                $QB->addLeftJoin($lefttable, $righttable, $righttable, $add->toSQL());
             } else {
                 // first table
                 $this->qb->addTable($datatable);
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 973599da..36aaebe3 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -390,6 +390,43 @@ public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $va
         }
     }
 
+    /**
+     * Place a condition expression in $add which $left_table and
+     * $right_table can be JOINed ON.  Semantically, this provides an
+     * equality comparison between two columns in the two
+     * schemas. However, in practice it may require more complex
+     * logic, including additional JOINs to pull in other data or
+     * handle multi-valued columns.
+     *
+     * @param QueryBuilderWhere $add The condition ON which to JOIN the tables
+     * @param string $left_table The name of the left table being JOINed
+     * @param string $left_colname The name of the column in the left table being compared against for the JOIN
+     * @param string $right_table The name of the right table being JOINed
+     * @param string $right_colname The name of hte column in the right table being compared against for hte JOIN
+     * @param AbstractBaseType $right_coltype The type of $right_colname
+     */
+    public function joinCondition(QueryBuilderWhere $add, $left_table, $left_colname, $right_table, $right_colname, $right_coltype)
+    {
+        $lhs = $this->joinArgument($add, $left_table, $left_colname);
+        $rhs = $right_coltype->joinArgument($add, $right_table, $right_colname);
+        $add->where('AND', "$lhs = $rhs");
+    }
+
+    /**
+     * Returns an expression for one side of the equality-comparison
+     * used when JOINing schemas in aggregations. It may add
+     * additional conditions to the $add expression or JOIN other
+     * tables, as needed.
+     *
+     * @param QueryBuilderWhere $add The condition ON which to JOIN the tables. May not be used.
+     * @param string $table Name of the table being JOINed
+     * @param string $colname Name of the column being JOINed ON
+     * @return string One side of the equality comparion being used for the JOIN
+     */
+    protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
+        return "$table.$colname";
+    }
+    
     /**
      * Add the proper selection for this type to the current Query
      *

From a1c85df0d7b6f8684907f94b7b0e6bfb8de08405 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Mon, 10 Apr 2023 11:45:41 +0100
Subject: [PATCH 07/27] Move logic for selecting columns inside column types

---
 types/AbstractBaseType.php | 53 ++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 36aaebe3..407bc89e 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -361,6 +361,30 @@ public function renderTagCloudLink($value, \Doku_Renderer $R, $mode, $page, $fil
         $R->internallink("$page?$filter", $value);
     }
 
+    /**
+     * Creates a JOIN to handle multi-valued columns.
+     *
+     * @param QueryBuilder $QB
+     * @param string $datatable The table for the schema in question
+     * @param string $multitable The table containing multi-valued data for this schema
+     * @param int $colref The ID of the multi-valued column
+     * @param bool $test_rid Whether to require RIDs to be equal in the JOIN condition
+     * @return string Alias for the multi-table
+     */
+    public function joinMulti(QueryBuilder $QB, $datatable, $multitable, $colref, $test_rid = true) {
+        $MN = $QB->generateTableAlias('M');
+        $condition = "$datatable.pid = $MN.pid";
+        if ($test_rid) $condition .= "AND $datatable.rid = $MN.rid";
+        $condition .= "AND $datatable.rev = $MN.rev AND $MN.colref = $colref";
+        $QB->addLeftJoin(
+            $datatable,
+            $multitable,
+            $MN,
+            $condition
+        );
+        return $MN;
+    }
+    
     /**
      * This function is used to modify an aggregation query to add a filter
      * for the given column matching the given value. A type should add at
@@ -443,13 +467,32 @@ protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
      * current page context for a join or sub select.
      *
      * @param QueryBuilder $QB
-     * @param string $tablealias The table the currently saved value(s) are stored in
+     * @param string $tablename The plane name of the table the currently saved value(s) are stored in (no prefixed with data_ or multi_)
      * @param string $colname The column name on above table
+     * @param bool $test_rid Whether to require RIDs to be equal if JOINing multi-table
      * @param string $alias The added selection *has* to use this column alias
-     */
-    public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
-    {
-        $QB->addSelectColumn($tablealias, $colname, $alias);
+     * @param string|null $concat_sep Seperator to concatenate mutli-values together. If null, don't perform concatentation.
+     */
+    public function select(QueryBuilder $QB, $tablename, $multialias, $alias, $test_rid = true, $concat_sep = null)
+    {
+        $datatable = 'data_' . $tablename;
+        if ($this->isMulti()) {
+            $multitable = 'multi_' . $tablename;
+            $colref = $this->getContext()->getColref();
+            $datatable = $this->joinMulti($QB, $datatable, $multitable, $colref, $test_rid);
+            $colname = 'value';
+        } else {
+            $colname = $this->getContext()->getColName();
+        }
+        $QB->addSelectColumn($datatable, $colname, $alias);
+        if ($this->isMulti()) {
+            if (!is_null($concat_multi)) {
+                $sel = $QB->getSelectStatement($alias);
+                $QB->addSelectStatement("GROUP_CONCAT($sel, '$concat_sep')", $alias);
+            }
+        } else {
+            $QB->addGroupByStatement($alias);
+        }
     }
 
     /**

From 395df62009b10b42f38a382695c40613aa20d1f9 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 22:52:59 +0000
Subject: [PATCH 08/27] Refactored selecting columns to hide complexity

Moving the left-join needed for multi-columns into a separate method
should prove useful for filtering and joining as well.
---
 _test/AccessTableDataSQLTest.php   | 10 +++++-----
 _test/mock/AccessTableDataNoDB.php |  8 ++++++--
 meta/AccessTable.php               | 22 +++------------------
 meta/QueryBuilder.php              |  7 ++++---
 meta/Search.php                    | 28 ++++++++++++++++++++-------
 meta/SearchCloud.php               | 31 ++++++++++--------------------
 meta/SearchSQLBuilder.php          | 27 ++++----------------------
 types/AbstractBaseType.php         |  2 +-
 types/AutoSummary.php              |  2 +-
 types/DateTime.php                 |  2 +-
 types/Lookup.php                   |  6 +++---
 types/Page.php                     |  4 ++--
 types/User.php                     |  4 ++--
 13 files changed, 63 insertions(+), 90 deletions(-)

diff --git a/_test/AccessTableDataSQLTest.php b/_test/AccessTableDataSQLTest.php
index 5cc02360..ff5ee096 100644
--- a/_test/AccessTableDataSQLTest.php
+++ b/_test/AccessTableDataSQLTest.php
@@ -51,12 +51,12 @@ public static function buildGetDataSQL_testdata()
                 "SELECT DATA.pid AS PID,
                         DATA.col1 AS out1,
                         DATA.col2 AS out2,
-                        GROUP_CONCAT_DISTINCT(M3.value,'" . Search::CONCAT_SEPARATOR . "') AS out3
+                        GROUP_CONCAT_DISTINCT(M1.value,'" . Search::CONCAT_SEPARATOR . "') AS out3
                    FROM data_testtable AS DATA
-                   LEFT OUTER JOIN multi_testtable AS M3
-                     ON DATA.pid = M3.pid
-                    AND DATA.rev = M3.rev
-                    AND M3.colref = 3
+                   LEFT OUTER JOIN multi_testtable AS M1
+                     ON DATA.pid = M1.pid
+                    AND DATA.rev = M1.rev
+                    AND M1.colref = 3
                   WHERE (DATA.pid = ?
                     AND DATA.rev = ?)
                GROUP BY DATA.pid,out1,out2",
diff --git a/_test/mock/AccessTableDataNoDB.php b/_test/mock/AccessTableDataNoDB.php
index 052493c3..64676a1d 100644
--- a/_test/mock/AccessTableDataNoDB.php
+++ b/_test/mock/AccessTableDataNoDB.php
@@ -34,11 +34,15 @@ public function setColumns($singles, $multis)
         $sort = 0;
         foreach ($singles as $single) {
             $sort += 1;
-            $this->schema->columns[] = new Column($sort, new $single(), $sort);
+            $col = new Column($sort, new $single(), $sort);
+            $col->getType()->setContext($col);
+            $this->schema->columns[] = $col;
         }
         foreach ($multis as $multi) {
             $sort += 1;
-            $this->schema->columns[] = new Column($sort, new $multi(null, null, true), $sort);
+            $col = new Column($sort, new $multi(null, null, true), $sort);
+            $col->getType()->setContext($col);
+            $this->schema->columns[] = $col;
         }
     }
 
diff --git a/meta/AccessTable.php b/meta/AccessTable.php
index 353147a4..f7144145 100644
--- a/meta/AccessTable.php
+++ b/meta/AccessTable.php
@@ -505,25 +505,9 @@ protected function buildGetDataSQL($idColumn = 'pid')
         $QB->addGroupByStatement("DATA.$idColumn");
 
         foreach ($this->schema->getColumns(false) as $col) {
-            $colref = $col->getColref();
-            $colname = 'col' . $colref;
-            $outname = 'out' . $colref;
-
-            if ($col->getType()->isMulti()) {
-                $tn = 'M' . $colref;
-                $QB->addLeftJoin(
-                    'DATA',
-                    $mtable,
-                    $tn,
-                    "DATA.$idColumn = $tn.$idColumn AND DATA.rev = $tn.rev AND $tn.colref = $colref"
-                );
-                $col->getType()->select($QB, $tn, 'value', $outname);
-                $sel = $QB->getSelectStatement($outname);
-                $QB->addSelectStatement("GROUP_CONCAT_DISTINCT($sel, '$sep')", $outname);
-            } else {
-                $col->getType()->select($QB, 'DATA', $colname, $outname);
-                $QB->addGroupByStatement($outname);
-            }
+            $col->getType()->select(
+                $QB, 'DATA', $mtable, 'out' . $col->getColref(), false, $sep
+            );
         }
 
         $pl = $QB->addValue($this->{$idColumn});
diff --git a/meta/QueryBuilder.php b/meta/QueryBuilder.php
index 1eb42fe2..4bde4079 100644
--- a/meta/QueryBuilder.php
+++ b/meta/QueryBuilder.php
@@ -23,6 +23,8 @@ class QueryBuilder
     /** @var  string[] */
     protected $groupby = [];
 
+    protected $aliasCount = 0;
+
     /**
      * QueryBuilder constructor.
      */
@@ -196,9 +198,8 @@ public function addValue($value)
      */
     public function generateTableAlias($prefix = 'T')
     {
-        static $count = 0;
-        $count++;
-        return $prefix . $count;
+        $this->aliasCount++;
+        return $prefix . $this->aliasCount;
     }
 
     /**
diff --git a/meta/Search.php b/meta/Search.php
index 13bd0698..db9139e1 100755
--- a/meta/Search.php
+++ b/meta/Search.php
@@ -678,25 +678,39 @@ public function findColumn($colname, $strict = false)
 
         // add "fake" column for special col
         if ($colname == '%pageid%') {
-            return new PageColumn(0, new Page(), $table_or_first);
+            $col = new PageColumn(0, new Page(), $table_or_first);
+            $col->getType()->setContext($col);
+            return $col;
         }
         if ($colname == '%title%') {
-            return new PageColumn(0, new Page(['usetitles' => true]), $table_or_first);
+            $col = new PageColumn(0, new Page(['usetitles' => true]), $table_or_first);
+            $col->getType()->setContext($col);
+            return $col;
         }
         if ($colname == '%lastupdate%') {
-            return new RevisionColumn(0, new DateTime(), $table_or_first);
+            $col = new RevisionColumn(0, new DateTime(), $table_or_first);
+            $col->getType()->setContext($col);
+            return $col;
         }
         if ($colname == '%lasteditor%') {
-            return new UserColumn(0, new User(), $table_or_first);
+            $col = new UserColumn(0, new User(), $table_or_first);
+            $col->getType()->setContext($col);
+            return $col;
         }
         if ($colname == '%lastsummary%') {
-            return new SummaryColumn(0, new AutoSummary(), $table_or_first);
+            $col = new SummaryColumn(0, new AutoSummary(), $table_or_first);
+            $col->getType()->setContext($col);
+            return $col;
         }
         if ($colname == '%rowid%') {
-            return new RowColumn(0, new Decimal(), $table_or_first);
+            $col = new RowColumn(0, new Decimal(), $table_or_first);
+            $col->getType()->setContext($col);
+            return $col;
         }
         if ($colname == '%published%') {
-            return new PublishedColumn(0, new Decimal(), $schema_list[0]);
+            $col = new PublishedColumn(0, new Decimal(), $schema_list[0]);
+            $col->getType()->setContext($col);
+            return $col;
         }
 
         /*
diff --git a/meta/SearchCloud.php b/meta/SearchCloud.php
index 8a53a0e5..0feb9f3c 100644
--- a/meta/SearchCloud.php
+++ b/meta/SearchCloud.php
@@ -56,29 +56,18 @@ public function getSQL()
         $QB->filters()->where('AND', 'tag IS NOT \'\'');
 
         $col = $this->columns[0];
+        $col->getType()->select(
+            $QB, 'data_' . $datatable, 'multi_' . $col->getTable(), 'tag', true
+        );
+        
+        $QB->addSelectStatement('COUNT(tag)', 'count');
+        $QB->addSelectColumn('schema_assignments', 'assigned', 'ASSIGNED');
         if ($col->isMulti()) {
-            $multitable = "multi_{$col->getTable()}";
-            $MN = $QB->generateTableAlias('M');
-
-            $QB->addLeftJoin(
-                $datatable,
-                $multitable,
-                $MN,
-                "$datatable.pid = $MN.pid AND
-                     $datatable.rid = $MN.rid AND
-                     $datatable.rev = $MN.rev AND
-                     $MN.colref = {$col->getColref()}"
-            );
-
-            $col->getType()->select($QB, $MN, 'value', 'tag');
-            $colname = $MN . '.value';
-        } else {
-            $col->getType()->select($QB, $datatable, $col->getColName(), 'tag');
-            $colname = $datatable . '.' . $col->getColName();
+            // This GROUP BY was added with the SELECT statement for a
+            // single-valued column, just need to make sure it's added
+            // in the case of multi-valued as well.
+            $QB->addGroupByStatement('tag');
         }
-        $QB->addSelectStatement("COUNT($colname)", 'count');
-        $QB->addSelectColumn('schema_assignments', 'assigned', 'ASSIGNED');
-        $QB->addGroupByStatement('tag');
         $QB->addOrderBy('count DESC');
 
         [$sql, $opts] = $QB->getSQL();
diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index d2b98e6e..db2b93e9 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -91,29 +91,10 @@ public function addColumns($columns)
         $sep = Search::CONCAT_SEPARATOR;
         $n = 0;
         foreach ($columns as $col) {
-            $CN = 'C' . $n++;
-
-            if ($col->isMulti()) {
-                $datatable = "data_{$col->getTable()}";
-                $multitable = "multi_{$col->getTable()}";
-                $MN = $this->qb->generateTableAlias('M');
-
-                $this->qb->addLeftJoin(
-                    $datatable,
-                    $multitable,
-                    $MN,
-                    "$datatable.pid = $MN.pid AND $datatable.rid = $MN.rid AND
-                     $datatable.rev = $MN.rev AND
-                     $MN.colref = {$col->getColref()}"
-                );
-
-                $col->getType()->select($this->qb, $MN, 'value', $CN);
-                $sel = $this->qb->getSelectStatement($CN);
-                $this->qb->addSelectStatement("GROUP_CONCAT_DISTINCT($sel, '$sep')", $CN);
-            } else {
-                $col->getType()->select($this->qb, 'data_' . $col->getTable(), $col->getColName(), $CN);
-                $this->qb->addGroupByStatement($CN);
-            }
+            $col->getType()->select(
+                $QB, 'data_' . $col->getTable(), 'multi_' . $col->getTable(),
+                'C' . $n++, true, $sep
+            );
         }
     }
 
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 407bc89e..58e710a9 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -488,7 +488,7 @@ public function select(QueryBuilder $QB, $tablename, $multialias, $alias, $test_
         if ($this->isMulti()) {
             if (!is_null($concat_multi)) {
                 $sel = $QB->getSelectStatement($alias);
-                $QB->addSelectStatement("GROUP_CONCAT($sel, '$concat_sep')", $alias);
+                $QB->addSelectStatement("GROUP_CONCAT_DISTINCT($sel, '$concat_sep')", $alias);
             }
         } else {
             $QB->addGroupByStatement($alias);
diff --git a/types/AutoSummary.php b/types/AutoSummary.php
index 902bc2bc..1a3c7dca 100644
--- a/types/AutoSummary.php
+++ b/types/AutoSummary.php
@@ -16,7 +16,7 @@ class AutoSummary extends AbstractBaseType
      * @param string $colname
      * @param string $alias
      */
-    public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
+    public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
     {
         $rightalias = $QB->generateTableAlias();
         $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
diff --git a/types/DateTime.php b/types/DateTime.php
index 3c8678b2..d8a4c89f 100644
--- a/types/DateTime.php
+++ b/types/DateTime.php
@@ -102,7 +102,7 @@ public function validate($rawvalue)
      * @param string $colname
      * @param string $alias
      */
-    public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
+    public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
     {
         $col = "$tablealias.$colname";
 
diff --git a/types/Lookup.php b/types/Lookup.php
index edd9d76f..d9742755 100644
--- a/types/Lookup.php
+++ b/types/Lookup.php
@@ -230,12 +230,12 @@ public function compareValue($value)
      * @param string $colname
      * @param string $alias
      */
-    public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
+    public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
     {
         $schema = 'data_' . $this->config['schema'];
         $column = $this->getLookupColumn();
         if (!$column) {
-            parent::select($QB, $tablealias, $colname, $alias);
+            parent::selectCol($QB, $tablealias, $colname, $alias);
             return;
         }
 
@@ -248,7 +248,7 @@ public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
             "$tablealias.$colname = STRUCT_JSON($rightalias.pid, CAST($rightalias.rid AS DECIMAL)) " .
             "AND $rightalias.latest = 1"
         );
-        $column->getType()->select($QB, $rightalias, $field, $alias);
+        $column->getType()->selectCol($QB, $rightalias, $field, $alias);
         $sql = $QB->getSelectStatement($alias);
         $QB->addSelectStatement("STRUCT_JSON($tablealias.$colname, $sql)", $alias);
     }
diff --git a/types/Page.php b/types/Page.php
index 0d2d1270..87814367 100644
--- a/types/Page.php
+++ b/types/Page.php
@@ -121,10 +121,10 @@ public function handleAjax()
      * @param string $colname
      * @param string $alias
      */
-    public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
+    public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
     {
         if (!$this->config['usetitles']) {
-            parent::select($QB, $tablealias, $colname, $alias);
+            parent::selectCol($QB, $tablealias, $colname, $alias);
             return;
         }
         $rightalias = $QB->generateTableAlias();
diff --git a/types/User.php b/types/User.php
index 93aa2c08..0c2f9b39 100644
--- a/types/User.php
+++ b/types/User.php
@@ -106,7 +106,7 @@ public function handleAjax()
      * @param string $colname
      * @param string $alias
      */
-    public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
+    public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
     {
         if (is_a($this->context, 'dokuwiki\plugin\struct\meta\UserColumn')) {
             $rightalias = $QB->generateTableAlias();
@@ -115,7 +115,7 @@ public function select(QueryBuilder $QB, $tablealias, $colname, $alias)
             return;
         }
 
-        parent::select($QB, $tablealias, $colname, $alias);
+        parent::selectCol($QB, $tablealias, $colname, $alias);
     }
 
     /**

From 59b0a867506b043287dd423dbe2c4aeed3fd58b7 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 23:11:46 +0000
Subject: [PATCH 09/27] Add tests that page exists and is accessible for custom
 joins

---
 _test/AccessTableDataReplacementTest.php |  2 +-
 meta/SearchSQLBuilder.php                | 43 +++++++------
 types/AbstractBaseType.php               | 77 ++++++++++++++++--------
 3 files changed, 79 insertions(+), 43 deletions(-)

diff --git a/_test/AccessTableDataReplacementTest.php b/_test/AccessTableDataReplacementTest.php
index e6388425..bc924748 100644
--- a/_test/AccessTableDataReplacementTest.php
+++ b/_test/AccessTableDataReplacementTest.php
@@ -184,7 +184,7 @@ public function test_DataFiltersAsSubQuery($inputFilterLines, $expectedFilterWhe
                         AND PAGEEXISTS(data_bar.pid) = 1
                         AND (
                             data_bar.rid != 0
-                            OR (ASSIGNED = 1 OR ASSIGNED IS NULL)
+                            OR (data_bar_ASSIGNED = 1 OR data_bar_ASSIGNED IS NULL)
                         )
                     )
                 )
diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index db2b93e9..c2320d01 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -32,32 +32,32 @@ public function addSchemas($schemas)
     {
         // basic tables
         $first_table = '';
+        $added_schemas = [];
         foreach ($schemas as $schema) {
             $datatable = 'data_' . $schema->getTable();
+            $new_pid = false;
             if ($first_table) {
                 // follow up tables
                 [$lcol, $rcol] = $this->joins[$schema->getTable()];
-                $lefttable = 'data_' . $lcol->getTable();
-                $righttable = 'data_' . $rcol->getTable();
-                $add = new QueryBuilderWhere($QB);
-                $lcol->getType()->joinCondition(
-                    $add, $lefttable, $lcol->getColName(),
-                    $righttable, $rcol->getColName(), $rcol->getType()
-                );
-                $QB->addLeftJoin($lefttable, $righttable, $righttable, $add->toSQL());
+                if ($lcol->getLabel() == '%pageid%' and $rcol->getLabel() == '%pageid%') {
+                    // Simple (default) case where we join on page IDs
+                    $QB->addLeftJoin($first_table, $datatable, $datatable, "$first_table.pid = $datatable.pid");
+                } else {
+                    // Custom join on some other columns
+                    $lefttable = 'data_' . $lcol->getTable();
+                    $righttable = 'data_' . $rcol->getTable();
+                    $add = new QueryBuilderWhere($QB);
+                    // TODO: Only tricky columns to join over are pages, as they are the only ones that add some OR clauses. For rest, think I can modify filter to handle case of $value being another column.
+                    $lcol->getType()->joinCondition(
+                        $add, $lefttable, $lcol->getColName(),
+                        $righttable, $rcol->getColName(), $rcol->getType()
+                    );
+                    $QB->addLeftJoin($lefttable, $righttable, $righttable, $add->toSQL());
+                }
             } else {
                 // first table
                 $this->qb->addTable($datatable);
 
-                // add conditional page clauses if pid has a value
-                $subAnd = $this->qb->filters()->whereSubAnd();
-                $subAnd->whereAnd("$datatable.pid = ''");
-                $subOr = $subAnd->whereSubOr();
-                $subOr->whereAnd("GETACCESSLEVEL($datatable.pid) > 0");
-                $subOr->whereAnd("PAGEEXISTS($datatable.pid) = 1");
-                // make sure to check assignment for page data only
-                $subOr->whereAnd("($datatable.rid != 0 OR (ASSIGNED = 1 OR ASSIGNED IS NULL))");
-
                 // add conditional schema assignment check
                 $this->qb->addLeftJoin(
                     $datatable,
@@ -68,6 +68,15 @@ public function addSchemas($schemas)
                     AND schema_assignments.tbl = '{$schema->getTable()}'"
                 );
 
+                // add conditional page clauses if pid has a value
+                $subAnd = $this->qb->filters()->whereSubAnd();
+                $subAnd->whereAnd("$datatable.pid = ''");
+                $subOr = $subAnd->whereSubOr();
+                $subOr->whereAnd("GETACCESSLEVEL($datatable.pid) > 0");
+                $subOr->whereAnd("PAGEEXISTS($datatable.pid) = 1");
+                // make sure to check assignment for page data only
+                $subOr->whereAnd("($datatable.rid != 0 OR (ASSIGNED = 1 OR ASSIGNED IS NULL))");
+
                 $this->qb->addSelectColumn($datatable, 'rid');
                 $this->qb->addSelectColumn($datatable, 'pid', 'PID');
                 $this->qb->addSelectColumn($datatable, 'rev');
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 58e710a9..146b8a7d 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -373,8 +373,8 @@ public function renderTagCloudLink($value, \Doku_Renderer $R, $mode, $page, $fil
      */
     public function joinMulti(QueryBuilder $QB, $datatable, $multitable, $colref, $test_rid = true) {
         $MN = $QB->generateTableAlias('M');
-        $condition = "$datatable.pid = $MN.pid";
-        if ($test_rid) $condition .= "AND $datatable.rid = $MN.rid";
+        $condition = "$datatable.pid = $MN.pid ";
+        if ($test_rid) $condition .= "AND $datatable.rid = $MN.rid ";
         $condition .= "AND $datatable.rev = $MN.rev AND $MN.colref = $colref";
         $QB->addLeftJoin(
             $datatable,
@@ -434,6 +434,18 @@ public function joinCondition(QueryBuilderWhere $add, $left_table, $left_colname
         $lhs = $this->joinArgument($add, $left_table, $left_colname);
         $rhs = $right_coltype->joinArgument($add, $right_table, $right_colname);
         $add->where('AND', "$lhs = $rhs");
+        $AN = $add->getQB()->generateTableAlias('A');
+        $subquery = "(SELECT assigned
+                     FROM schema_assignments AS $AN
+                     WHERE $left_table.pid != '' AND
+                           $left_table.pid = $AN.pid AND
+                           $AN.tbl = '{$this->getContext()->getTable()}')";
+        $subAnd = $add->whereSubAnd();
+        $subAnd->whereOr("$left_table.pid = ''");
+        $subOr = $subAnd->whereSubOr();
+        $subOr->whereAnd("GETACCESSLEVEL($left_table.pid) > 0");
+        $subOr->whereAnd("PAGEEXISTS($left_table.pid) = 1");
+        $subOr->whereAnd("($subquery = 1 OR $subquery IS NULL)");
     }
 
     /**
@@ -452,41 +464,29 @@ protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
     }
     
     /**
-     * Add the proper selection for this type to the current Query
-     *
-     * The default implementation here should be good for nearly all types, it simply
-     * passes the given parameters to the query builder. But type may do more fancy
-     * stuff here, eg. join more tables or select multiple values and combine them to
-     * JSON. If you do, be sure implement a fitting rawValue() method.
-     *
-     * The passed $tablealias.$columnname might be a data_* table (referencing a single
-     * row) or a multi_* table (referencing multiple rows). In the latter case the
-     * multi table has already been joined with the proper conditions.
-     *
-     * You may assume a column alias named 'PID' to be available, should you need the
-     * current page context for a join or sub select.
+     * Add the proper selection for this type to the current Query. Handles the
+     * possibility of multi-valued columns.
      *
      * @param QueryBuilder $QB
-     * @param string $tablename The plane name of the table the currently saved value(s) are stored in (no prefixed with data_ or multi_)
-     * @param string $colname The column name on above table
-     * @param bool $test_rid Whether to require RIDs to be equal if JOINing multi-table
+     * @param string $singletable The name of the table the saved value(s) are stored in, if the column is single-valued
+     * @param string $multitable The name of the table the values are stored in if the column is multi-valued
      * @param string $alias The added selection *has* to use this column alias
+     * @param bool $test_rid Whether to require RIDs to be equal if JOINing multi-table
      * @param string|null $concat_sep Seperator to concatenate mutli-values together. If null, don't perform concatentation.
      */
-    public function select(QueryBuilder $QB, $tablename, $multialias, $alias, $test_rid = true, $concat_sep = null)
+    public function select(QueryBuilder $QB, $singletable, $multitable, $alias, $test_rid = true, $concat_sep = null, )
     {
-        $datatable = 'data_' . $tablename;
         if ($this->isMulti()) {
-            $multitable = 'multi_' . $tablename;
             $colref = $this->getContext()->getColref();
-            $datatable = $this->joinMulti($QB, $datatable, $multitable, $colref, $test_rid);
+            $datatable = $this->joinMulti($QB, $singletable, $multitable, $colref, $test_rid);
             $colname = 'value';
         } else {
+            $datatable = $singletable;
             $colname = $this->getContext()->getColName();
         }
-        $QB->addSelectColumn($datatable, $colname, $alias);
+        $this->selectCol($QB, $datatable, $colname, $alias);
         if ($this->isMulti()) {
-            if (!is_null($concat_multi)) {
+            if (!is_null($concat_sep)) {
                 $sel = $QB->getSelectStatement($alias);
                 $QB->addSelectStatement("GROUP_CONCAT_DISTINCT($sel, '$concat_sep')", $alias);
             }
@@ -495,6 +495,33 @@ public function select(QueryBuilder $QB, $tablename, $multialias, $alias, $test_
         }
     }
 
+    /**
+     * Internal function to add the proper selection for a column of this type to the
+     * current Query. It is called from the `select` method, after any joins needed
+     * for multi-valued tables are handled.
+     *
+     * The default implementation here should be good for nearly all types, it simply
+     * passes the given parameters to the query builder. But type may do more fancy
+     * stuff here, eg. join more tables or select multiple values and combine them to
+     * JSON. If you do, be sure implement a fitting rawValue() method.
+     *
+     * The passed $tablealias.$columnname might be a data_* table (referencing a single
+     * row) or a multi_* table (referencing multiple rows). In the latter case the
+     * multi table has already been joined with the proper conditions.
+     *
+     * You may assume a column alias named 'PID' to be available, should you need the
+     * current page context for a join or sub select.
+     *
+     * @param QueryBuilder $QB
+     * @param string $tablealias The table the currently saved value(s) are stored in
+     * @param string $colname The column name on above table
+     * @param string $alias The added selection *has* to use this column alias
+     */
+    protected function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
+    {
+        $QB->addSelectColumn($tablealias, $colname, $alias);
+    }
+
     /**
      * Sort results by this type
      *
@@ -505,7 +532,7 @@ public function select(QueryBuilder $QB, $tablename, $multialias, $alias, $test_
      * @param string $tablealias The table the currently saved value is stored in
      * @param string $colname The column name on above table (always single column!)
      * @param string $order either ASC or DESC
-     * @see select() you probably want to implement this,
+     * @see selectCol() you probably want to implement this,
      * too.
      *
      */

From 4ff5cceeeab02dadaad5a2aa2a81937330d0377a Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 23:20:03 +0000
Subject: [PATCH 10/27] Refactored filtering so can reuse logic for joins

---
 _test/AccessTableDataReplacementTest.php |  2 +-
 types/AbstractBaseType.php               | 48 ++++++++++++++++++++++--
 types/AutoSummary.php                    | 22 +++++------
 types/DateTime.php                       | 31 +++++++--------
 types/Decimal.php                        | 31 +++++++--------
 types/Lookup.php                         | 35 +++++++++++------
 types/Page.php                           | 42 ++++++++++++---------
 types/Tag.php                            | 35 ++++++++---------
 types/TraitFilterPrefix.php              | 47 +++++++++--------------
 types/User.php                           | 27 +++++--------
 10 files changed, 177 insertions(+), 143 deletions(-)

diff --git a/_test/AccessTableDataReplacementTest.php b/_test/AccessTableDataReplacementTest.php
index bc924748..e6388425 100644
--- a/_test/AccessTableDataReplacementTest.php
+++ b/_test/AccessTableDataReplacementTest.php
@@ -184,7 +184,7 @@ public function test_DataFiltersAsSubQuery($inputFilterLines, $expectedFilterWhe
                         AND PAGEEXISTS(data_bar.pid) = 1
                         AND (
                             data_bar.rid != 0
-                            OR (data_bar_ASSIGNED = 1 OR data_bar_ASSIGNED IS NULL)
+                            OR (ASSIGNED = 1 OR ASSIGNED IS NULL)
                         )
                     )
                 )
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 146b8a7d..166c0aa3 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -403,17 +403,59 @@ public function joinMulti(QueryBuilder $QB, $datatable, $multitable, $colref, $t
      */
     public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
     {
+        $compareVal = $this->getSqlCompareValue($add, $tablealias, $colname, $op);
         /** @var QueryBuilderWhere $add Where additionional queries are added to */
         if (is_array($value)) {
             $add = $add->where($op); // sub where group
             $op = 'OR';
         }
         foreach ((array)$value as $item) {
-            $pl = $add->getQB()->addValue($item);
-            $add->where($op, "$tablealias.$colname $comp $pl");
+            if (is_array($compareVal)) {
+                $sub = $add->where($op);
+                $op = 'OR'; // safe to do, as if the previous line is
+                            // executed again it means $value is an
+                            // array and $op was already 'OR' anyway
+            } else {
+                $sub = $add;
+            }
+            $pl = $this->getSqlConstantValue($add->getQB()->addValue($item));
+            foreach ((array)$compareVal as $lhs) {
+                $sub->where($op, "$lhs $comp $pl");
+            }
         }
     }
 
+    /**
+     * This function provides the SQL expression for this column which is used to
+     * compare against in a filter expression or a JOIN condition. In simple cases
+     * that is all it will need to do. However, for some columnt types, it may
+     * need to add additional logic to the conditional expression or make
+     * additional JOINs.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return string|array The SQL expression to be used on one side of the comparison operator
+     */
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op) {
+        return "$tablealias.$colname";
+    }
+
+    /**
+     * Handle the value that a column is being compared against. In
+     * most cases this method will just return the value unchanged,
+     * but for some types it may be necessary to preform some sort of
+     * transformation (e.g., casting it to a decimal).
+     *
+     * @param string $value The value a column is being compared to
+     * @return string A SQL expression processing the value in some way.
+     */
+    protected function getSqlConstantValue($value) {
+        return $value;
+    }
+
     /**
      * Place a condition expression in $add which $left_table and
      * $right_table can be JOINed ON.  Semantically, this provides an
@@ -474,7 +516,7 @@ protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
      * @param bool $test_rid Whether to require RIDs to be equal if JOINing multi-table
      * @param string|null $concat_sep Seperator to concatenate mutli-values together. If null, don't perform concatentation.
      */
-    public function select(QueryBuilder $QB, $singletable, $multitable, $alias, $test_rid = true, $concat_sep = null, )
+    public function select(QueryBuilder $QB, $singletable, $multitable, $alias, $test_rid = true, $concat_sep = null)
     {
         if ($this->isMulti()) {
             $colref = $this->getContext()->getColref();
diff --git a/types/AutoSummary.php b/types/AutoSummary.php
index 1a3c7dca..72185399 100644
--- a/types/AutoSummary.php
+++ b/types/AutoSummary.php
@@ -41,22 +41,18 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
     /**
      * When using `%lastsummary%`, we need to compare against the `title` table.
      *
-     * @param QueryBuilderWhere $add
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string|\string[] $value
-     * @param string $op
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return string The SQL expression to be used on one side of the comparison operator
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
-    {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op) {
         $QB = $add->getQB();
         $rightalias = $QB->generateTableAlias();
         $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
 
-        // compare against page and title
-        $sub = $add->where($op);
-        $pl = $QB->addValue($value);
-        $sub->whereOr("$rightalias.lastsummary $comp $pl");
-    }
+        return "$rightalias.lastsummary";
+    }    
 }
diff --git a/types/DateTime.php b/types/DateTime.php
index d8a4c89f..9c26fab7 100644
--- a/types/DateTime.php
+++ b/types/DateTime.php
@@ -116,16 +116,19 @@ public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
         $QB->addSelectStatement($col, $alias);
     }
 
+
     /**
-     * @param QueryBuilderWhere $add
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string|\string[] $value
-     * @param string $op
+     * Handle case of a revision column, where you need to convert from a Unix 
+     * timestamp.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @return string The SQL expression to be used on one side of the comparison operator
+     * @param string &$op the logical operator this filter shoudl use
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
-    {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op) {
         $col = "$tablealias.$colname";
         $QB = $add->getQB();
 
@@ -136,17 +139,9 @@ public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $va
             $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
         }
 
-        /** @var QueryBuilderWhere $add Where additional queries are added to */
-        if (is_array($value)) {
-            $add = $add->where($op); // sub where group
-            $op = 'OR';
-        }
-        foreach ((array)$value as $item) {
-            $pl = $QB->addValue($item);
-            $add->where($op, "$col $comp $pl");
-        }
+        return $col;
     }
-
+ 
     /**
      * When sorting `%lastupdated%`, then sort the data from the `titles` table instead the `data_` table.
      *
diff --git a/types/Decimal.php b/types/Decimal.php
index 6aa68f58..56b92108 100644
--- a/types/Decimal.php
+++ b/types/Decimal.php
@@ -155,30 +155,27 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
     /**
      * Decimals need to be casted to proper type for comparison
      *
-     * @param QueryBuilderWhere $add
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string|\string[] $value
-     * @param string $op
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return string The SQL expression to be used on one side of the comparison operator
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
     {
         $add = $add->where($op); // open a subgroup
         $add->where('AND', "$tablealias.$colname != ''");
          // make sure the field isn't empty
         $op = 'AND';
+        return "CAST($tablealias.$colname AS DECIMAL)";
+    }
 
-        /** @var QueryBuilderWhere $add Where additionional queries are added to */
-        if (is_array($value)) {
-            $add = $add->where($op); // sub where group
-            $op = 'OR';
-        }
-
-        foreach ((array)$value as $item) {
-            $pl = $add->getQB()->addValue($item);
-            $add->where($op, "CAST($tablealias.$colname AS DECIMAL) $comp CAST($pl AS DECIMAL)");
-        }
+    /**
+     * @param string $value The value a column is being compared to
+     * @return string SQL expression casting $value to a decimal
+     */
+    protected function getSqlConstantValue($value) {
+        return "CAST($value AS DECIMAL)";
     }
 
     /**
diff --git a/types/Lookup.php b/types/Lookup.php
index d9742755..236d8fce 100644
--- a/types/Lookup.php
+++ b/types/Lookup.php
@@ -256,20 +256,18 @@ public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
     /**
      * Compare against lookup table
      *
-     * @param QueryBuilderWhere $add
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string|\string[] $value
-     * @param string $op
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
-    {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op) {
         $schema = 'data_' . $this->config['schema'];
         $column = $this->getLookupColumn();
         if (!$column) {
-            parent::filter($add, $tablealias, $colname, $comp, $value, $op);
-            return;
+            return parent::getSqlCompareValue($add, $tablealias, $colname, $op);
         }
         $field = $column->getColName();
 
@@ -283,7 +281,22 @@ public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $va
             "$tablealias.$colname = STRUCT_JSON($rightalias.pid, CAST($rightalias.rid AS DECIMAL)) AND " .
             "$rightalias.latest = 1"
         );
-        $column->getType()->filter($add, $rightalias, $field, $comp, $value, $op);
+        return $column->getType()->getSqlCompareValue($add, $rightalias, $field, $op);
+    }
+
+    /**
+     * Handle the value that a column is being compared against.
+     *
+     * @param string $value The value a column is being compared to
+     * @return string A SQL expression processing the value in some way.
+     */
+    protected function getSqlConstantValue($value) {
+        $schema = 'data_' . $this->config['schema'];
+        $column = $this->getLookupColumn();
+        if (!$column) {
+            return parent::getSqlConstantValue($value);
+        }
+        return $column->getType()->getSqlConstantValue($value);
     }
 
     /**
diff --git a/types/Page.php b/types/Page.php
index 87814367..8b049d29 100644
--- a/types/Page.php
+++ b/types/Page.php
@@ -185,32 +185,25 @@ public function displayValue($value)
     }
 
     /**
-     * When using titles, we need to compare against the title table, too
+     * When using titles, we need to compare against the title table, too.
      *
-     * @param QueryBuilderWhere $add
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string $value
-     * @param string $op
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return array The SQL expression to be used on one side of the comparison operator
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op)
     {
         if (!$this->config['usetitles']) {
-            parent::filter($add, $tablealias, $colname, $comp, $value, $op);
-            return;
+            return parent::getSqlCompareValue($add, $tablealias, $colname, $op);
         }
 
         $QB = $add->getQB();
         $rightalias = $QB->generateTableAlias();
         $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.$colname = $rightalias.pid");
-
-        // compare against page and title
-        $sub = $add->where($op);
-        $pl = $QB->addValue($value);
-        $sub->whereOr("$tablealias.$colname $comp $pl");
-        $pl = $QB->addValue($value);
-        $sub->whereOr("$rightalias.title $comp $pl");
+        return ["$tablealias.$colname", "$rightalias.title"];
     }
 
     /**
@@ -266,4 +259,19 @@ protected function mergeConfig($current, &$config)
             }
         }
     }
+
+    // /**
+    //  * When using titles, we need to compare against the title table, too
+    //  *
+    //  * @param QueryBuilderWhere $add
+    //  * @param string $table
+    //  * @param string $colname
+    //  * @return string One side of the equality comparion being used for the JOIN
+    //  */
+    // protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
+    //     if (!$this->config['usetitles']) {
+    //         return parent::joinArgument($add, $table, $colname);
+    //     }
+    //     // FIXME: How to handle multiple values 
+    // }
 }
diff --git a/types/Tag.php b/types/Tag.php
index f9bfff8b..bd66b51b 100644
--- a/types/Tag.php
+++ b/types/Tag.php
@@ -115,23 +115,24 @@ protected function buildSQLFromContext(Column $context)
     /**
      * Normalize tags before comparing
      *
-     * @param QueryBuilder $QB
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string|string[] $value
-     * @param string $op
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return string The SQL expression to be used on one side of the comparison operator
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
-    {
-        /** @var QueryBuilderWhere $add Where additionional queries are added to */
-        if (is_array($value)) {
-            $add = $add->where($op); // sub where group
-            $op = 'OR';
-        }
-        foreach ((array)$value as $item) {
-            $pl = $add->getQB()->addValue($item);
-            $add->where($op, "LOWER(REPLACE($tablealias.$colname, ' ', '')) $comp LOWER(REPLACE($pl, ' ', ''))");
-        }
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op) {
+        return "LOWER(REPLACE($tablealias.$colname, ' ', ''))";
+    }
+
+    /**
+     * Normalize before comparing.
+     *
+     * @param string $value The value a column is being compared to
+     * @return string A SQL expression processing the value in some way.
+     */
+    protected function getSqlConstantValue($value) {
+        return "LOWER(REPLACE($value, ' ', ''))";
     }
 }
diff --git a/types/TraitFilterPrefix.php b/types/TraitFilterPrefix.php
index ddf8a16f..05ec6b86 100644
--- a/types/TraitFilterPrefix.php
+++ b/types/TraitFilterPrefix.php
@@ -17,40 +17,29 @@ trait TraitFilterPrefix
     /**
      * Comparisons are done against the full string (including prefix/postfix)
      *
-     * @param QueryBuilderWhere $add
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string|string[] $value
-     * @param string $op
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
-    {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op) {
+        $column = parent::getSqlCompareValue($add, $tablealias, $colname, $op);
+
         $add = $add->where($op); // open a subgroup
-        $add->where('AND', "$tablealias.$colname != ''");
-         // make sure the field isn't empty
+        $add->where('AND', "$column != ''"); // make sure the field isn't empty
         $op = 'AND';
 
-        /** @var QueryBuilderWhere $add Where additionional queries are added to */
-        if (is_array($value)) {
-            $add = $add->where($op); // sub where group
-            $op = 'OR';
-        }
         $QB = $add->getQB();
-        foreach ((array)$value as $item) {
-            $column = "$tablealias.$colname";
-
-            if ($this->config['prefix']) {
-                $pl = $QB->addValue($this->config['prefix']);
-                $column = "$pl || $column";
-            }
-            if ($this->config['postfix']) {
-                $pl = $QB->addValue($this->config['postfix']);
-                $column = "$column || $pl";
-            }
-
-            $pl = $QB->addValue($item);
-            $add->where($op, "$column $comp $pl");
+        if ($this->config['prefix']) {
+            $pl = $QB->addValue($this->config['prefix']);
+            $column = "$pl || $column";
+        }
+        if ($this->config['postfix']) {
+            $pl = $QB->addValue($this->config['postfix']);
+            $column = "$column || $pl";
         }
+        return $column;
     }
 }
diff --git a/types/User.php b/types/User.php
index 0c2f9b39..d4e2edf0 100644
--- a/types/User.php
+++ b/types/User.php
@@ -139,29 +139,22 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
     }
 
     /**
-     * When using `%lasteditor%`, we need to compare against the `title` table.
-     *
-     * @param QueryBuilderWhere $add
-     * @param string $tablealias
-     * @param string $colname
-     * @param string $comp
-     * @param string|string[] $value
-     * @param string $op
+     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @param string &$op the logical operator this filter shoudl use
+     * @return string The SQL expression to be used on one side of the comparison operator
      */
-    public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
-    {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
+                                          $colname, &$op) {
         if (is_a($this->context, 'dokuwiki\plugin\struct\meta\UserColumn')) {
             $QB = $add->getQB();
             $rightalias = $QB->generateTableAlias();
             $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
-
-            // compare against page and title
-            $sub = $add->where($op);
-            $pl = $QB->addValue($value);
-            $sub->whereOr("$rightalias.lasteditor $comp $pl");
-            return;
+            return "$rightalias.lasteditor";
         }
 
-        parent::filter($add, $tablealias, $colname, $comp, $value, $op);
+        return parent::getSqlCompareValue($add, $tablealias, $colname, $comp, $value, $op);
     }
+
 }

From 3605876f259a42e9c3548a2e25c9ddb07449825f Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sat, 18 Nov 2023 23:24:14 +0000
Subject: [PATCH 11/27] Refactor how join conditions are handled

---
 meta/SearchSQLBuilder.php  | 10 ++++------
 types/AbstractBaseType.php | 18 +++++++++++-------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index c2320d01..2139c626 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -46,13 +46,11 @@ public function addSchemas($schemas)
                     // Custom join on some other columns
                     $lefttable = 'data_' . $lcol->getTable();
                     $righttable = 'data_' . $rcol->getTable();
-                    $add = new QueryBuilderWhere($QB);
-                    // TODO: Only tricky columns to join over are pages, as they are the only ones that add some OR clauses. For rest, think I can modify filter to handle case of $value being another column.
-                    $lcol->getType()->joinCondition(
-                        $add, $lefttable, $lcol->getColName(),
-                        $righttable, $rcol->getColName(), $rcol->getType()
+                    $on = $lcol->getType()->joinCondition(
+                        $lefttable, $lcol->getColName(), $righttable,
+                        $rcol->getColName(), $rcol->getType()
                     );
-                    $QB->addLeftJoin($lefttable, $righttable, $righttable, $add->toSQL());
+                    $QB->addLeftJoin($lefttable, $righttable, $righttable, $on);
                 }
             } else {
                 // first table
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 166c0aa3..45ff7f5a 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -457,25 +457,29 @@ protected function getSqlConstantValue($value) {
     }
 
     /**
-     * Place a condition expression in $add which $left_table and
-     * $right_table can be JOINed ON.  Semantically, this provides an
+     * Returns a SQL expression ON which JOIN $left_table and
+     * $right_table.  Semantically, this provides an
      * equality comparison between two columns in the two
      * schemas. However, in practice it may require more complex
      * logic, including additional JOINs to pull in other data or
      * handle multi-valued columns.
      *
-     * @param QueryBuilderWhere $add The condition ON which to JOIN the tables
      * @param string $left_table The name of the left table being JOINed
      * @param string $left_colname The name of the column in the left table being compared against for the JOIN
      * @param string $right_table The name of the right table being JOINed
      * @param string $right_colname The name of hte column in the right table being compared against for hte JOIN
      * @param AbstractBaseType $right_coltype The type of $right_colname
+     * @return string SQL expression on which to join schemas
      */
-    public function joinCondition(QueryBuilderWhere $add, $left_table, $left_colname, $right_table, $right_colname, $right_coltype)
+    public function joinCondition($left_table, $left_colname, $right_table, $right_colname, $right_coltype)
     {
-        $lhs = $this->joinArgument($add, $left_table, $left_colname);
-        $rhs = $right_coltype->joinArgument($add, $right_table, $right_colname);
-        $add->where('AND', "$lhs = $rhs");
+        $add = new QueryBuilderWhere();
+        $op = 'AND';
+        $lhs = $this->getSqlCompareValue($add, $left_table, $left_colname, $op);
+        $rhs = $this->getSqlConstantValue($right_coltype->getSqlCompareValue($add, $right_table, $right_colname, $op));
+        // FIXME: Need to handle possibility of getSqlCompareValue returning multiple values (i.e., due to joining on page name)
+        // FIXME: Need to consider how/whether to handle multi-valued columns
+        $add->where($op, "$lhs = $rhs");
         $AN = $add->getQB()->generateTableAlias('A');
         $subquery = "(SELECT assigned
                      FROM schema_assignments AS $AN

From d452b994b689ab7cd071f0cf518f24ba91ebdfd5 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sun, 19 Nov 2023 09:42:18 +0000
Subject: [PATCH 12/27] Fixed regression from rebasing

---
 meta/SearchSQLBuilder.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index 2139c626..6a1932db 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -99,7 +99,7 @@ public function addColumns($columns)
         $n = 0;
         foreach ($columns as $col) {
             $col->getType()->select(
-                $QB, 'data_' . $col->getTable(), 'multi_' . $col->getTable(),
+                $this->qb, 'data_' . $col->getTable(), 'multi_' . $col->getTable(),
                 'C' . $n++, true, $sep
             );
         }

From 3cbc3ab888a07ac3b91762e7692e51750c2d34b5 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sun, 19 Nov 2023 10:06:12 +0000
Subject: [PATCH 13/27] Further regression-fixes from rebasing

---
 meta/Search.php           | 2 +-
 meta/SearchSQLBuilder.php | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/meta/Search.php b/meta/Search.php
index db9139e1..9cbb9f8a 100755
--- a/meta/Search.php
+++ b/meta/Search.php
@@ -581,7 +581,7 @@ protected function runSQLBuilder()
     {
         $sqlBuilder = new SearchSQLBuilder();
         $sqlBuilder->setSelectLatest($this->selectLatest);
-        $sqlBuilder->addSchemas($this->schemas);
+        $sqlBuilder->addSchemas($this->schemas, $this->joins);
         $sqlBuilder->addColumns($this->columns);
         $sqlBuilder->addFilters($this->filter);
         $sqlBuilder->addFilters($this->dynamicFilter);
diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index 6a1932db..39b71165 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -27,8 +27,9 @@ public function __construct()
      * Add the schemas to the query
      *
      * @param Schema[] $schemas Schema names to query
+     * @param array $joins Conditionals to be used when joining tables
      */
-    public function addSchemas($schemas)
+    public function addSchemas($schemas, $joins)
     {
         // basic tables
         $first_table = '';
@@ -38,10 +39,10 @@ public function addSchemas($schemas)
             $new_pid = false;
             if ($first_table) {
                 // follow up tables
-                [$lcol, $rcol] = $this->joins[$schema->getTable()];
+                [$lcol, $rcol] = $joins[$schema->getTable()];
                 if ($lcol->getLabel() == '%pageid%' and $rcol->getLabel() == '%pageid%') {
                     // Simple (default) case where we join on page IDs
-                    $QB->addLeftJoin($first_table, $datatable, $datatable, "$first_table.pid = $datatable.pid");
+                    $this->qb->addLeftJoin($first_table, $datatable, $datatable, "$first_table.pid = $datatable.pid");
                 } else {
                     // Custom join on some other columns
                     $lefttable = 'data_' . $lcol->getTable();
@@ -50,7 +51,7 @@ public function addSchemas($schemas)
                         $lefttable, $lcol->getColName(), $righttable,
                         $rcol->getColName(), $rcol->getType()
                     );
-                    $QB->addLeftJoin($lefttable, $righttable, $righttable, $on);
+                    $this->qb->addLeftJoin($lefttable, $righttable, $righttable, $on);
                 }
             } else {
                 // first table

From 096529f077823a8c5085c54fc252078b6b20c6e4 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sun, 19 Nov 2023 20:47:56 +0000
Subject: [PATCH 14/27] Fixed bug building searches with custom joins

---
 meta/SearchSQLBuilder.php  | 2 +-
 types/AbstractBaseType.php | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index 39b71165..ef4339f6 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -48,7 +48,7 @@ public function addSchemas($schemas, $joins)
                     $lefttable = 'data_' . $lcol->getTable();
                     $righttable = 'data_' . $rcol->getTable();
                     $on = $lcol->getType()->joinCondition(
-                        $lefttable, $lcol->getColName(), $righttable,
+                        $this->qb, $lefttable, $lcol->getColName(), $righttable,
                         $rcol->getColName(), $rcol->getType()
                     );
                     $this->qb->addLeftJoin($lefttable, $righttable, $righttable, $on);
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 45ff7f5a..5e501002 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -464,6 +464,7 @@ protected function getSqlConstantValue($value) {
      * logic, including additional JOINs to pull in other data or
      * handle multi-valued columns.
      *
+     * @param QueryBuilder $QB
      * @param string $left_table The name of the left table being JOINed
      * @param string $left_colname The name of the column in the left table being compared against for the JOIN
      * @param string $right_table The name of the right table being JOINed
@@ -471,15 +472,14 @@ protected function getSqlConstantValue($value) {
      * @param AbstractBaseType $right_coltype The type of $right_colname
      * @return string SQL expression on which to join schemas
      */
-    public function joinCondition($left_table, $left_colname, $right_table, $right_colname, $right_coltype)
+    public function joinCondition($QB, $left_table, $left_colname, $right_table, $right_colname, $right_coltype)
     {
-        $add = new QueryBuilderWhere();
+        $add = new QueryBuilderWhere($QB);
         $op = 'AND';
         $lhs = $this->getSqlCompareValue($add, $left_table, $left_colname, $op);
         $rhs = $this->getSqlConstantValue($right_coltype->getSqlCompareValue($add, $right_table, $right_colname, $op));
         // FIXME: Need to handle possibility of getSqlCompareValue returning multiple values (i.e., due to joining on page name)
         // FIXME: Need to consider how/whether to handle multi-valued columns
-        $add->where($op, "$lhs = $rhs");
         $AN = $add->getQB()->generateTableAlias('A');
         $subquery = "(SELECT assigned
                      FROM schema_assignments AS $AN
@@ -492,6 +492,7 @@ public function joinCondition($left_table, $left_colname, $right_table, $right_c
         $subOr->whereAnd("GETACCESSLEVEL($left_table.pid) > 0");
         $subOr->whereAnd("PAGEEXISTS($left_table.pid) = 1");
         $subOr->whereAnd("($subquery = 1 OR $subquery IS NULL)");
+        return "$lhs = $rhs";
     }
 
     /**

From 4ebe1fc6d8df7993832d6a1798eb1945c951958a Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sun, 19 Nov 2023 21:27:36 +0000
Subject: [PATCH 15/27] Updated test in line with refactoring

After my refactoring, prefixes and postfixes are only registered as
values once when comparing columns against multiple RHS values. This
will work just fine but means that things differ slightly from
existing tests.
---
 _test/types/TextTest.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/_test/types/TextTest.php b/_test/types/TextTest.php
index 9b482adf..fe64aca3 100644
--- a/_test/types/TextTest.php
+++ b/_test/types/TextTest.php
@@ -125,7 +125,7 @@ public function data()
                 'NOT LIKE', // comp
                 ['%val1%', '%val2%'], // multiple values
                 '((T.col != \'\' AND (? || T.col || ? NOT LIKE ? OR ? || T.col || ? NOT LIKE ?)))', // expect sql
-                ['before', 'after', '%val1%', 'before', 'after', '%val2%',], // expect opts
+                ['before', 'after', '%val1%', '%val2%',], // expect opts
             ],
         ];
 

From e1da59c65b9d8639709dcef9d26377428b5999c9 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Sun, 19 Nov 2023 22:00:47 +0000
Subject: [PATCH 16/27] Fixed code-sniffer issues

---
 meta/AccessTable.php        |  7 ++++++-
 meta/Search.php             |  9 +++++----
 meta/SearchCloud.php        |  6 ++----
 meta/SearchSQLBuilder.php   | 16 ++++++++++++----
 types/AbstractBaseType.php  | 30 ++++++++++++++++++------------
 types/AutoSummary.php       | 10 +++++-----
 types/DateTime.php          | 13 +++++++------
 types/Decimal.php           |  7 ++++---
 types/Lookup.php            | 11 ++++++-----
 types/Page.php              |  9 ++++-----
 types/Tag.php               | 11 ++++++-----
 types/TraitFilterPrefix.php |  8 ++++----
 types/User.php              |  9 ++++-----
 13 files changed, 83 insertions(+), 63 deletions(-)

diff --git a/meta/AccessTable.php b/meta/AccessTable.php
index f7144145..346128c5 100644
--- a/meta/AccessTable.php
+++ b/meta/AccessTable.php
@@ -506,7 +506,12 @@ protected function buildGetDataSQL($idColumn = 'pid')
 
         foreach ($this->schema->getColumns(false) as $col) {
             $col->getType()->select(
-                $QB, 'DATA', $mtable, 'out' . $col->getColref(), false, $sep
+                $QB,
+                'DATA',
+                $mtable,
+                'out' . $col->getColref(),
+                false,
+                $sep
             );
         }
 
diff --git a/meta/Search.php b/meta/Search.php
index 9cbb9f8a..ce89263d 100755
--- a/meta/Search.php
+++ b/meta/Search.php
@@ -115,7 +115,7 @@ public function addSchema($table, $alias = '', $joinon = array())
                 );
             }
             if ($joinon[1] != '=') {
-                throw new StructException('Only equality comparison is supported for JOIN conditions'); 
+                throw new StructException('Only equality comparison is supported for JOIN conditions');
             }
             $this->joins[$schema->getTable()] = $this->getJoinColumns($schema, $joinon[0], $joinon[2]);
         }
@@ -125,13 +125,14 @@ public function addSchema($table, $alias = '', $joinon = array())
      * Returns the columns being matched against for a JOIN ... ON
      * expression. The result will be ordered such that the first
      * column is the one from a previously-joined schema.
-     * 
+     *
      * @param Schema $schema The schema being JOINed to the query
      * @param string $left The LHS of the JOIN ON comparison
      * @param string $right the RHS of the JOIN ON comparison
      * @return array The first element is the LHS column object and second is the RHS
      */
-    protected function getJoinColumns($schema, $left, $right) {
+    protected function getJoinColumns($schema, $left, $right)
+    {
         $lcol = $this->findColumn($left);
         if ($lcol === false) {
             throw new StructException('Unrecognoside field ' . $left);
@@ -143,7 +144,7 @@ protected function getJoinColumns($schema, $left, $right) {
         $table = $schema->getTable();
         $left_is_old_table = $lcol->getTable() != $table;
         if ($left_is_old_table == ($rcol->getTable() != $table)) {
-            throw new StructException("Exactly one side of ON condition $left = $right must be a column of $table" );
+            throw new StructException("Exactly one side of ON condition $left = $right must be a column of $table");
         }
         if ($left_is_old_table) {
             return array($lcol, $rcol);
diff --git a/meta/SearchCloud.php b/meta/SearchCloud.php
index 0feb9f3c..85e85c25 100644
--- a/meta/SearchCloud.php
+++ b/meta/SearchCloud.php
@@ -56,10 +56,8 @@ public function getSQL()
         $QB->filters()->where('AND', 'tag IS NOT \'\'');
 
         $col = $this->columns[0];
-        $col->getType()->select(
-            $QB, 'data_' . $datatable, 'multi_' . $col->getTable(), 'tag', true
-        );
-        
+        $col->getType()->select($QB, 'data_' . $datatable, 'multi_' . $col->getTable(), 'tag', true);
+
         $QB->addSelectStatement('COUNT(tag)', 'count');
         $QB->addSelectColumn('schema_assignments', 'assigned', 'ASSIGNED');
         if ($col->isMulti()) {
diff --git a/meta/SearchSQLBuilder.php b/meta/SearchSQLBuilder.php
index ef4339f6..876ea780 100644
--- a/meta/SearchSQLBuilder.php
+++ b/meta/SearchSQLBuilder.php
@@ -48,8 +48,12 @@ public function addSchemas($schemas, $joins)
                     $lefttable = 'data_' . $lcol->getTable();
                     $righttable = 'data_' . $rcol->getTable();
                     $on = $lcol->getType()->joinCondition(
-                        $this->qb, $lefttable, $lcol->getColName(), $righttable,
-                        $rcol->getColName(), $rcol->getType()
+                        $this->qb,
+                        $lefttable,
+                        $lcol->getColName(),
+                        $righttable,
+                        $rcol->getColName(),
+                        $rcol->getType()
                     );
                     $this->qb->addLeftJoin($lefttable, $righttable, $righttable, $on);
                 }
@@ -100,8 +104,12 @@ public function addColumns($columns)
         $n = 0;
         foreach ($columns as $col) {
             $col->getType()->select(
-                $this->qb, 'data_' . $col->getTable(), 'multi_' . $col->getTable(),
-                'C' . $n++, true, $sep
+                $this->qb,
+                'data_' . $col->getTable(),
+                'multi_' . $col->getTable(),
+                'C' . $n++,
+                true,
+                $sep
             );
         }
     }
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 5e501002..692b73de 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -371,7 +371,8 @@ public function renderTagCloudLink($value, \Doku_Renderer $R, $mode, $page, $fil
      * @param bool $test_rid Whether to require RIDs to be equal in the JOIN condition
      * @return string Alias for the multi-table
      */
-    public function joinMulti(QueryBuilder $QB, $datatable, $multitable, $colref, $test_rid = true) {
+    public function joinMulti(QueryBuilder $QB, $datatable, $multitable, $colref, $test_rid = true)
+    {
         $MN = $QB->generateTableAlias('M');
         $condition = "$datatable.pid = $MN.pid ";
         if ($test_rid) $condition .= "AND $datatable.rid = $MN.rid ";
@@ -384,7 +385,7 @@ public function joinMulti(QueryBuilder $QB, $datatable, $multitable, $colref, $t
         );
         return $MN;
     }
-    
+
     /**
      * This function is used to modify an aggregation query to add a filter
      * for the given column matching the given value. A type should add at
@@ -432,14 +433,14 @@ public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $va
      * need to add additional logic to the conditional expression or make
      * additional JOINs.
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op) {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    {
         return "$tablealias.$colname";
     }
 
@@ -452,7 +453,8 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
      * @param string $value The value a column is being compared to
      * @return string A SQL expression processing the value in some way.
      */
-    protected function getSqlConstantValue($value) {
+    protected function getSqlConstantValue($value)
+    {
         return $value;
     }
 
@@ -477,8 +479,11 @@ public function joinCondition($QB, $left_table, $left_colname, $right_table, $ri
         $add = new QueryBuilderWhere($QB);
         $op = 'AND';
         $lhs = $this->getSqlCompareValue($add, $left_table, $left_colname, $op);
-        $rhs = $this->getSqlConstantValue($right_coltype->getSqlCompareValue($add, $right_table, $right_colname, $op));
-        // FIXME: Need to handle possibility of getSqlCompareValue returning multiple values (i.e., due to joining on page name)
+        $rhs = $this->getSqlConstantValue(
+            $right_coltype->getSqlCompareValue($add, $right_table, $right_colname, $op)
+        );
+        // FIXME: Need to handle possibility of getSqlCompareValue returning multiple
+        //        values (i.e., due to joining on page name)
         // FIXME: Need to consider how/whether to handle multi-valued columns
         $AN = $add->getQB()->generateTableAlias('A');
         $subquery = "(SELECT assigned
@@ -506,10 +511,11 @@ public function joinCondition($QB, $left_table, $left_colname, $right_table, $ri
      * @param string $colname Name of the column being JOINed ON
      * @return string One side of the equality comparion being used for the JOIN
      */
-    protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
+    protected function joinArgument(QueryBuilderWhere $add, $table, $colname)
+    {
         return "$table.$colname";
     }
-    
+
     /**
      * Add the proper selection for this type to the current Query. Handles the
      * possibility of multi-valued columns.
@@ -519,7 +525,7 @@ protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
      * @param string $multitable The name of the table the values are stored in if the column is multi-valued
      * @param string $alias The added selection *has* to use this column alias
      * @param bool $test_rid Whether to require RIDs to be equal if JOINing multi-table
-     * @param string|null $concat_sep Seperator to concatenate mutli-values together. If null, don't perform concatentation.
+     * @param string|null $concat_sep Seperator to concatenate mutli-values together. Don't concatenate if null.
      */
     public function select(QueryBuilder $QB, $singletable, $multitable, $alias, $test_rid = true, $concat_sep = null)
     {
diff --git a/types/AutoSummary.php b/types/AutoSummary.php
index 72185399..c5d1557a 100644
--- a/types/AutoSummary.php
+++ b/types/AutoSummary.php
@@ -41,18 +41,18 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
     /**
      * When using `%lastsummary%`, we need to compare against the `title` table.
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op) {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    {
         $QB = $add->getQB();
         $rightalias = $QB->generateTableAlias();
         $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
 
         return "$rightalias.lastsummary";
-    }    
+    }
 }
diff --git a/types/DateTime.php b/types/DateTime.php
index 9c26fab7..37b63ca8 100644
--- a/types/DateTime.php
+++ b/types/DateTime.php
@@ -118,17 +118,18 @@ public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
 
 
     /**
-     * Handle case of a revision column, where you need to convert from a Unix 
+     * Handle case of a revision column, where you need to convert from a Unix
      * timestamp.
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
      * @return string The SQL expression to be used on one side of the comparison operator
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
+     * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op) {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    {
         $col = "$tablealias.$colname";
         $QB = $add->getQB();
 
@@ -141,7 +142,7 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
 
         return $col;
     }
- 
+
     /**
      * When sorting `%lastupdated%`, then sort the data from the `titles` table instead the `data_` table.
      *
diff --git a/types/Decimal.php b/types/Decimal.php
index 56b92108..61f5ce6c 100644
--- a/types/Decimal.php
+++ b/types/Decimal.php
@@ -155,10 +155,10 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
     /**
      * Decimals need to be casted to proper type for comparison
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional  this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
     protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
@@ -174,7 +174,8 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $col
      * @param string $value The value a column is being compared to
      * @return string SQL expression casting $value to a decimal
      */
-    protected function getSqlConstantValue($value) {
+    protected function getSqlConstantValue($value)
+    {
         return "CAST($value AS DECIMAL)";
     }
 
diff --git a/types/Lookup.php b/types/Lookup.php
index 236d8fce..a57a38f7 100644
--- a/types/Lookup.php
+++ b/types/Lookup.php
@@ -256,14 +256,14 @@ public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
     /**
      * Compare against lookup table
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op) {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    {
         $schema = 'data_' . $this->config['schema'];
         $column = $this->getLookupColumn();
         if (!$column) {
@@ -290,7 +290,8 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
      * @param string $value The value a column is being compared to
      * @return string A SQL expression processing the value in some way.
      */
-    protected function getSqlConstantValue($value) {
+    protected function getSqlConstantValue($value)
+    {
         $schema = 'data_' . $this->config['schema'];
         $column = $this->getLookupColumn();
         if (!$column) {
diff --git a/types/Page.php b/types/Page.php
index 8b049d29..0c2b5bde 100644
--- a/types/Page.php
+++ b/types/Page.php
@@ -187,14 +187,13 @@ public function displayValue($value)
     /**
      * When using titles, we need to compare against the title table, too.
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
     {
         if (!$this->config['usetitles']) {
             return parent::getSqlCompareValue($add, $tablealias, $colname, $op);
@@ -272,6 +271,6 @@ protected function mergeConfig($current, &$config)
     //     if (!$this->config['usetitles']) {
     //         return parent::joinArgument($add, $table, $colname);
     //     }
-    //     // FIXME: How to handle multiple values 
+    //     // FIXME: How to handle multiple values
     // }
 }
diff --git a/types/Tag.php b/types/Tag.php
index bd66b51b..4caecd13 100644
--- a/types/Tag.php
+++ b/types/Tag.php
@@ -115,14 +115,14 @@ protected function buildSQLFromContext(Column $context)
     /**
      * Normalize tags before comparing
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op) {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    {
         return "LOWER(REPLACE($tablealias.$colname, ' ', ''))";
     }
 
@@ -132,7 +132,8 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
      * @param string $value The value a column is being compared to
      * @return string A SQL expression processing the value in some way.
      */
-    protected function getSqlConstantValue($value) {
+    protected function getSqlConstantValue($value)
+    {
         return "LOWER(REPLACE($value, ' ', ''))";
     }
 }
diff --git a/types/TraitFilterPrefix.php b/types/TraitFilterPrefix.php
index 05ec6b86..fb32a71e 100644
--- a/types/TraitFilterPrefix.php
+++ b/types/TraitFilterPrefix.php
@@ -17,14 +17,14 @@ trait TraitFilterPrefix
     /**
      * Comparisons are done against the full string (including prefix/postfix)
      *
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op) {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    {
         $column = parent::getSqlCompareValue($add, $tablealias, $colname, $op);
 
         $add = $add->where($op); // open a subgroup
diff --git a/types/User.php b/types/User.php
index d4e2edf0..ccc22919 100644
--- a/types/User.php
+++ b/types/User.php
@@ -139,14 +139,14 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
     }
 
     /**
-     * @param QueryBuilderWhere &$add The WHERE or ON clause which will contain the conditional expression this comparator will be used in
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @param string &$op the logical operator this filter shoudl use
+     * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
-                                          $colname, &$op) {
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    {
         if (is_a($this->context, 'dokuwiki\plugin\struct\meta\UserColumn')) {
             $QB = $add->getQB();
             $rightalias = $QB->generateTableAlias();
@@ -156,5 +156,4 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias,
 
         return parent::getSqlCompareValue($add, $tablealias, $colname, $comp, $value, $op);
     }
-
 }

From 8d0bdc37bece0e97cf6dc0a15ea6f9a527fb6ce1 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Wed, 29 Nov 2023 01:13:21 +0000
Subject: [PATCH 17/27] Separated additional joins from getSqlCompareValue

This allows the order of the additional join to be varied depending on
whether filtering or joining primary data.

Also renamed getSqlConstantValue to wrapValue, which is more
descriptive.
---
 _test/SearchTest.php        | 77 ++++++++++++++++++++++++++++++++++
 types/AbstractBaseType.php  | 83 ++++++++++++++++++++++++++++---------
 types/AutoSummary.php       | 23 ++++++++--
 types/DateTime.php          | 28 ++++++++++---
 types/Decimal.php           |  5 ++-
 types/Lookup.php            | 34 ++++++++++-----
 types/Page.php              | 43 ++++++++++---------
 types/Tag.php               |  5 ++-
 types/TraitFilterPrefix.php |  5 ++-
 types/User.php              | 28 ++++++++++---
 10 files changed, 261 insertions(+), 70 deletions(-)

diff --git a/_test/SearchTest.php b/_test/SearchTest.php
index e6396105..23df429e 100644
--- a/_test/SearchTest.php
+++ b/_test/SearchTest.php
@@ -20,12 +20,14 @@ public function setUp(): void
 
         $this->loadSchemaJSON('schema1');
         $this->loadSchemaJSON('schema2');
+        $this->loadSchemaJSON('pageschema');
         $_SERVER['REMOTE_USER'] = 'testuser';
 
         $as = mock\Assignments::getInstance();
         $page = 'page01';
         $as->assignPageSchema($page, 'schema1');
         $as->assignPageSchema($page, 'schema2');
+        $as->assignPageSchema($page, 'pageschema');
         saveWikiText($page, "===== TestTitle =====\nabc", "Summary");
         p_get_metadata($page);
         $now = time();
@@ -51,9 +53,21 @@ public function setUp(): void
             ],
             $now
         );
+        $this->saveData(
+            $page,
+            'pageschema',
+            [
+                'singlepage' => 'page12',
+                'multipage' => ['test:document', $page, 'page16'],
+                'singletitle' => 'page10',
+                'multititle' => ['page12', 'page10'],
+            ],
+            $now,
+        );
 
         $as->assignPageSchema('test:document', 'schema1');
         $as->assignPageSchema('test:document', 'schema2');
+        $as->assignPageSchema('test:document', 'pageschema');
         $this->saveData(
             'test:document',
             'schema1',
@@ -76,7 +90,31 @@ public function setUp(): void
             ],
             $now
         );
+        $this->saveData(
+            'test:document',
+            'pageschema',
+            [
+                'singlepage' => $page,
+                'multipage' => ['test:document', $page],
+                'singletitle' => 'page10',
+                'multititle' => ['page11', 'page16'],
+            ],
+            $now,
+        );
 
+        $as->assignPageSchema('test:document2', 'schema2');
+        $this->saveData(
+            'test:document2',
+            'schema2',
+            [
+                'afirst' => 'TestTitle',
+                'asecond' => ['test:document', 'fourth data'],
+                'athird' => '',
+                'afourth' => ''
+            ],
+            $now
+        );
+        
         for ($i = 10; $i <= 20; $i++) {
             $this->saveData(
                 "page$i",
@@ -483,6 +521,45 @@ public function test_join()
         $this->assertEquals('first data', $result[1][1]->getValue());
     }
 
+    public function test_join_pageid()
+    {
+        $search = new mock\Search();
+
+        $search->addSchema('schema2', 'foo');
+        $search->addSchema('pageschema', '', array('pageschema.singlepage', '=', 'foo.%pageid%'));
+        $this->assertEquals(2, count($search->schemas));
+
+        $this->assertEquals(1, count($search->joins));
+        $joincols = $search->joins['pageschema'];
+        $this->assertEquals(2, count($joincols));
+        $this->assertEquals('schema2', $joincols[0]->getTable());
+        $this->assertEquals('%pageid%', $joincols[0]->getLabel());
+        $this->assertEquals('pageschema', $joincols[1]->getTable());
+        $this->assertEquals('singlepage', $joincols[1]->getLabel());
+ 
+        $search->addColumn('foo.%pageid%');
+        $search->addColumn('pageschema.%pageid%');
+        $search->addColumn('afourth');
+        $search->addColumn('singletitle');
+
+        $result = $search->execute();
+        $count = $search->getCount();
+
+        // check result dimensions
+        $this->assertEquals(2, $count, 'result count'); // full result set
+        $this->assertEquals(2, count($result), 'result rows'); // wanted result set
+
+        // check the values
+        $this->assertEquals('page01', $result[0][0]->getValue());
+        $this->assertEquals('test:document', $result[0][1]->getValue());
+        $this->assertEquals('fourth data', $result[0][2]->getValue());
+        $this->assertEquals('["page10",null]', $result[0][3]->getValue());
+        $this->assertEquals('page12', $result[1][0]->getValue());
+        $this->assertEquals('page01', $result[1][1]->getValue());
+        $this->assertEquals('page12 fourth data', $result[1][2]->getValue());
+        $this->assertEquals('["page10",null]', $result[1][3]->getValue());
+    }
+
 
     public function invalidJoins_testdata() {
         return array(
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 692b73de..62232c2c 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -404,7 +404,15 @@ public function joinMulti(QueryBuilder $QB, $datatable, $multitable, $colref, $t
      */
     public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $value, $op)
     {
-        $compareVal = $this->getSqlCompareValue($add, $tablealias, $colname, $op);
+        $additional_join = $this->getAdditionalJoinForComparison($add, $tablealias, $colname);
+        if (!is_null($additional_join)) {
+            $add->getQB()->addLeftJoin($additional_join[0], $additional_join[1], $additional_join[2], $additional_join[3]);
+            $oldalias = $tablealias;
+            $tablealias = $additional_join[2];
+        } else {
+            $oldalias = null;
+        }
+        $compareVal = $this->getSqlCompareValue($add, $tablealias, $oldalias, $colname, $op);
         /** @var QueryBuilderWhere $add Where additionional queries are added to */
         if (is_array($value)) {
             $add = $add->where($op); // sub where group
@@ -419,7 +427,7 @@ public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $va
             } else {
                 $sub = $add;
             }
-            $pl = $this->getSqlConstantValue($add->getQB()->addValue($item));
+            $pl = $this->wrapValue($add->getQB()->addValue($item));
             foreach ((array)$compareVal as $lhs) {
                 $sub->where($op, "$lhs $comp $pl");
             }
@@ -435,15 +443,31 @@ public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $va
      *
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
         return "$tablealias.$colname";
     }
 
+    /**
+     * This function provides arguments for an additional JOIN operation needed
+     * to perform a comparison (e.g., for a JOIN or FILTER), or null if no
+     * additional JOIN is needed.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @return null|array [$leftalias, $righttable, $rightalias, $onclause]
+     */
+    protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tablealias, $colname)
+    {
+        return null;
+    }
+
     /**
      * Handle the value that a column is being compared against. In
      * most cases this method will just return the value unchanged,
@@ -453,7 +477,7 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $col
      * @param string $value The value a column is being compared to
      * @return string A SQL expression processing the value in some way.
      */
-    protected function getSqlConstantValue($value)
+    protected function wrapValue($value)
     {
         return $value;
     }
@@ -474,30 +498,49 @@ protected function getSqlConstantValue($value)
      * @param AbstractBaseType $right_coltype The type of $right_colname
      * @return string SQL expression on which to join schemas
      */
-    public function joinCondition($QB, $left_table, $left_colname, $right_table, $right_colname, $right_coltype)
+    public function joinCondition($QB, &$left_table, $left_colname, $right_table, $right_colname, $right_coltype)
     {
         $add = new QueryBuilderWhere($QB);
         $op = 'AND';
-        $lhs = $this->getSqlCompareValue($add, $left_table, $left_colname, $op);
-        $rhs = $this->getSqlConstantValue(
-            $right_coltype->getSqlCompareValue($add, $right_table, $right_colname, $op)
-        );
-        // FIXME: Need to handle possibility of getSqlCompareValue returning multiple
-        //        values (i.e., due to joining on page name)
-        // FIXME: Need to consider how/whether to handle multi-valued columns
+        $additional_join = $this->getAdditionalJoinForComparison($add, $left_table, $left_colname);
+        // FIXME: How to work with multiple lhs or rhs values (i.e., due to Page types)?
+        // In rhs secondary join, compare lhs ID and (possibly) title against rhs title only. In returned join condition, compare against secondary PID and the column value against lhs ID/title. At most one side of comparison will have multiple values.
+        if (!is_null($additional_join)) {
+            $add->getQB()->addLeftJoin($additional_join[0], $additional_join[1], $additional_join[2], $additional_join[3]);
+            $lhs = $this->getSqlCompareValue($add, $additional_join[2], $left_table, $left_colname, $op);
+        } else {
+            $lhs = $this->getSqlCompareValue($add, $left_table, null, $left_colname, $op);
+        }
+        $additional_join = $right_coltype->getAdditionalJoinForComparison($add, $right_table, $right_colname);
+        // FIXME: Are calls to wrapValue redundant, given that I'm already translating the current value using $this->getSqlCompareValue? They can be, if both lhs and rhs columns are of the same type.
+        if (!is_null($additional_join)) {
+            $rhs = $this->wrapValue(
+                $right_coltype->getSqlCompareValue($add, $additional_join[2], $right_table, $right_colname, $op)
+            );
+            $add->getQB()->addLeftJoin($left_table, $additional_join[1], $additional_join[2], "$lhs = $rhs");
+            $left_table = $additional_join[2];
+            $result = $additional_join[3];
+        } else {
+            $rhs = $this->wrapValue(
+                $right_coltype->getSqlCompareValue($add, $right_table, null, $right_colname, $op)
+            );
+            $result = "$lhs = $rhs";
+        }
+        // FIXME: Can I do this somehow without needing to execute the subquery twice?
         $AN = $add->getQB()->generateTableAlias('A');
         $subquery = "(SELECT assigned
                      FROM schema_assignments AS $AN
-                     WHERE $left_table.pid != '' AND
-                           $left_table.pid = $AN.pid AND
-                           $AN.tbl = '{$this->getContext()->getTable()}')";
+                     WHERE $right_table.pid != '' AND
+                           $right_table.pid = $AN.pid AND
+                           $AN.tbl = '{$right_coltype->getContext()->getTable()}')";
         $subAnd = $add->whereSubAnd();
-        $subAnd->whereOr("$left_table.pid = ''");
+        $subAnd->whereOr("$right_table.pid = ''");
         $subOr = $subAnd->whereSubOr();
-        $subOr->whereAnd("GETACCESSLEVEL($left_table.pid) > 0");
-        $subOr->whereAnd("PAGEEXISTS($left_table.pid) = 1");
-        $subOr->whereAnd("($subquery = 1 OR $subquery IS NULL)");
-        return "$lhs = $rhs";
+        $subOr->whereAnd("GETACCESSLEVEL($right_table.pid) > 0");
+        $subOr->whereAnd("PAGEEXISTS($right_table.pid) = 1");
+        $subOr->whereAnd("($right_table.rid != 0 OR ($subquery = 1 OR $subquery IS NULL))");
+        // FIXME: Need to consider how/whether to handle multi-valued columns
+        return $result;
     }
 
     /**
diff --git a/types/AutoSummary.php b/types/AutoSummary.php
index c5d1557a..40c6ed6d 100644
--- a/types/AutoSummary.php
+++ b/types/AutoSummary.php
@@ -43,16 +43,31 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
      *
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
+    {
+        return "$table.lastsummary";
+    }
+
+    /**
+     * This function provides arguments for an additional JOIN operation needed
+     * to perform a comparison (e.g., for a JOIN or FILTER), or null if no
+     * additional JOIN is needed.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @return null|array [$leftalias, $righttable, $rightalias, $onclause]
+     */
+    protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tablealias, $colname)
     {
         $QB = $add->getQB();
         $rightalias = $QB->generateTableAlias();
-        $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
-
-        return "$rightalias.lastsummary";
+        return [$tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid"];
     }
+
 }
diff --git a/types/DateTime.php b/types/DateTime.php
index 37b63ca8..bd094690 100644
--- a/types/DateTime.php
+++ b/types/DateTime.php
@@ -124,25 +124,41 @@ public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
      * @param string $colname The column name on the above table
-     * @return string The SQL expression to be used on one side of the comparison operator
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string &$op the logical operator this filter should use
      * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
         $col = "$tablealias.$colname";
-        $QB = $add->getQB();
 
         // when accessing the revision column we need to convert from Unix timestamp
         if (is_a($this->context, 'dokuwiki\plugin\struct\meta\RevisionColumn')) {
-            $rightalias = $QB->generateTableAlias();
-            $col = "DATETIME($rightalias.lastrev, 'unixepoch', 'localtime')";
-            $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
+            $col = "DATETIME($tablealias.lastrev, 'unixepoch', 'localtime')";
         }
 
         return $col;
     }
 
+    /**
+     * This function provides arguments for an additional JOIN operation needed
+     * to perform a comparison (e.g., for a JOIN or FILTER), or null if no
+     * additional JOIN is needed.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @return null|array [$leftalias, $righttable, $rightalias, $onclause]
+     */
+    protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tablealias, $colname)
+    {
+        if (is_a($this->context, 'dokuwiki\plugin\struct\meta\RevisionColumn')) {
+            $rightalias = $QB->generateTableAlias();
+            return [$tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid"];
+        }
+        return parent::getAdditionalJoinForComparison($add, $tablealias, $colname);
+    }
+
     /**
      * When sorting `%lastupdated%`, then sort the data from the `titles` table instead the `data_` table.
      *
diff --git a/types/Decimal.php b/types/Decimal.php
index 61f5ce6c..53b02354 100644
--- a/types/Decimal.php
+++ b/types/Decimal.php
@@ -157,11 +157,12 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
      *
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional  this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
         $add = $add->where($op); // open a subgroup
         $add->where('AND', "$tablealias.$colname != ''");
@@ -174,7 +175,7 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $col
      * @param string $value The value a column is being compared to
      * @return string SQL expression casting $value to a decimal
      */
-    protected function getSqlConstantValue($value)
+    protected function wrapValue($value)
     {
         return "CAST($value AS DECIMAL)";
     }
diff --git a/types/Lookup.php b/types/Lookup.php
index a57a38f7..44772753 100644
--- a/types/Lookup.php
+++ b/types/Lookup.php
@@ -258,30 +258,44 @@ public function selectCol(QueryBuilder $QB, $tablealias, $colname, $alias)
      *
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
-        $schema = 'data_' . $this->config['schema'];
         $column = $this->getLookupColumn();
         if (!$column) {
-            return parent::getSqlCompareValue($add, $tablealias, $colname, $op);
+            return parent::getSqlCompareValue($add, $tablealias, $oldalias, $colname, $op);
         }
         $field = $column->getColName();
+        return $column->getType()->getSqlCompareValue($add, $tablealias, $oldalias, $field, $op);
+    }
 
-        // compare against lookup field
+    /**
+     * This function provides arguments for an additional JOIN operation needed
+     * to perform a comparison (e.g., for a JOIN or FILTER), or null if no
+     * additional JOIN is needed.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @return null|array [$leftalias, $righttable, $rightalias, $onclause]
+     */
+    protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tablealias, $colname)
+    {
+        if (!$this->getLookupColumn()) return null;
+        $schema = 'data_' . $this->config['schema'];
         $QB = $add->getQB();
         $rightalias = $QB->generateTableAlias();
-        $QB->addLeftJoin(
+        return [
             $tablealias,
             $schema,
             $rightalias,
             "$tablealias.$colname = STRUCT_JSON($rightalias.pid, CAST($rightalias.rid AS DECIMAL)) AND " .
             "$rightalias.latest = 1"
-        );
-        return $column->getType()->getSqlCompareValue($add, $rightalias, $field, $op);
+        ];
     }
 
     /**
@@ -290,14 +304,14 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $col
      * @param string $value The value a column is being compared to
      * @return string A SQL expression processing the value in some way.
      */
-    protected function getSqlConstantValue($value)
+    protected function wrapValue($value)
     {
         $schema = 'data_' . $this->config['schema'];
         $column = $this->getLookupColumn();
         if (!$column) {
-            return parent::getSqlConstantValue($value);
+            return parent::wrapValue($value);
         }
-        return $column->getType()->getSqlConstantValue($value);
+        return $column->getType()->wrapValue($value);
     }
 
     /**
diff --git a/types/Page.php b/types/Page.php
index 0c2b5bde..60702df8 100644
--- a/types/Page.php
+++ b/types/Page.php
@@ -189,20 +189,40 @@ public function displayValue($value)
      *
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
         if (!$this->config['usetitles']) {
-            return parent::getSqlCompareValue($add, $tablealias, $colname, $op);
+            return parent::getSqlCompareValue($add, $tablealias, $oldalias, $colname, $op);
         }
+        if (is_null($oldalias)) {
+            throw new StructException('Table name for Page column not specified.');
+        }
+        return ["$oldalias.$colname", "$tablealias.title"];
+    }
 
+    /**
+     * This function provides arguments for an additional JOIN operation needed
+     * to perform a comparison (e.g., for a JOIN or FILTER), or null if no
+     * additional JOIN is needed.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @return null|array [$leftalias, $righttable, $rightalias, $onclause]
+     */
+    protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tablealias, $colname)
+    {
+        if (!$this->config['usetitles']) {
+            return parent::getAdditionalJoinForComparison($add, $tablealias, $colname);
+        }
         $QB = $add->getQB();
         $rightalias = $QB->generateTableAlias();
-        $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.$colname = $rightalias.pid");
-        return ["$tablealias.$colname", "$rightalias.title"];
+        return [$tablealias, 'titles', $rightalias, "$tablealias.$colname = $rightalias.pid"];
     }
 
     /**
@@ -258,19 +278,4 @@ protected function mergeConfig($current, &$config)
             }
         }
     }
-
-    // /**
-    //  * When using titles, we need to compare against the title table, too
-    //  *
-    //  * @param QueryBuilderWhere $add
-    //  * @param string $table
-    //  * @param string $colname
-    //  * @return string One side of the equality comparion being used for the JOIN
-    //  */
-    // protected function joinArgument(QueryBuilderWhere $add, $table, $colname) {
-    //     if (!$this->config['usetitles']) {
-    //         return parent::joinArgument($add, $table, $colname);
-    //     }
-    //     // FIXME: How to handle multiple values
-    // }
 }
diff --git a/types/Tag.php b/types/Tag.php
index 4caecd13..087046d1 100644
--- a/types/Tag.php
+++ b/types/Tag.php
@@ -117,11 +117,12 @@ protected function buildSQLFromContext(Column $context)
      *
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
         return "LOWER(REPLACE($tablealias.$colname, ' ', ''))";
     }
@@ -132,7 +133,7 @@ protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $col
      * @param string $value The value a column is being compared to
      * @return string A SQL expression processing the value in some way.
      */
-    protected function getSqlConstantValue($value)
+    protected function wrapValue($value)
     {
         return "LOWER(REPLACE($value, ' ', ''))";
     }
diff --git a/types/TraitFilterPrefix.php b/types/TraitFilterPrefix.php
index fb32a71e..a2140645 100644
--- a/types/TraitFilterPrefix.php
+++ b/types/TraitFilterPrefix.php
@@ -19,13 +19,14 @@ trait TraitFilterPrefix
      *
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return string|array The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
-        $column = parent::getSqlCompareValue($add, $tablealias, $colname, $op);
+        $column = parent::getSqlCompareValue($add, $tablealias, $oldalias, $colname, $op);
 
         $add = $add->where($op); // open a subgroup
         $add->where('AND', "$column != ''"); // make sure the field isn't empty
diff --git a/types/User.php b/types/User.php
index ccc22919..0c1b7459 100644
--- a/types/User.php
+++ b/types/User.php
@@ -141,19 +141,37 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
     /**
      * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
      * @param string $tablealias The table the values are stored in
+     * @param string|null $oldalias A previous alias used for this table (only used by Page)
      * @param string $colname The column name on the above table
      * @param string &$op the logical operator this filter should use
      * @return string The SQL expression to be used on one side of the comparison operator
      */
-    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $colname, &$op)
+    protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
+    {
+        if (is_a($this->context, 'dokuwiki\plugin\struct\meta\UserColumn')) {
+            return "$tablealias.lasteditor";
+        }
+
+        return parent::getSqlCompareValue($add, $tablealias, $oldalias, $colname, $comp, $value, $op);
+    }
+
+    /**
+     * This function provides arguments for an additional JOIN operation needed
+     * to perform a comparison (e.g., for a JOIN or FILTER), or null if no
+     * additional JOIN is needed.
+     *
+     * @param QueryBuilderWhere &$add The WHERE or ON clause to contain the conditional this comparator will be used in
+     * @param string $tablealias The table the values are stored in
+     * @param string $colname The column name on the above table
+     * @return null|array [$leftalias, $righttable, $rightalias, $onclause]
+     */
+    protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tablealias, $colname)
     {
         if (is_a($this->context, 'dokuwiki\plugin\struct\meta\UserColumn')) {
             $QB = $add->getQB();
             $rightalias = $QB->generateTableAlias();
-            $QB->addLeftJoin($tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid");
-            return "$rightalias.lasteditor";
+            return [$tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid"];
         }
-
-        return parent::getSqlCompareValue($add, $tablealias, $colname, $comp, $value, $op);
+        return parent::getAdditionalJoinForComparison($add, $tablealias, $colname);
     }
 }

From 5d60c3b5b1e4e5f216823ab1916d237369640a42 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 01:14:02 +0000
Subject: [PATCH 18/27] Decided not to support joins over multi-valued columns

---
 _test/SearchTest.php       |  2 ++
 meta/Search.php            | 10 ++++++++++
 types/AbstractBaseType.php |  1 -
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/_test/SearchTest.php b/_test/SearchTest.php
index 23df429e..a95f3d0b 100644
--- a/_test/SearchTest.php
+++ b/_test/SearchTest.php
@@ -569,6 +569,8 @@ public function invalidJoins_testdata() {
             array('notaschema.first', '=', 'schema1.first'),
             array('notacolumn', '=', 'schema1.first'),
             array('foo.athird', '=', 'schema1.notacolumn'),
+            array('schema1.second', '=', 'schema2.afirst'),
+            array('schema1.first', '=', 'scheam2.asecond'),
         );
     }
     
diff --git a/meta/Search.php b/meta/Search.php
index ce89263d..fd509aa8 100755
--- a/meta/Search.php
+++ b/meta/Search.php
@@ -137,10 +137,20 @@ protected function getJoinColumns($schema, $left, $right)
         if ($lcol === false) {
             throw new StructException('Unrecognoside field ' . $left);
         }
+        if ($lcol->getType()->isMulti()) {
+            throw new StructException(
+                "Column $left is multi-valued, but JOINs are not supported on multi-valued columns"
+            );
+        }
         $rcol = $this->findColumn($right);
         if ($rcol === false) {
             throw new StructException('Unrecognoside field ' . $right);
         }
+        if ($rcol->getType()->isMulti()) {
+            throw new StructException(
+                "Column $right is multi-valued, but JOINs are not supported on multi-valued columns"
+            );
+        }
         $table = $schema->getTable();
         $left_is_old_table = $lcol->getTable() != $table;
         if ($left_is_old_table == ($rcol->getTable() != $table)) {
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 62232c2c..a850c3b4 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -539,7 +539,6 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
         $subOr->whereAnd("GETACCESSLEVEL($right_table.pid) > 0");
         $subOr->whereAnd("PAGEEXISTS($right_table.pid) = 1");
         $subOr->whereAnd("($right_table.rid != 0 OR ($subquery = 1 OR $subquery IS NULL))");
-        // FIXME: Need to consider how/whether to handle multi-valued columns
         return $result;
     }
 

From 2e6f73c77a0045bb10b915466075e390f1b30e67 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 01:15:13 +0000
Subject: [PATCH 19/27] Attempt to implement joins against page titles

This is known not to work when the RHS of the JOIN is a page title;
the SQL ends up broken somehow. It does work when only the LHS is a
title though
---
 _test/SearchTest.php       | 60 +++++++++++++++++++++++++++++++++++++-
 types/AbstractBaseType.php | 48 +++++++++++++++++++++++++++---
 2 files changed, 103 insertions(+), 5 deletions(-)

diff --git a/_test/SearchTest.php b/_test/SearchTest.php
index a95f3d0b..3eaa95f0 100644
--- a/_test/SearchTest.php
+++ b/_test/SearchTest.php
@@ -114,7 +114,20 @@ public function setUp(): void
             ],
             $now
         );
-        
+
+        $as->assignPageSchema('test:document3', 'schema2');
+        $this->saveData(
+            'test:document3',
+            'schema2',
+            [
+                'afirst' => 'test:document',
+                'asecond' => [],
+                'athird' => '1234',
+                'afourth' => 'abcd'
+            ],
+            $now
+        );
+
         for ($i = 10; $i <= 20; $i++) {
             $this->saveData(
                 "page$i",
@@ -560,6 +573,51 @@ public function test_join_pageid()
         $this->assertEquals('["page10",null]', $result[1][3]->getValue());
     }
 
+    public function test_join_pagetitle_against_string()
+    {
+        $search = new mock\Search();
+
+        $search->addSchema('schema2', 'foo');
+        $search->addSchema('pageschema', '', array('pageschema.%title%', '=', 'afirst'));
+        $this->assertEquals(2, count($search->schemas));
+
+        $this->assertEquals(1, count($search->joins));
+        $joincols = $search->joins['pageschema'];
+        $this->assertEquals(2, count($joincols));
+        $this->assertEquals('schema2', $joincols[0]->getTable());
+        $this->assertEquals('afirst', $joincols[0]->getLabel());
+        $this->assertEquals('pageschema', $joincols[1]->getTable());
+        $this->assertEquals('%title%', $joincols[1]->getLabel());
+ 
+        $search->addColumn('foo.%pageid%');
+        $search->addColumn('pageschema.%pageid%');
+        $search->addColumn('afourth');
+        $search->addColumn('singlepage');
+
+        $result = $search->execute();
+        $count = $search->getCount();
+
+        // check result dimensions
+        $this->assertEquals(2, $count, 'result count'); // full result set
+        $this->assertEquals(2, count($result), 'result rows'); // wanted result set
+
+        // check the values
+        $this->assertEquals('test:document2', $result[0][0]->getValue());
+        $this->assertEquals('page01', $result[0][1]->getValue());
+        $this->assertEquals('', $result[0][2]->getValue());
+        $this->assertEquals('page12', $result[0][3]->getValue());
+        $this->assertEquals('test:document3', $result[1][0]->getValue());
+        $this->assertEquals('test:document', $result[1][1]->getValue());
+        $this->assertEquals('abcd', $result[1][2]->getValue());
+        $this->assertEquals('page01', $result[1][3]->getValue());
+    }
+
+    public function test_join_string_against_pagetitle()
+    {
+    }
+
+    public function test_join_pagetitle_against_pagetitle() {
+    }
 
     public function invalidJoins_testdata() {
         return array(
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index a850c3b4..dc6d27c6 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -482,6 +482,34 @@ protected function wrapValue($value)
         return $value;
     }
 
+    /**
+     * Returns a SQL expression making an equality comparison between
+     * two values. If one of the values is an array, then the
+     * conditional expression will evaluate to true if the other value
+     * is equal to any element in the array. If both values are arrays
+     * then they must be the same length and the expression will
+     * evaluate to true if a pair of items with the same indices in
+     * the two arrays are equal.
+     *
+     * @param string|array $lhs The first value to be compared
+     * @param string|array $rhs The second value to be compared
+     * @return string SQL expression for equality comparison
+     */
+    protected function equalityComparison($lhs, $rhs)
+    {
+        $lhs_array = is_array($lhs);
+        $rhs_array = is_array($rhs);
+        $nlhs = $lhs_array ? count($lhs) : 1;
+        $nrhs = $rhs_array ? count($rhs) : 1;
+        if ($lhs_array and $rhs_array and count($lhs) != count($rhs)) {
+            throw new StructException("Arrays not of equal length.");
+        }
+        if (!$lhs_array) $lhs = array_fill(0, $nrhs, $lhs);
+        if (!$rhs_array) $rhs = array_fill(0, $nlhs, $rhs);
+        $comparisons = array_map(function ($l, $r) { return "($l = $r)"; }, $lhs, $rhs);
+        return implode(' OR ', $comparisons);
+    }
+
     /**
      * Returns a SQL expression ON which JOIN $left_table and
      * $right_table.  Semantically, this provides an
@@ -512,19 +540,31 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
             $lhs = $this->getSqlCompareValue($add, $left_table, null, $left_colname, $op);
         }
         $additional_join = $right_coltype->getAdditionalJoinForComparison($add, $right_table, $right_colname);
-        // FIXME: Are calls to wrapValue redundant, given that I'm already translating the current value using $this->getSqlCompareValue? They can be, if both lhs and rhs columns are of the same type.
         if (!is_null($additional_join)) {
+            // FIXME: This won't necessarily work over arrays
             $rhs = $this->wrapValue(
                 $right_coltype->getSqlCompareValue($add, $additional_join[2], $right_table, $right_colname, $op)
             );
-            $add->getQB()->addLeftJoin($left_table, $additional_join[1], $additional_join[2], "$lhs = $rhs");
-            $left_table = $additional_join[2];
             $result = $additional_join[3];
+            if (is_array($rhs)) {
+                // Case where right column is a Page type that is using titles
+                // FIXME: Bad to have implementation of subclass bleading in like this.
+                [$rhs_id, $rhs] = $rhs;
+                $result .= ' OR ' . $this->equalityComparison($lhs, $rhs_id);
+            }
+            $add->getQB()->addLeftJoin(
+                $left_table,
+                $additional_join[1],
+                $additional_join[2],
+                $this->equalityComparison($lhs, $rhs)
+            );
+            $left_table = $additional_join[2];
         } else {
+            // FIXME: This won't necessarily work over arrays
             $rhs = $this->wrapValue(
                 $right_coltype->getSqlCompareValue($add, $right_table, null, $right_colname, $op)
             );
-            $result = "$lhs = $rhs";
+            $result = $this->equalityComparison($lhs, $rhs);
         }
         // FIXME: Can I do this somehow without needing to execute the subquery twice?
         $AN = $add->getQB()->generateTableAlias('A');

From 38630c3b58a9f6addffaaedae6389f48bfcf65fd Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 02:19:31 +0000
Subject: [PATCH 20/27] Improved robustness of addLeftJoin

It now respects order not only for the existing table being joined
against, but also for any other tables referenced in the ON
clause. This was necessary to get custom joins against page titles to
work properly.
---
 _test/SearchTest.php  | 26 ++++++++++++++++++++++++++
 meta/QueryBuilder.php | 15 ++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/_test/SearchTest.php b/_test/SearchTest.php
index 3eaa95f0..43ffeb64 100644
--- a/_test/SearchTest.php
+++ b/_test/SearchTest.php
@@ -614,6 +614,32 @@ public function test_join_pagetitle_against_string()
 
     public function test_join_string_against_pagetitle()
     {
+        $search = new mock\Search();
+
+        $search->addSchema('pageschema', '');
+        $search->addSchema('schema2', 'foo', array('pageschema.%title%', '=', 'afirst'));
+        $search->addColumn('foo.%pageid%');
+        $search->addColumn('pageschema.%pageid%');
+        $search->addColumn('afourth');
+        $search->addColumn('singlepage');
+
+        $result = $search->execute();
+        $count = $search->getCount();
+
+        // check result dimensions
+        $this->assertEquals(2, $count, 'result count'); // full result set
+        $this->assertEquals(2, count($result), 'result rows'); // wanted result set
+
+        // check the values
+        $this->assertEquals('test:document2', $result[0][0]->getValue());
+        $this->assertEquals('page01', $result[0][1]->getValue());
+        $this->assertEquals('', $result[0][2]->getValue());
+        $this->assertEquals('page12', $result[0][3]->getValue());
+        $this->assertEquals('test:document3', $result[1][0]->getValue());
+        $this->assertEquals('test:document', $result[1][1]->getValue());
+        $this->assertEquals('abcd', $result[1][2]->getValue());
+        $this->assertEquals('page01', $result[1][3]->getValue());
+
     }
 
     public function test_join_pagetitle_against_pagetitle() {
diff --git a/meta/QueryBuilder.php b/meta/QueryBuilder.php
index 4bde4079..c88cba03 100644
--- a/meta/QueryBuilder.php
+++ b/meta/QueryBuilder.php
@@ -118,7 +118,20 @@ public function addLeftJoin($leftalias, $righttable, $rightalias, $onclause)
             throw new StructException('Table Alias already exists');
         }
 
-        $pos = array_search($leftalias, array_keys($this->from));
+        if (count($this->from) > 0){
+            $pos = 0;
+            $matches = [];
+            preg_match_all('/\w+(?=\.\w+)/', $onclause, $matches);
+            $matches[0][] = $leftalias;
+            $matches = array_unique($matches[0]);
+            $existing = array_keys($this->from);
+            foreach ($matches as $match) {
+                $p = array_search($match, $existing);
+                if ($p > $pos) $pos = $p;
+            }
+        } else {
+            $pos = false;
+        }
         $statement = "LEFT OUTER JOIN $righttable AS $rightalias ON $onclause";
         $this->from = $this->arrayInsert($this->from, [$rightalias => $statement], $pos + 1);
         $this->type[$rightalias] = 'join';

From de60ccfb0d999086f907b8aaa5b4d0b4be9c34b8 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 02:28:57 +0000
Subject: [PATCH 21/27] Fixed style problems flagged on CI

---
 meta/QueryBuilder.php      |  2 +-
 types/AbstractBaseType.php | 23 +++++++++++++++++++----
 types/AutoSummary.php      |  1 -
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/meta/QueryBuilder.php b/meta/QueryBuilder.php
index c88cba03..743e7538 100644
--- a/meta/QueryBuilder.php
+++ b/meta/QueryBuilder.php
@@ -118,7 +118,7 @@ public function addLeftJoin($leftalias, $righttable, $rightalias, $onclause)
             throw new StructException('Table Alias already exists');
         }
 
-        if (count($this->from) > 0){
+        if (count($this->from) > 0) {
             $pos = 0;
             $matches = [];
             preg_match_all('/\w+(?=\.\w+)/', $onclause, $matches);
diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index dc6d27c6..6f94784f 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -406,7 +406,12 @@ public function filter(QueryBuilderWhere $add, $tablealias, $colname, $comp, $va
     {
         $additional_join = $this->getAdditionalJoinForComparison($add, $tablealias, $colname);
         if (!is_null($additional_join)) {
-            $add->getQB()->addLeftJoin($additional_join[0], $additional_join[1], $additional_join[2], $additional_join[3]);
+            $add->getQB()->addLeftJoin(
+                $additional_join[0],
+                $additional_join[1],
+                $additional_join[2],
+                $additional_join[3]
+            );
             $oldalias = $tablealias;
             $tablealias = $additional_join[2];
         } else {
@@ -506,7 +511,13 @@ protected function equalityComparison($lhs, $rhs)
         }
         if (!$lhs_array) $lhs = array_fill(0, $nrhs, $lhs);
         if (!$rhs_array) $rhs = array_fill(0, $nlhs, $rhs);
-        $comparisons = array_map(function ($l, $r) { return "($l = $r)"; }, $lhs, $rhs);
+        $comparisons = array_map(
+            function ($l, $r) {
+                return "($l = $r)";
+            },
+            $lhs,
+            $rhs
+        );
         return implode(' OR ', $comparisons);
     }
 
@@ -531,8 +542,12 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
         $add = new QueryBuilderWhere($QB);
         $op = 'AND';
         $additional_join = $this->getAdditionalJoinForComparison($add, $left_table, $left_colname);
-        // FIXME: How to work with multiple lhs or rhs values (i.e., due to Page types)?
-        // In rhs secondary join, compare lhs ID and (possibly) title against rhs title only. In returned join condition, compare against secondary PID and the column value against lhs ID/title. At most one side of comparison will have multiple values.
+        // When dealing with joins over page types:
+        // In rhs secondary join, compare lhs ID and (possibly) title
+        // against rhs title only. In returned join condition, compare
+        // against secondary PID and the column value against lhs
+        // ID/title. At most one side of comparison will have multiple
+        // values.
         if (!is_null($additional_join)) {
             $add->getQB()->addLeftJoin($additional_join[0], $additional_join[1], $additional_join[2], $additional_join[3]);
             $lhs = $this->getSqlCompareValue($add, $additional_join[2], $left_table, $left_colname, $op);
diff --git a/types/AutoSummary.php b/types/AutoSummary.php
index 40c6ed6d..17395744 100644
--- a/types/AutoSummary.php
+++ b/types/AutoSummary.php
@@ -69,5 +69,4 @@ protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tabl
         $rightalias = $QB->generateTableAlias();
         return [$tablealias, 'titles', $rightalias, "$tablealias.pid = $rightalias.pid"];
     }
-
 }

From 9df21fccbf79ce9f1721c57726f1fecb4ba04e4d Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 02:33:03 +0000
Subject: [PATCH 22/27] Shortened long line

---
 types/AbstractBaseType.php | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 6f94784f..4ce70632 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -549,7 +549,9 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
         // ID/title. At most one side of comparison will have multiple
         // values.
         if (!is_null($additional_join)) {
-            $add->getQB()->addLeftJoin($additional_join[0], $additional_join[1], $additional_join[2], $additional_join[3]);
+            $add->getQB()->addLeftJoin(
+                $additional_join[0], $additional_join[1], $additional_join[2], $additional_join[3]
+            );
             $lhs = $this->getSqlCompareValue($add, $additional_join[2], $left_table, $left_colname, $op);
         } else {
             $lhs = $this->getSqlCompareValue($add, $left_table, null, $left_colname, $op);

From 2e02e3c09ad12db85b4fddc3f21efdef78ec456b Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 02:36:10 +0000
Subject: [PATCH 23/27] Fixed code style issues

---
 types/AbstractBaseType.php | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 4ce70632..efae80bd 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -550,7 +550,10 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
         // values.
         if (!is_null($additional_join)) {
             $add->getQB()->addLeftJoin(
-                $additional_join[0], $additional_join[1], $additional_join[2], $additional_join[3]
+                $additional_join[0],
+                $additional_join[1],
+                $additional_join[2],
+                $additional_join[3]
             );
             $lhs = $this->getSqlCompareValue($add, $additional_join[2], $left_table, $left_colname, $op);
         } else {

From 330a612ab4f157feb1963f7f9d5501955fd4ae8b Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 20:57:13 +0000
Subject: [PATCH 24/27] Added tests for joining against page titles and
 summaries

These both represent complicated joins which actually require joining
against more than 1 table. The page title one is particularly
complicated, as it should actually match against two possible
values (id or title).
---
 _test/SearchTest.php  | 120 +++++++++++++++++++++++++++++++++++++++++-
 types/AutoSummary.php |   2 +-
 2 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/_test/SearchTest.php b/_test/SearchTest.php
index 43ffeb64..c381a9b5 100644
--- a/_test/SearchTest.php
+++ b/_test/SearchTest.php
@@ -74,7 +74,7 @@ public function setUp(): void
             [
                 'first' => 'document first data',
                 'second' => ['second', 'more'],
-                'third' => '',
+                'third' => 'Summary',
                 'fourth' => 'fourth data'
             ],
             $now
@@ -573,6 +573,8 @@ public function test_join_pageid()
         $this->assertEquals('["page10",null]', $result[1][3]->getValue());
     }
 
+    // Test joining against Autosummary?
+    
     public function test_join_pagetitle_against_string()
     {
         $search = new mock\Search();
@@ -642,7 +644,123 @@ public function test_join_string_against_pagetitle()
 
     }
 
+    private function setup_pagetitle_join_test_page($letter, $title, $singletitle) {
+        $as = mock\Assignments::getInstance();
+        $letter_up = strtoupper($letter);
+        $now = time();
+        $name = "page_$letter";
+        $as->assignPageSchema($name, 'pageschema');
+        saveWikiText($name, "===== $title =====\nabc", "Summary");
+        p_get_metadata($name);
+        $this->saveData(
+            $name,
+            'pageschema',
+            ['singlepage' => $letter_up, 'multipage' => [], 'singletitle' => $singletitle, 'multititle' => []],
+            $now,
+        );
+        $this->saveData(
+            $name,
+            'schema1',
+            ['first' => "first $letter_up", 'second' => [], 'third' => '', 'fourth' => ''],
+            $now
+        );
+
+    }
+
     public function test_join_pagetitle_against_pagetitle() {
+        $this->setup_pagetitle_join_test_page('a', 'page_b', 'page_b');
+        $this->setup_pagetitle_join_test_page('b', 'B', 'page_c');
+        $this->setup_pagetitle_join_test_page('c', 'Title', 'Title');
+        $this->setup_pagetitle_join_test_page('d', 'Title', 'page_b');
+        
+        $search = new mock\Search();
+        $search->addSchema('pageschema', '');
+        $search->addSchema('schema1', 'foo', array('pageschema.singletitle', '=', 'foo.%title%'));
+        $search->addColumn('pageschema.%pageid%');
+        $search->addColumn('foo.%pageid%');
+
+        $result = $search->execute();
+        $count = $search->getCount();
+
+        $this->assertEquals(8, $count, 'result count'); // full result set
+        $this->assertEquals(8, count($result), 'result rows'); // wanted result set
+
+        // check the values
+        $this->assertEquals('page_a', $result[0][0]->getValue());
+        $this->assertEquals('page_a', $result[0][1]->getValue());
+
+        $this->assertEquals('page_a', $result[1][0]->getValue());
+        $this->assertEquals('page_b', $result[1][1]->getValue());
+
+        $this->assertEquals('page_b', $result[2][0]->getValue());
+        $this->assertEquals('page_c', $result[2][1]->getValue());
+
+        $this->assertEquals('page_b', $result[3][0]->getValue());
+        $this->assertEquals('page_d', $result[3][1]->getValue());
+
+        $this->assertEquals('page_c', $result[4][0]->getValue());
+        $this->assertEquals('page_c', $result[4][1]->getValue());
+
+        $this->assertEquals('page_c', $result[5][0]->getValue());
+        $this->assertEquals('page_d', $result[5][1]->getValue());
+
+        $this->assertEquals('page_d', $result[6][0]->getValue());
+        $this->assertEquals('page_a', $result[6][1]->getValue());
+
+        $this->assertEquals('page_d', $result[7][0]->getValue());
+        $this->assertEquals('page_b', $result[7][1]->getValue());
+    }
+
+    public function test_join_summary()
+    {
+        $search = new mock\Search();
+
+        $search->addSchema('schema1', 'foo');
+        $search->addSchema('schema2', '', array('schema1.third', '=', 'schema2.%lastsummary%'));
+
+        $search->addColumn('foo.%pageid%');
+        $search->addColumn('schema2.%pageid%');
+        $search->addColumn('afourth');
+        $search->addColumn('first');
+
+        $result = $search->execute();
+        $count = $search->getCount();
+
+        // check result dimensions
+        $this->assertEquals(1, $count, 'result count'); // full result set
+        $this->assertEquals(1, count($result), 'result rows'); // wanted result set
+
+        // check the values
+        $this->assertEquals('test:document', $result[0][0]->getValue());
+        $this->assertEquals('page01', $result[0][1]->getValue());
+        $this->assertEquals('fourth data', $result[0][2]->getValue());
+        $this->assertEquals('document first data', $result[0][3]->getValue());
+    }
+
+    public function test_join_summary2()
+    {
+        $search = new mock\Search();
+
+        $search->addSchema('schema2', '');
+        $search->addSchema('schema1', 'foo', array('schema1.third', '=', 'schema2.%lastsummary%'));
+
+        $search->addColumn('foo.%pageid%');
+        $search->addColumn('schema2.%pageid%');
+        $search->addColumn('afourth');
+        $search->addColumn('first');
+
+        $result = $search->execute();
+        $count = $search->getCount();
+
+        // check result dimensions
+        $this->assertEquals(1, $count, 'result count'); // full result set
+        $this->assertEquals(1, count($result), 'result rows'); // wanted result set
+
+        // check the values
+        $this->assertEquals('test:document', $result[0][0]->getValue());
+        $this->assertEquals('page01', $result[0][1]->getValue());
+        $this->assertEquals('fourth data', $result[0][2]->getValue());
+        $this->assertEquals('document first data', $result[0][3]->getValue());
     }
 
     public function invalidJoins_testdata() {
diff --git a/types/AutoSummary.php b/types/AutoSummary.php
index 17395744..219c8c8e 100644
--- a/types/AutoSummary.php
+++ b/types/AutoSummary.php
@@ -50,7 +50,7 @@ public function sort(QueryBuilder $QB, $tablealias, $colname, $order)
      */
     protected function getSqlCompareValue(QueryBuilderWhere &$add, $tablealias, $oldalias, $colname, &$op)
     {
-        return "$table.lastsummary";
+        return "$tablealias.lastsummary";
     }
 
     /**

From f4d9287d2b10cb5d18630270079fd5b98cc7f4c3 Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 22:14:56 +0000
Subject: [PATCH 25/27] Move page-type-specific join logic into separate method

---
 types/AbstractBaseType.php | 34 ++++++++++++++++++++--------------
 types/Page.php             | 19 +++++++++++++++++++
 2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index efae80bd..9bd112cd 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -522,7 +522,25 @@ function ($l, $r) {
     }
 
     /**
-     * Returns a SQL expression ON which JOIN $left_table and
+     * Returns a SQL expression on which to join two tables, when the
+     * column of the right table being joined on is of this data
+     * type. This should only be called if joining on this data type
+     * requires introducing an additional join (i.e., if
+     * getAdditionalJoinForComparison returns an array).
+     *
+     * @param QueryBuilder $QB
+     * @param string $lhs Left hand side of the ON clause (for left table)
+     * @param string $rhs Right hand side of the ON clause (for right table)
+     * @param string $additional_join_condition The ON clause of the additional join
+     * @return string SQL expression to be returned by joinCondition
+     */
+    protected function joinConditionIfAdditionalJoin($lhs, &$rhs, $additional_join_condition)
+    {
+        return $additional_join_condition;
+    }
+
+    /**
+     * Returns a SQL expression ON which to JOIN $left_table and
      * $right_table.  Semantically, this provides an
      * equality comparison between two columns in the two
      * schemas. However, in practice it may require more complex
@@ -542,12 +560,6 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
         $add = new QueryBuilderWhere($QB);
         $op = 'AND';
         $additional_join = $this->getAdditionalJoinForComparison($add, $left_table, $left_colname);
-        // When dealing with joins over page types:
-        // In rhs secondary join, compare lhs ID and (possibly) title
-        // against rhs title only. In returned join condition, compare
-        // against secondary PID and the column value against lhs
-        // ID/title. At most one side of comparison will have multiple
-        // values.
         if (!is_null($additional_join)) {
             $add->getQB()->addLeftJoin(
                 $additional_join[0],
@@ -565,13 +577,7 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
             $rhs = $this->wrapValue(
                 $right_coltype->getSqlCompareValue($add, $additional_join[2], $right_table, $right_colname, $op)
             );
-            $result = $additional_join[3];
-            if (is_array($rhs)) {
-                // Case where right column is a Page type that is using titles
-                // FIXME: Bad to have implementation of subclass bleading in like this.
-                [$rhs_id, $rhs] = $rhs;
-                $result .= ' OR ' . $this->equalityComparison($lhs, $rhs_id);
-            }
+            $result = $right_coltype->joinConditionIfAdditionalJoin($lhs, $rhs, $additional_join[3]);
             $add->getQB()->addLeftJoin(
                 $left_table,
                 $additional_join[1],
diff --git a/types/Page.php b/types/Page.php
index 60702df8..2501d243 100644
--- a/types/Page.php
+++ b/types/Page.php
@@ -225,6 +225,25 @@ protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tabl
         return [$tablealias, 'titles', $rightalias, "$tablealias.$colname = $rightalias.pid"];
     }
 
+    /**
+     * Returns a SQL expression on which to join two tables, when the
+     * column of the right table being joined on is of this data
+     * type. This should only be called if joining on this data type
+     * requires introducing an additional join (i.e., if
+     * getAdditionalJoinForComparison returns an array).
+     *
+     * @param QueryBuilder $QB
+     * @param string $lhs Left hand side of the ON clause (for left table)
+     * @param string $rhs Right hand side of the ON clause (for right table)
+     * @param string $additional_join_condition The ON clause of the additional join
+     * @return string SQL expression to be returned by joinCondition
+     */
+    protected function joinConditionIfAdditionalJoin($lhs, &$rhs, $additional_join_condition)
+    {
+        [$rhs_id, $rhs] = $rhs;
+        return $additional_join_condition . ' OR ' . $this->equalityComparison($lhs, $rhs_id);
+    }
+
     /**
      * Check if the given id matches a configured filter pattern
      *

From 474f0eab176d0481dec16fc0aaaab014410aaf1e Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 22:37:54 +0000
Subject: [PATCH 26/27] Make sure can wrap either scalar or arrays of values

---
 types/AbstractBaseType.php | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 9bd112cd..162f1787 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -473,6 +473,20 @@ protected function getAdditionalJoinForComparison(QueryBuilderWhere &$add, $tabl
         return null;
     }
 
+    /**
+     * Handle the value(s) that a column is being compared against. If $value is an array, each element will be wrapped.
+     *
+     * @param string|array $value The value(s) a column is being compared to
+     * @return string|array SQL expression(s) processing the value in some way.
+     */
+    protected function wrapValues($value)
+    {
+        if (is_array($value)) {
+            return array_map([$this, 'wrapValue'], $value);
+        }
+        return $this->wrapValue($value);
+    }
+    
     /**
      * Handle the value that a column is being compared against. In
      * most cases this method will just return the value unchanged,
@@ -573,8 +587,7 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
         }
         $additional_join = $right_coltype->getAdditionalJoinForComparison($add, $right_table, $right_colname);
         if (!is_null($additional_join)) {
-            // FIXME: This won't necessarily work over arrays
-            $rhs = $this->wrapValue(
+            $rhs = $this->wrapValues(
                 $right_coltype->getSqlCompareValue($add, $additional_join[2], $right_table, $right_colname, $op)
             );
             $result = $right_coltype->joinConditionIfAdditionalJoin($lhs, $rhs, $additional_join[3]);
@@ -586,8 +599,7 @@ public function joinCondition($QB, &$left_table, $left_colname, $right_table, $r
             );
             $left_table = $additional_join[2];
         } else {
-            // FIXME: This won't necessarily work over arrays
-            $rhs = $this->wrapValue(
+            $rhs = $this->wrapValues(
                 $right_coltype->getSqlCompareValue($add, $right_table, null, $right_colname, $op)
             );
             $result = $this->equalityComparison($lhs, $rhs);

From bbd6891e896a1da10914a07db8b1303ef32b122c Mon Sep 17 00:00:00 2001
From: Chris MacMackin <cmacmackin@gmail.com>
Date: Thu, 30 Nov 2023 22:48:47 +0000
Subject: [PATCH 27/27] Removed trailing whitespace

---
 types/AbstractBaseType.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/types/AbstractBaseType.php b/types/AbstractBaseType.php
index 162f1787..05225773 100644
--- a/types/AbstractBaseType.php
+++ b/types/AbstractBaseType.php
@@ -486,7 +486,7 @@ protected function wrapValues($value)
         }
         return $this->wrapValue($value);
     }
-    
+
     /**
      * Handle the value that a column is being compared against. In
      * most cases this method will just return the value unchanged,