feat: Add support for pgsql's search_path

[johannsa]

Fixes #4240

Changes proposed in this pull request:
Added support for specifying and using pgsql's search_path

Reviewers should focus on:

• Some changes to existing classes and method signatures.
• Some changes to UI:
  • Addition of a "Schema" field.
  • Triggering the update of the UI upon DOM loading, so that the field is only displayed for the relevant driver.

Screenshot

Necessity

• Has the problem that is being solved here been clearly explained?
• If applicable, have various options for solving this problem been considered?
  • Not acceptable: Install in "public" schema, then move tables to other schema and finally change config.php.
• For core PRs, does this need to be in core, or could it be in an extension?
  • Needs to be in core as the issue/limitation exists since install.
• Are we willing to maintain this for years / potentially forever?

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).
  • There is no such command on my composer.jsons
• Core developer confirmed locally this works as intended.
• Tests have been added, or are not appropriate here.

Required changes:

• Related documentation PR: (Remove if irrelevant)

[johannsa]

I am working on those failed test ✅ Done!

[imorland]

Hello @johannsa and thanks for the PR!

I must admit I'm not overly familiar with pgsql, so I relied on AI to help with reviewing this. Output is below:

---

In PostgreSQL, search_path is similar to namespaces in other languages. It determines which schema(s) PostgreSQL searches when you reference a table without specifying the schema explicitly. By default, PostgreSQL uses the public schema, but in enterprise environments or multi-tenant applications, you might want to use different schemas to organize or isolate data.

For example:

• public.users - table in the public schema
• myapp.users - table in the myapp schema

The search_path setting tells PostgreSQL which schema to use by default.

---

Issues Found in the PR:

🔴 Critical Issue #1: Schema Validation Logic is Wrong

In the PR diff for DatabaseConfig.php, lines 62-64 add problematic validation:

if (empty($this->schema) && $this->driver == 'pgsql') {
    throw new ValidationFailed('Please specify the schema name.');
}

Problem: This makes the schema required for PostgreSQL, but based on the PR, the default should be 'public'. This validation will fail when $this->schema is null even though the code elsewhere treats null as defaulting to 'public'. What should happen:

