perseas / Pyrseas

Provides utilities for Postgres database schema versioning.
https://perseas.github.io/
BSD 3-Clause "New" or "Revised" License
395 stars 67 forks source link

plv8 to extension and fix reserved words in the primary key #252

Closed albrov closed 1 year ago

albrov commented 1 year ago

Hi.

  1. Please add plv8 to KNOWN_LANGS (file extension.py)

  2. A patch to fix the unloading of a table with reserved words in the primary key


database.py 
437c437
<     def to_map(self):
---
>     def to_map(self,quote_reserved=True):
477a478,479
>         if quote_reserved:
>             fetch_reserved_words(self.dbconn)

function data_export
dbobject/table.py 
740c740
<             order_by = [self.columns[col - 1].name
---
>             order_by = [quote_id(self.columns[col - 1].name) ``

For such trifles, I see no reason to fork

Thank you
jmafc commented 1 year ago

Regarding the first point: KNOWN_LANGS is a replacement for the pg_pltemplate catalog which was dropped in PG 13. The catalog only listed PL's that were available in the core distribution. As per https://www.postgresql.org/docs/current/external-pl.html, PL/v8 is still an externally-maintained language, so I don't think we should add it to KNOWN_LANGS. If you have PL/v8 installed, it will be handled as an extension. If that is causing some issues, please describe them so that they can be looked into.

albrov commented 1 year ago

To repeat the problem:
test database

CREATE EXTENSION IF NOT EXISTS plv8;

CREATE FUNCTION public.test() RETURNS integer
    LANGUAGE plv8
    AS $$return 1$$;

dbtoyaml

dbtoyaml -o test.yml  test

test.yml

extension plpgsql:
  description: PL/pgSQL procedural language
  owner: postgres
  schema: pg_catalog
  version: '1.0'
extension plv8:
  description: PL/JavaScript (v8) trusted procedural language
  owner: billuser
  schema: pg_catalog
  version: 3.0alpha
schema public:
  description: standard public schema
  function test():
    language: plv8
    owner: billuser
    returns: integer
    source: |-
      return 1
  owner: postgres
  privileges:
  - PUBLIC:
    - all
  - postgres:
    - all

create new database test_new (does not contain plv8 extension) and yamltodb

yamltodb  -1 -o create.sql  test_new test.yml

error

Traceback (most recent call last):
  File "/usr/local/bin/yamltodb", line 33, in <module>
    sys.exit(load_entry_point('Pyrseas==0.10.0', 'console_scripts', 'yamltodb')())
  File "/usr/local/lib/python3.9/dist-packages/Pyrseas-0.10.0-py3.9.egg/pyrseas/yamltodb.py", line 49, in main
    stmts = db.diff_map(inmap)
  File "/usr/local/lib/python3.9/dist-packages/Pyrseas-0.10.0-py3.9.egg/pyrseas/database.py", line 526, in diff_map
    self.from_map(input_map)
  File "/usr/local/lib/python3.9/dist-packages/Pyrseas-0.10.0-py3.9.egg/pyrseas/database.py", line 397, in from_map
    self._link_refs(self.ndb)
  File "/usr/local/lib/python3.9/dist-packages/Pyrseas-0.10.0-py3.9.egg/pyrseas/database.py", line 218, in _link_refs
    db.languages.link_refs(db.functions, langs)
  File "/usr/local/lib/python3.9/dist-packages/Pyrseas-0.10.0-py3.9.egg/pyrseas/dbobject/language.py", line 143, in link_refs
    raise exc
  File "/usr/local/lib/python3.9/dist-packages/Pyrseas-0.10.0-py3.9.egg/pyrseas/dbobject/language.py", line 139, in link_refs
    language = self[(func.language)]
KeyError: 'plv8'
jmafc commented 1 year ago

I've managed to reproduce the problem using plr (which is available as a package on Debian). This appears to be another one of those "special-casing of extensions" problems we recently discussed in https://github.com/perseas/Pyrseas/discussions/248. Have to dig deeper.

jmafc commented 1 year ago

I've been experimenting with some ways to deal with externally managed languages. Part of the problem stems from what @dvarrazzo has pointed out elsewhere various times, i.e., the special-case extension stuff in catalog queries. In this case, in dbobject/language.py the query has the following clause:

 AND l.oid NOT IN (
                  SELECT objid FROM pg_depend WHERE deptype = 'e'
                               AND classid = 'pg_language'::regclass)

If I remove that, in a database where both plpgsql and plr are present, dbtoyaml prints both extensions and both languages in the YAML output. I believe we need to print the language plr section in order for the inbound mapping to work properly. Otherwise, yamltodb has no way of knowing that a particular external language needs to be present in the target (unless, of course, we parse extension <lang> and we maintain a special list of externally maintained languages that we're willing to support).

So the interim plan is as follows (until we do something more comprehensive):

  1. Remove the special-case clause shown above.
  2. On the to_map side, exclude the core maintained languages from the YAML output, most likely by using the already defined KNOWN_LANGS, while allowing the externally maintained languages to be shown.
  3. On the to_sql side, the presence of a language x on the input YAML already allows a function in that language to be processed properly. However, it also causes a CREATE LANGUAGE x to be emitted and that will have to be suppressed. Daniele, your review/comments will be appreciated.
jmafc commented 1 year ago

Ok, I have made the changes discussed above and I'll be submitting the fix shortly. @albrov please test with plv8 (I tested with plr because it was more convenient for me). @dvarrazzo I've removed tests/dbobject/test_language.py because the only thing left was a test (which you created) for bug #103 for plpython3u which as I see it is no longer in use. I've also essentially nulled-out the code for Language.create (and eliminated the special case in Language.drop) because CREATE/DROP LANGUAGE should all be occurring as a result of CREATE/DROP EXTENSION, regardless of whether they're core- or externally maintained.