Doctrine DBAL
  1. Doctrine DBAL
  2. DBAL-474

SchemaManager / Connection on PostgreSQL platform does not respect filterExpression for sequences

    Details

    • Type: Bug Bug
    • Status: In Progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.2.2
    • Fix Version/s: None
    • Component/s: Schema Managers
    • Security Level: All
    • Environment:
      Windows & Linux

      Description

      Dear Symfony team,

      the filterExpression on AbstractSchemaManager seems not to work for sequences.

      This only happens under postgres.

      It seems the way the sequences are handled are the culprit: It tries to get min_value etc of sequences without matching sequence names to the filter expression in advance.

      If for example access to the sequences is denied, (Different schema without permissions for the current entity manager), any higher-level ORM operations like generating migration versions fail.

      --------------------- UPDATE

      the context is when using migrations. Positive regexp expressions do not limit the migration to a single schema. eg ^schemaname.$
      Instead, all sequences on the current database are returned.
      When trying to limit a migration to a single schema consecutively this doesn't work.
      We are using a per-schema connection, so this results in a lot of hassle for us.

        Activity

        jos de witte created issue -
        Hide
        Benjamin Eberlei added a comment -

        Can you paste an exception trace? I see that filtering is applied to sequences, but your description seems to indicate this happens due to an SQL query much earlier?

        Show
        Benjamin Eberlei added a comment - Can you paste an exception trace? I see that filtering is applied to sequences, but your description seems to indicate this happens due to an SQL query much earlier?
        Benjamin Eberlei made changes -
        Field Original Value New Value
        Status Open [ 1 ] Awaiting Feedback [ 10000 ]
        Hide
        jos de witte added a comment -

        Dear Benjamin,

        the context is when using migrations. Positive regexp expressions do not limit the migration to a single schema. eg ^schemaname.$
        Instead, all sequences on the current database are returned.
        When trying to limit a migration to a single schema consecutively this doesn't work.
        We are using a per-schema connection, so this results in a lot of hassle for us.

        Show
        jos de witte added a comment - Dear Benjamin, the context is when using migrations. Positive regexp expressions do not limit the migration to a single schema. eg ^schemaname.$ Instead, all sequences on the current database are returned. When trying to limit a migration to a single schema consecutively this doesn't work. We are using a per-schema connection, so this results in a lot of hassle for us.
        jos de witte made changes -
        Description Dear Symfony team,

        the filterExpression on AbstractSchemaManager seems not to work for sequences.

        This only happens under postgres.

        It seems the way the sequences are handled are the culprit: It tries to get min_value etc of sequences without matching sequence names to the filter expression in advance.

        If for example access to the sequences is denied, (Different schema without permissions for the current entity manager), any higher-level ORM operations like generating migration versions fail.
        Dear Symfony team,

        the filterExpression on AbstractSchemaManager seems not to work for sequences.

        This only happens under postgres.

        It seems the way the sequences are handled are the culprit: It tries to get min_value etc of sequences without matching sequence names to the filter expression in advance.

        If for example access to the sequences is denied, (Different schema without permissions for the current entity manager), any higher-level ORM operations like generating migration versions fail.

        --------------------- UPDATE

        the context is when using migrations. Positive regexp expressions do not limit the migration to a single schema. eg ^schemaname.$
        Instead, all sequences on the current database are returned.
        When trying to limit a migration to a single schema consecutively this doesn't work.
        We are using a per-schema connection, so this results in a lot of hassle for us.
        Hide
        Steve Müller added a comment -

        jos de witte I think your issue got fixed in commit: https://github.com/doctrine/dbal/commit/8beb732fe9d5cd40a0d677f250d2be325482744f
        This patch was first introduced in 2.3. Can you please confirm that this is fixed? Otherwise can you please provide a concrete example so that we can reproduce you issue?

        Show
        Steve Müller added a comment - jos de witte I think your issue got fixed in commit: https://github.com/doctrine/dbal/commit/8beb732fe9d5cd40a0d677f250d2be325482744f This patch was first introduced in 2.3. Can you please confirm that this is fixed? Otherwise can you please provide a concrete example so that we can reproduce you issue?
        Hide
        Arnout Standaert added a comment -

        I believe we are bumping into the same issue. Our webapp uses Migrations, but for our webapp we are limited to a certain schema within a bigger PostgreSQL database. We only have permissions on our own schema and public.
        Now, listSequences in AbstractSchemaManager does filter the asset names correctly with the mentioned fix.

        But the problem is with the step before, _getPortableSequencesList (see line 135 of AbstractSchemaManager):

                return $this->filterAssetNames($this->_getPortableSequencesList($sequences));
        

        This function goes out and does a _getPortableSequenceDefinition on every sequence in the unfiltered list. For every sequence, the _getPortableSequenceDefinition in PostgreSqlSchemaManager performs a SELECT:

                $data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM ' . $this->_platform->quoteIdentifier($sequenceName));
        

        Now, our user role in the database doesn't have SELECT permissions on these sequences in other schemas, so the migration fails with a privilege error.

        There should be some kind of filtering on the sequence list BEFORE the SELECT statement in the _getPortableSequenceDefinition function are performed, no?

        Show
        Arnout Standaert added a comment - I believe we are bumping into the same issue. Our webapp uses Migrations, but for our webapp we are limited to a certain schema within a bigger PostgreSQL database. We only have permissions on our own schema and public . Now, listSequences in AbstractSchemaManager does filter the asset names correctly with the mentioned fix. But the problem is with the step before, _getPortableSequencesList (see line 135 of AbstractSchemaManager ): return $ this ->filterAssetNames($ this ->_getPortableSequencesList($sequences)); This function goes out and does a _getPortableSequenceDefinition on every sequence in the unfiltered list. For every sequence, the _getPortableSequenceDefinition in PostgreSqlSchemaManager performs a SELECT : $data = $ this ->_conn->fetchAll('SELECT min_value, increment_by FROM ' . $ this ->_platform->quoteIdentifier($sequenceName)); Now, our user role in the database doesn't have SELECT permissions on these sequences in other schemas, so the migration fails with a privilege error. There should be some kind of filtering on the sequence list BEFORE the SELECT statement in the _getPortableSequenceDefinition function are performed, no?
        Hide
        Arnout Standaert added a comment -

        I currently have a workaround running locally, which filters the sequences list before creating the PortableSequence's. This is probably hackish and a poor workaround, just posting here as a temporary solution and further illustration of the problem.

        Altered line 135 in AbstractSchemaManager:

                return $this->_getPortableSequencesList($this->filterSequenceNames($sequences));
        

        I added function filterSequenceNames() in AbstractSchemaManager for appropriate sequence filtering:

            /**
             * Filters sequence names if they are configured to return only a subset of all
             * the found elements.
             *5
             * @param array $sequenceNames
             *
             * @return array
             */
            protected function filterSequenceNames($sequenceNames)
            {
                $filterExpr = $this->getFilterSchemaAssetsExpression();
                if ( ! $filterExpr) {
                    return $sequenceNames;
                }
                
                return array_values ( array_filter($sequences, function ($sequenceName) use ($filterExpr) {
                        $sequenceName = $sequenceName["schemaname"].".".$sequenceName["relname"];
                        return preg_match($filterExpr, $sequenceName);
                    })
                );
        
            }
        

        After these modifications, doctrine:migrations:migrate operations complete succesfully, even with our limited-permission database account.

        Show
        Arnout Standaert added a comment - I currently have a workaround running locally, which filters the sequences list before creating the PortableSequence 's. This is probably hackish and a poor workaround, just posting here as a temporary solution and further illustration of the problem. Altered line 135 in AbstractSchemaManager : return $ this ->_getPortableSequencesList($ this ->filterSequenceNames($sequences)); I added function filterSequenceNames() in AbstractSchemaManager for appropriate sequence filtering: /** * Filters sequence names if they are configured to return only a subset of all * the found elements. *5 * @param array $sequenceNames * * @ return array */ protected function filterSequenceNames($sequenceNames) { $filterExpr = $ this ->getFilterSchemaAssetsExpression(); if ( ! $filterExpr) { return $sequenceNames; } return array_values ( array_filter($sequences, function ($sequenceName) use ($filterExpr) { $sequenceName = $sequenceName[ "schemaname" ]. "." .$sequenceName[ "relname" ]; return preg_match($filterExpr, $sequenceName); }) ); } After these modifications, doctrine:migrations:migrate operations complete succesfully, even with our limited-permission database account.
        Hide
        Steve Müller added a comment -

        Arnout Standaert Your fix looks promising and reasonable. Can you create a PR on the DBAL repo for that?

        Show
        Steve Müller added a comment - Arnout Standaert Your fix looks promising and reasonable. Can you create a PR on the DBAL repo for that?
        Hide
        Viktor Sidochenko added a comment -

        Why this fix is not in master?

        Show
        Viktor Sidochenko added a comment - Why this fix is not in master?
        Hide
        Steve Müller added a comment -

        Viktor Sidochenko because nobody has fixed it yet Feel free to provide a patch on GitHub.

        Show
        Steve Müller added a comment - Viktor Sidochenko because nobody has fixed it yet Feel free to provide a patch on GitHub.
        Hide
        Arnout Standaert added a comment -

        I haven't gotten around to doing the PR on GitHub yet, I'm not yet too familiar with that.
        I'll try to find some time for this the coming days.

        Show
        Arnout Standaert added a comment - I haven't gotten around to doing the PR on GitHub yet, I'm not yet too familiar with that. I'll try to find some time for this the coming days.
        Hide
        Viktor Sidochenko added a comment -

        Will be good. I`m not professional developer to make patches to upstream. So just voted for this issue.

        Show
        Viktor Sidochenko added a comment - Will be good. I`m not professional developer to make patches to upstream. So just voted for this issue.
        Steve Müller made changes -
        Assignee Benjamin Eberlei [ beberlei ] Steve Müller [ deeky666 ]
        Steve Müller made changes -
        Status Awaiting Feedback [ 10000 ] In Progress [ 3 ]
        Hide
        Steve Müller added a comment -

        Patch supplied in PR: https://github.com/doctrine/dbal/pull/546
        jos de witte, Arnout Standaert, Viktor Sidochenko can you please test if the supplied PR fixes the problem?

        Show
        Steve Müller added a comment - Patch supplied in PR: https://github.com/doctrine/dbal/pull/546 jos de witte , Arnout Standaert , Viktor Sidochenko can you please test if the supplied PR fixes the problem?

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DBAL-474, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Steve Müller
            Reporter:
            jos de witte
          • Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated: