Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel 9, Postgres can't change search_path to valid schema #41062

Closed
xorock opened this issue Feb 16, 2022 · 3 comments · Fixed by #41088
Closed

Laravel 9, Postgres can't change search_path to valid schema #41062

xorock opened this issue Feb 16, 2022 · 3 comments · Fixed by #41088
Labels

Comments

@xorock
Copy link

xorock commented Feb 16, 2022

  • Laravel Version: 9.1.0
  • PHP Version: 8.1.2
  • Database Driver & Version: PostgreSQL 14.1 (Debian 14.1-1.pgdg110+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 10.2.1-6) 10.2.1 20210110, 64-bit

Description:

I am trying to change the search_path to the current schema and unfortunately neither queries nor migrations work. Laravel fails to connect to the database on the defined schema.

Steps To Reproduce:

  • create new laravel 9 project
  • change driver to pgsql
  • create test-db schema
  • change database.php, postgres setting to 'search_path' => 'test-db',
  • try to run migration or select query from database

dd(DB::getPdo()->query('select current_schemas(true)')->fetchAll());

^ array:1 [[▼]()
  0 => array:2 [[▼]()
    "current_schemas" => "{pg_catalog}"
    0 => "{pg_catalog}"
  ]
]
php artisan migrate

   Illuminate\Database\QueryException

  SQLSTATE[3F000]: Invalid schema name: 7 ERROR:  no schema has been selected to create in at character 14 (SQL: create table "migrations" ("id" serial primary key not null, "migration" varchar(255) not null, "batch" integer not null))

  at vendor/laravel/framework/src/Illuminate/Database/Connection.php:712
    708▕         // If an exception occurs when attempting to run a query, we'll format the error
    709▕         // message to include the bindings with SQL, which will make this exception a
    710▕         // lot more helpful to the developer instead of just the database's errors.
    711▕         catch (Exception $e) {
  ➜ 712▕             throw new QueryException(
    713▕                 $query, $this->prepareBindings($bindings), $e
    714▕             );
    715▕         }
    716▕     }

      +37 vendor frames
  38  artisan:37
      Illuminate\Foundation\Console\Kernel::handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
@derekmd
Copy link
Contributor

derekmd commented Feb 16, 2022

Character - in test-db is the issue.

protected function parseSearchPath($searchPath)
{
if (is_string($searchPath)) {
preg_match_all('/[a-zA-z0-9$]{1,}/i', $searchPath, $matches);
$searchPath = $matches[0];
}

It looks PostgresConnector's parsing of config('database.connections.pgsql.search_path') doesn't cover every accepted Postgres symbol character.

preg_match_all('/[a-zA-z0-9$]{1,}/i', 'test-db', $matches);
$matches;
// [
//     [
//         "test",
//         "db",
//     ],
// ]

which should be [["test-db"]].


Funnily enough, 'test_db' is (correctly) parsed only because the regex mistakenly (?) contains range [A-z] instead of [A-Z]:

preg_match_all('/[a-zA-z0-9$]{1,}/i', 'test_db', $matches);
$matches;
// [
//     [
//         "test_db",
//     ],
// ]

Regex '/[a-z0-9$_\-]{1,}/i' might fix it but I have to find an exact definition in the Postgres docs. e.g., whether extended ASCII or multibyte characters are accepted too.


Edit: Yup, ¡tëst-db! is an acceptable schema name too.

According the originating PR's examples, a naive negating regex might be the easiest solution for parsing the config string:

preg_match_all('/[^\s,"\']+/', $searchPath, $matches);

@xorock
Copy link
Author

xorock commented Feb 17, 2022

This is also funny because, according to this code, when I pass search_path as array it will work - which looks like a nice workaround temporarily.

But.... also looking at method below

protected function quoteSearchPath($searchPath)

It looks potentially buggy. It uses count($searchPath) and according to phpdoc @param array|string $searchPath. So, if string is passed it will throw Fatal error: Uncaught TypeError: count(): Argument #1 ($var) must be of type Countable|array, string given. I see that configureSearchPath first uses parseSearchPath, but if so then $searchPath should be typehinted to array.

@driesvints
Copy link
Member

@derekmd can you maybe PR that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants