Skip to content

Commit

Permalink
Use list of languages in pg_pltemplate in Database._link_refs. Fixes #…
Browse files Browse the repository at this point in the history 
…103.

 * pyrseas/database.py: If running in PG 9.1+, select the list of
   pg_pltemplate languages and pass it to the languages link_refs
   method.
 * pyrseas/dbobject/language.py: Use the list of languages and not just
   plpgsql.
 * tests/dbobject/test_function.py: New test, courtesy of DVarrazzo.
  • Loading branch information
@jmafc
jmafc committed Sep 17, 2014
1 parent 540ea78 commit 692ef0c
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
5 changes: 4 additions & 1 deletion pyrseas/database.py
View file
Expand Up @@ -124,7 +124,10 @@ def __init__(self, config):

def _link_refs(self, db):
"""Link related objects"""
db.languages.link_refs(db.functions)
if self.dbconn.version >= 90100:
langs = [lang[0] for lang in self.dbconn.fetchall(
"SELECT tmplname FROM pg_pltemplate")]

This comment has been minimized.

Sign in to view

Copy link
@dvarrazzo

dvarrazzo Sep 17, 2014

Contributor

Joe, is this pg_pltemplate filtering doing the right thing? Because it seems the source of all the problems. I mean, ISTM you added this check to work around the problems with the plv8 language, but these had to be solved by checking its belonging to an exception. Just wanted to know because I'll end up hacking on that table later on, when working at the dependencies between extensions and languages.

