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

Inheritance of defaults #191

Closed olvidio closed 6 years ago

olvidio commented 6 years ago

I have some iuses with pyrseas: a) I use schemas and user with char '-' in name, so i have to enclose it into double quote. b) i use extensive inheritance inter schemas

I modified table.py and ini.py to correct some iuses, but:

basicly:

ini.py

line 82 (aprox) qualsch = qualsch.strip('"')

line 431 return "ALTER %s %s OWNER TO \"%s\"" % ( self.objtype, self.identifier(), owner or self.owner)

table.py

line 420:

def inhquery(): """reglass no put schema 'public' in name of parent. Generate new query: """ return """SELECT inhrelid::regclass AS sub, quote_ident(nspname) || '.' || quote_ident(relname) as parent, inhseqno FROM pg_inherits INNER JOIN pg_class ON(inhparent = pg_class.oid) INNER JOIN pg_namespace ON (pg_class.relnamespace = pg_namespace.oid) ORDER BY 1, 3"""

line 704:

add defaults in inherits cols (overwrite, i don't kown how to chek if exists)

        if incol.inherited and incol.default:
            stmts.append(base + "ALTER COLUMN %s SET DEFAULT %s" % (incol.name, incol.default))

table.py.txt init.py.txt

jmafc commented 6 years ago

With regard to the quotes in schemas, a month ago I committed a change to fix issue #186 which is very similar to your suggested change (uses a local undelim function that is more precise than str.strip and it applies it to both schemas and objects). The fix is available on both the master and r0.8 branches.

jmafc commented 6 years ago

As far as the second issue in __init__.py is concerned, I'll have to check and test your suggested fix.

jmafc commented 6 years ago

I'll take a look at the suggested changes to table.py later and let you know. I'll leave them as they are for now, but it's preferable (although it may be a bit more work on your side) to post each issue separately, unless they're closely related.

jmafc commented 6 years ago

Concerning the ALTER OWNER issue, I added the following test to tests/dbobject/test_owner.py:

@@ -103,3 +103,15 @@ class OwnerToSqlTestCase(InputMapToSqlTestCase):
             'owner': 'someuser'}})
         sql = self.to_sql(inmap, [CREATE_TABLE])
         assert sql[0] == "ALTER TABLE sd.t1 OWNER TO someuser"
+
+    def test_change_table_owner_delim(self):
+        "Change the owner of a table with delimited identifiers"
+        inmap = self.std_map()
+        inmap.update({'schema a-schema': {'table a-table': {
+            'columns': [{'c1': {'type': 'integer'}}, {'c2': {'type': 'text'}}],
+            'owner': 'someuser'}}})
+        sql = self.to_sql(inmap, ["CREATE SCHEMA \"a-schema\"",
+                                  "CREATE TABLE \"a-schema\".\"a-table\" ("
+                                  "c1 integer, c2 text)"])
+        assert sql[0] == "ALTER TABLE \"a-schema\".\"a-table\" OWNER TO " \
+                          "someuser"

The test passes without any change to pyrseas/dbobject/__init__.py. Therefore, I don't see any need to take further action (but I'll submit the test since it's useful).

jmafc commented 6 years ago

I have also added two tests to tests/dbobject/test_table.py based on the existing test_map_inherit and test_table_inheritance but using delimited identifiers. The tests pass without any changes to Table.inhquery. I suspect this is due to the undelim function in split_schema_obj which handles both schema and object names, rather than only your suggested change which only did a strip on the qualsch variable.

olvidio commented 6 years ago

OK. Thanks. I close this issue and re-open separate concepts

jmafc commented 6 years ago

OK, but the only issue which I consider as open is the very last one, i.e., the one about column defaults in inherited columns, for which I still have to build a test case. I could reopen if necessary (and change its title).

jmafc commented 6 years ago

@olvidio I'm considering reopening this issue to deal with the inherited defaults, but I need some feedback. Please take a look at the test starting at https://github.com/perseas/Pyrseas/blob/8b787fe51ed7e226dc3805bf19c2e67b0a9e895a/tests/dbobject/test_table.py#L404 In the test, column c1 is created in table t1 and inherited in table t2, simply as an integer. If I were to add, for example, NOT NULL and DEFAULT 1 to the column, by using ALTER TABLE on t1, t2 would automatically inherit those properties. In other words, I don't think it's necessary to issue an ALTER TABLE t2 SET DEFAULT 1, as it seems you're suggesting in your proposed change.