Skip to content

Support for custom schemas per database and custom extensions per schema #147

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

Closed
wants to merge 2 commits into from
Closed

Conversation

YetAnotherMinion
Copy link
Contributor

Resolves #146 and #140

Removes the top level configuration of schemas and extensions in the pillar and
moved schemas under the scope of each individual db configuration. Now it is
possible to use the same name for a schema in different databases. Extensions are
configured as a list under each schema configuration. Now it is possible to
install the same extension under multiple schemas in the same database and
different databases. I updated the pillar.example to reflect the new structure.

Resolves #146 and #140

Removes the top level configuration of schemas and extensions in the pillar and
moved schemas under the scope of each individual db configuration. Now it is
possible to use the same name for a schema in different databases. Extensions are
configured as a list under each schema configuration. Now it is possible to
install the same extension under multiple schemas in the same database and
different databases. I updated the pillar.example to reflect the new structure.
@@ -34,6 +36,7 @@ postgres-reload-modules:
# Tablespace states

{%- for name, tblspace in postgres.tablespaces|dictsort() %}
{%- do tblspace.update({'name': name}) %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required because I removed the automatic generation of name parameter from format_state macro

@YetAnotherMinion
Copy link
Contributor Author

@ek9 I took a shot at solving the per db extensions and schemas problem. I got it working on one instance of postgres in dev. I have not tried the pilliar.example file verbatim though.

@YetAnotherMinion
Copy link
Contributor Author

I am having trouble getting the extensions to install on existing databases with this patch.

@YetAnotherMinion YetAnotherMinion changed the title Support for custom schemas per database and custom extensions per schema [WIP] Support for custom schemas per database and custom extensions per schema Feb 9, 2017
@YetAnotherMinion YetAnotherMinion changed the title [WIP] Support for custom schemas per database and custom extensions per schema Support for custom schemas per database and custom extensions per schema Feb 9, 2017
@YetAnotherMinion
Copy link
Contributor Author

@javierbertoli Are you free to review this, or recommend anyone else?

@noelmcloughlin
Copy link
Contributor

noelmcloughlin commented Feb 6, 2018

@YetAnotherMinion I have tested PR successfully with one cavet.

Regarding your comments ....

I am having trouble getting the extensions to install on existing databases with this patch.

This is by design. Salt info messages report failure, but debug logging shows "The tablespace directory pg_tblspc/bla/bla/bla is missing". For explanation, see the red warning on this page for reason. I don't see this as issue.

I have not tried the pilliar.example file verbatim though.

The following works on PG 9.6 and 10 with cavet (see below).

..........
    db2:
      owner: 'remoteUser'
      template: 'template0'
      lc_ctype: 'en_US.UTF-8'
      lc_collate: 'en_US.UTF-8'
      tablespace: 'my_space'

      # set custom schema
      schemas:
        public:
          owner: 'localUser'

Cavet: SLS ID name collisions occur if an extension appears in more than one schema (same DB). Is this feature, or limitation, of your implementation???

          **# enable per-schema extension
         # extensions:
         #--> name collision!!!!  - uuid-ossp**

Workaround: Commented out above to fix uuid-ossp name collision. Tested following schema....

        secret_business_logic:
          owner: 'remoteUser'
          # enable per-schema extension
          extensions:
            - autoinc
            - btree_gin
            - btree_gist
            - chkpass
            - citext
            - cube
            - dblink
            - dict_int
            - dict_xsyn
            - earthdistance
            - file_fdw
            - fuzzystrmatch
            - hstore
            - insert_username
            - intagg
            - intarray
            - isn
            - lo
            - ltree
            - moddatetime
            - pageinspect
            - pg_buffercache
            - pg_freespacemap
            - pg_stat_statements
            - pg_trgm
            - pgcrypto
            - pgrowlocks
            - pgstattuple
            - postgres_fdw
            - refint
            - seg
            - sslinfo
            - tablefunc
            - tcn
            - timetravel
            - unaccent
            - uuid-ossp
            - xml2

OLD SOLUTION (working pillars)
Is your PR improving solution? If yes, good, but your PR description should communicate this!!!

    db2:
      owner: 'remoteUser'
      template: 'template0'
      lc_ctype: 'en_US.UTF-8'
      lc_collate: 'en_US.UTF-8'
      tablespace: 'my_space'
      # set custom schema
      schemas:
        public:
          owner: 'localUser'
         # enable per-db extension
      extensions:
        adminpack:
          schema: 'pg_catalog'
        autoinc:
          schema: 'public'
..... etc ...... lots more.

schemas:
    pg_catalog:
      dbname: db1
      owner: localUser
    autoinc:
      dbname: db1
      owner: localUser
....... etc .......... lots more ...

  # optional extensions to install in schema
 extensions:
    adminpack:
      schema: pg_catalog
      maintenance_db: db1
    autoinc:
      schema: autoinc
      maintenance_db: db1
..... etc .... lots more ....

@YetAnotherMinion
Copy link
Contributor Author

Cavet: SLS ID name collisions occur if an extension appears in more than one schema (same DB). Is this feature, or limitation, of your implementation???

@noelmcloughlin It is a mixture. On one hand it matches the behavior of Postgres exactly, where you cannot install an extension more than once per database. On the other hand, it should be possible to omit the schema attribute for an extension, which this PR fails to do. As written right now the end user would have to know the default schema name for each extension and add that to their pillar.

See https://www.postgresql.org/docs/current/static/sql-createextension.html:

schema_name

The name of the schema in which to install the extension's objects, given that the extension allows its contents to be relocated. The named schema must already exist. If not specified, and the extension's control file does not specify a schema either, the current default object creation schema is used.

If the extension specifies a schema parameter in its control file, then that schema cannot be overridden with a SCHEMA clause. Normally, an error will be raised if a SCHEMA clause is given and it conflicts with the extension's schema parameter. However, if the CASCADE clause is also given, then schema_name is ignored when it conflicts. The given schema_name will be used for installation of any needed extensions that do not specify schema in their control files.

Remember that the extension itself is not considered to be within any schema: extensions have unqualified names that must be unique database-wide. But objects belonging to the extension can be within schemas.

@YetAnotherMinion
Copy link
Contributor Author

The primary feature of this PR is to enable this formula to be used with saltstack Nitrogen, see #146.

@noelmcloughlin
Copy link
Contributor

noelmcloughlin commented Feb 6, 2018

Thanks @YetAnotherMinion I understand the intention now. This is not backwards compatible - but #146 needs to be fixed or formula breaks in Nitrogen.

On the other hand, it should be possible to omit the schema attribute for an extension, which this PR fails to do.

Yes, the PG docs say that, on omitting the schema attribute, the current default object creation schema is used. Should a future PR address this possibility? If yes, consider raising as issue.

@YetAnotherMinion
Copy link
Contributor Author

To clarify what is needed from me to merge this PR:

  • Make extension's schema field optional
  • Implement backwards compatibility? (The code will be more complicated, but I am pretty sure it will be easy to implement.)

Copy link
Contributor

@vutny vutny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work here @YetAnotherMinion !

Unfortunately, this PR has been outdated and now is going to be superseded with #223 implementing both support for per-db schema and extensions with preserving backward compatibility.

@noelmcloughlin @aboe76 This could be closed now.

@noelmcloughlin
Copy link
Contributor

Thanks @YetAnotherMinion for the work on this. Cheers @vutny @aboe76

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

Successfully merging this pull request may close these issues.

3 participants