• Either remove this validation entirely (since there's a default)
• Or change it to validate the schema name format if provided, but allow it to be empty/null

🟡 Issue #2: Inconsistent Default Handling

The PR has inconsistent default value handling:

1. In Migrator.php:230: $pgSearchPath = $dbConfig->toArray()['search_path'] ?? 'public';
2. In UserDataProvider.php:63: Default is 'public' when asking user
3. In FileDataProvider.php:76: $this->databaseConfiguration['search_path'] ?? 'public'
4. In install.php (from PR): <input class="FormControl" name="dbSchema" value="public">

Problem: The DatabaseConfig constructor receives this value but the driverOptions() method at line 105 (from PR) has:

'search_path' => $this->schema,

If $this->schema is null, this will set search_path to null instead of 'public'.

Solution: In DatabaseConfig, either:

• Make the default 'public' in the constructor parameter: private readonly string $schema = 'public'
• Or handle the null case in driverOptions(): 'search_path' => $this->schema ?? 'public'

🟡 Issue #3: Regex Replacement Could Be Fragile

In the PR's Migrator.php changes:

if ($driver === 'pgsql' && $pgSearchPath != 'public') {
    $statement = preg_replace('/(public)([\s.;])/', "$pgSearchPath\$2", $statement);
}

Concerns:

1. This regex will replace ANY occurrence of the word "public" followed by whitespace, dot, or semicolon - even in comments, string literals, or column names
2. The pgsql-install.dump has SELECT pg_catalog.set_config('search_path', '', false);, CREATE TABLE public.db_prefix_access_tokens, and COMMENT ON SCHEMA public IS ''

Better approach:

• Use word boundaries for safety: /\bpublic\b([\s.;])/
• This ensures you don't accidentally replace "public" within other words

However, since tests are passing with PostgreSQL 10, this might work in practice. Just be aware it's a bit fragile.

🟢 Good Things:

1. UI changes look good - The schema field only shows for PostgreSQL with the data-group="pgsql" attribute and the JavaScript properly triggers on page load via dbDriver.dispatchEvent(new Event('change', { bubbles: true }));
2. Tests all pass - Including PostgreSQL 10 tests with prefix
3. Signature changes are consistent - All places that create DatabaseConfig have been updated to include the schema parameter
4. The feature is genuinely useful - Many organizations require non-public schemas for security/organization reasons

Recommendations

Must Fix:

1. Fix the validation logic - Either remove the required check for schema or handle the null/default properly
2. Fix default value handling - Ensure $this->schema defaults to 'public' consistently

Nice to Have:

1. Improve the regex - Make it more robust with word boundaries: /\bpublic\b([\s.;])/
2. Add a comment explaining why the regex replacement is needed (for developers reading the code later)

[johannsa]

Thanks for your comments, @imorland!

Issues Found in the PR:

🔴 Critical Issue #1: Schema Validation Logic is Wrong

In the PR diff for DatabaseConfig.php, lines 62-64 add problematic validation:

if (empty($this->schema) && $this->driver == 'pgsql') {
    throw new ValidationFailed('Please specify the schema name.');
}

Problem: This makes the schema required for PostgreSQL, but based on the PR, the default should be 'public'. This validation will fail when $this->schema is null even though the code elsewhere treats null as defaulting to 'public'. What should happen:

• Either remove this validation entirely (since there's a default)
• Or change it to validate the schema name format if provided, but allow it to be empty/null

I believe that at the driver level, we should avoid assumptions and rigid defaults (especially when these can be changed). This is consistent with the same way that the driver is not assuming the ports for each drive (e.g. 3306 for MySQL, 5432 for PostgreSQL, etc.).

At some point, for the sake of flexibility, we might want to re-evaluate other hardcoded assumptions and rigid defaults that are still in the DatabaseConfig.php such as the character sets and collations.

🟡 Issue #2: Inconsistent Default Handling

The PR has inconsistent default value handling:

1. In Migrator.php:230: $pgSearchPath = $dbConfig->toArray()['search_path'] ?? 'public';

I have not tested it, but perhaps we could remove that default value as it should never be null since validation to ensure that this is not the case is enforced in the constructor (see here).

2. In UserDataProvider.php:63: Default is 'public' when asking user

This is a sane default in user input that should cover the majority of cases, reduce the amount of nulls sent to DatabaseConfig and help with UX.

3. In FileDataProvider.php:76: $this->databaseConfiguration['search_path'] ?? 'public'

This is avoid breaking existing automations and scripts depending on the current behaviour (backwards compatibility).

4. In install.php (from PR): <input class="FormControl" name="dbSchema" value="public">

[Same as 2] This is a sane default in user input that should cover the majority of cases, reduce the amount of nulls sent to DatabaseConfig and help with UX.

Problem: The DatabaseConfig constructor receives this value but the driverOptions() method at line 105 (from PR) has:

'search_path' => $this->schema,

If $this->schema is null, this will set search_path to null instead of 'public'.

Solution: In DatabaseConfig, either:

• Make the default 'public' in the constructor parameter: private readonly string $schema = 'public'
• Or handle the null case in driverOptions(): 'search_path' => $this->schema ?? 'public'

This should not be an issue since validation to ensure that this is not the case is enforced in the constructor (see here) and we should reasonably expect that any code calling the constructor is doing so with the required parameters, would handle exceptions during validation, etc... And also for host, port, etc.!

🟡 Issue #3: Regex Replacement Could Be Fragile

In the PR's Migrator.php changes:

if ($driver === 'pgsql' && $pgSearchPath != 'public') {
    $statement = preg_replace('/(public)([\s.;])/', "$pgSearchPath\$2", $statement);
}

Concerns:

1. This regex will replace ANY occurrence of the word "public" followed by whitespace, dot, or semicolon - even in comments, string literals, or column names

This is to deal with the high possibility that the schema dump will possibly be with schema "public" kind of "hardcoded" and that we need to change that before we install in a separate schema.

2. The pgsql-install.dump has SELECT pg_catalog.set_config('search_path', '', false);, CREATE TABLE public.db_prefix_access_tokens, and COMMENT ON SCHEMA public IS ''

See above.

Better approach:

Use word boundaries for safety: /\bpublic\b([\s.;])/
This ensures you don't accidentally replace "public" within other words

Will test that suggestion and, if it works, add it to my PR to avoid any tech debt.

In the meantime, please, let me know what you think about my reply to your comments in Issue 1 and Issue 3.

Thanks beforehand!