Anyway, once I'm back hacking on pyrseas (probably later) I'll see if master is working now with my testing db (i.e. if there's no regression compared to 0.7.1). If it works I'll try to rebase the deptrack branch on master.

This comment has been minimized.

Sign in to view

Copy link
@jmafc

jmafc Sep 17, 2014

Author Member

In so far as plplythonu and this issue are concerned, I believe it's correct. This whole languages/extensions problems are complicated because of various issues: (a) in 9.1 extensions were introduced and PLs were "converted" into extensions, (b) there are three kinds of languages: internal ones such as C, SQL and 'internal', "core" languages such as plpgsql (which is also special case because it gets installed automatically), plperl and plpythonu, which are listed in pg_pltemplates so that they can still be created simply with CREATE LANGUAGE as oppposed to CREATE EXTENSION, and external languages like plv8 that can only be created as EXTENSIONs.

This comment has been minimized.

Sign in to view

Copy link
@dvarrazzo

dvarrazzo Sep 18, 2014

Contributor

In so far as plplythonu and this issue are concerned, I believe it's correct.

I think your mental model of the languages is slightly off and this commit doesn't fix the issue. I've tried to diff a dump of my production database with 0.7.1 and with master. The quotes bug and index bugs are solved but now the database.yaml file is not produced (that's because in 0.7.1 it should only contain language plpythonu, which you are discarding) and as a consequence yaml2db doesn't generate the CREATE LANGUAGE plpgsqlu required by the following objects.

What is wrong in your assumptions is:

This whole languages/extensions problems are complicated because of various issues: (a) in 9.1 extensions were introduced and PLs were "converted" into extensions

This is not an issue: if you CREATE EXTENSION lang it will create the language. In 9.1 extensions wrap the languages, don't replace it. A language is not different in any aspect from any other object that belongs to an extension, and you should follow the normal approach as for other objects.

(b) there are three kinds of languages: internal ones such as C, SQL and 'internal',

right: these are the only three core language and the thing you can compare to builtin types such as int, but...

"core" languages such as plpgsql (which is also special case because it gets installed automatically), plperl and plpythonu, which are listed in pg_pltemplates so that they can still be created simply with CREATE LANGUAGE as oppposed to CREATE EXTENSION,

no: this category doesn't exist. plpgsql is no different from plpythonu and both are no different from pl/lolcode for what Pyrseas is concerned (that's because Pyrseas can only handle languages that are already installed in the cluster: calling pgxnclient is not its task). The table pl_pgtemplates only states the defaults for the function handlers; if the records weren't there you should specify the various handler, validator, etc clauses in CREATE LANGUAGE. That's it, that's the only bloody use of pl_pgtemplates. PL/PgSQL is not different from any other language except that it is installed in the template1 database so that, if you don't modify the template (which you can), when you CREATE DATABASE it gets installed by default. But apart from that the language is a normal object in the database: \dL shows you the language and on PG 9.1 \dx shows the extension too: they can be removed from a database any moment with a DROP.

and external languages like plv8 that can only be created as EXTENSIONs.

No: plv8 is normally packaged as an extension but it can probably be installed without it: extensions are only a superstrutcture. I can make a language without extension no problem, as it was the way before 9.1.

In branch 0.7, any language which is not C, sql or internal must be dumped by dbtoyaml, or it is a bug. You must dump plpgsql too, because it's no problem dropping it from a database, and if it's not on the target db you want to create it. From 9.1 you may detect that the language belongs to an extension and avoid dumping it, but then you must dump the extension. If you don't do these two things, bug #103 is still open.

On the opposite end, handling the language must be done the normal way: if the yaml contains an extension and the database not, create it. Normally you will find plpgsql both in the yaml and in the target database and you don't need doing anything. If the yaml contains a language and the target database not, create that language, otherwise the sql fails loading. Probably you won't have to create plpgsql but this is not a-priory knowledge: it must be inferred by the fact the language or the extensions are already on the target db so they don't need re-creating.

Please read the documentation of pg_pltemplate: you are using it wrong. It only provides models to avoid specifing stupid HANDLER and VALIDATOR on CREATE LANGUAGE but it doesn't make the language any special. You may decide to avoid dumping HANDLER etc in yaml if they match the record in pg_pltemplate but that's it, uou still have to create the language so you still need to store it in yaml.

This comment has been minimized.

Sign in to view

Copy link
@jmafc

jmafc Sep 18, 2014

Author Member

Thanks for setting me straight. However, in a sense, plpgsql, plperl[u], pltcl[u] and plpython[23]u are core languages because they're the only ones that are distributed and documented with the core product. Nevertheless, I see your point and will work to address the problems you highlighted.
db.languages.link_refs(db.functions, langs)
copycfg = {}
if 'datacopy' in self.config:
copycfg = self.config['datacopy']
Expand Down
4 changes: 2 additions & 2 deletions pyrseas/dbobject/language.py
View file
Expand Up @@ -92,7 +92,7 @@ def from_map(self, inmap):
if 'oldname' in inlanguage:
del inlanguage['oldname']

def link_refs(self, dbfunctions):
def link_refs(self, dbfunctions, langs):
"""Connect functions to their respective languages
:param dbfunctions: dictionary of functions
Expand All @@ -108,7 +108,7 @@ def link_refs(self, dbfunctions):
try:
language = self[(func.language)]
except KeyError as exc:
if func.language == 'plpgsql':
if func.language in langs:
continue
raise exc
if isinstance(func, Function):
Expand Down
10 changes: 10 additions & 0 deletions tests/dbobject/test_function.py
View file
Expand Up @@ -2,6 +2,7 @@
"""Test functions"""

import pytest
import psycopg2

from pyrseas.testutils import DatabaseToMapTestCase
from pyrseas.testutils import InputMapToSqlTestCase, fix_indent
Expand Down Expand Up @@ -124,6 +125,15 @@ def test_map_function_leakproof(self):
assert dbmap['schema public']['function f1(integer, integer)'] == \
expmap

def test_bug_103(self):
"Test a function created with language other than plpgsql or plperl"
try:
self.to_map(["CREATE OR REPLACE LANGUAGE plpythonu"])
except psycopg2.OperationalError as e:
self.skipTest("plpython installation failed: %s" % e)
self.to_map(["CREATE FUNCTION test103() RETURNS int AS "
"'return 1' LANGUAGE plpythonu"])


class FunctionToSqlTestCase(InputMapToSqlTestCase):
"""Test SQL generation from input functions"""
Expand Down

0 comments on commit 692ef0c

Please sign in to comment.