Improve metadata bwc tests #352
Conversation
| c.execute("INSERT INTO doc.t1 (title, author, o) VALUES ('prefix_check', {\"dyn_empty_array\" = []}, {\"dyn_ignored_subcol\" = 'hello'})") | ||
|
|
||
| if int(path.from_version.split('.')[0]) >= 5: |
There was a problem hiding this comment.
constraint causes a syntax error and the create fails, iirc 4.6.
| @@ -148,8 +161,13 @@ def _test_rolling_upgrade(self, path, nodes): | |||
| # Run a query as a user created on an older version (ensure user is read correctly from cluster state, auth works, etc) | |||
| with connect(cluster.node().http_url, username='arthur', password='secret', error_trace=True) as custom_user_conn: | |||
| c = custom_user_conn.cursor() | |||
| wait_for_active_shards(c, expected_active_shards) | |||
| wait_for_active_shards(c) | |||
There was a problem hiding this comment.
Behaviour change starting 5.6. With deny dql on table doc.t1 to arthur, arthur cannot see t1 anymore -> cannot wait for t1 to be active anymore.
We could add version dependent behaviours or simply wait for all shards to be active.
There was a problem hiding this comment.
thx!
I know that the purpose is to test the metadata but what do you think about testing the constraint, the generated & default cols for their values as well?
Additionally, I think we should check the same cols on a partitioned table as well.
Please also check the test_upgrade to see if we have any duplicate testing, I see we also have users/privileges there and a parted table with a generated col.
|
Thanks for the pointers @matriv. I took a look at 3df0642 which does overlaps with my commit in terms of testing user persistence. But I think we need to move the test to rolling upgrades in order to also test that the privileges do continue to behave as expected in a mixed cluster. BTW, I did not parted table with generated cols in test_upgrade. I will add the same test for parted tables and also test the values, too. |
tests/bwc/test_rolling_upgrade.py
Outdated
| c.execute("refresh table t2") | ||
| c.execute(f"select a, b, c>=1 and c<=2, d>a+b from doc.t2 where a = {idx}") | ||
| self.assertEqual(c.fetchall(), [[idx, idx, True, True]]) | ||
| if int(path.from_version.split('.')[1]) >= 4: |
There was a problem hiding this comment.
if int(path.from_version.split('.')[1]) >= 4: is needed because there are a few bugs that are fixed in later releases so it cannot be merged with existing parted table and tested for version older than 5.4.
…st version after completing the upgrade
|
Hi @matriv I am trying cover everything mentioned except publications, subscriptions and FDW. Please check for updates. |
| old_version = '.'.join(map(str, node.version)) | ||
| c.execute("select distinct(version['created']) from information_schema.tables where table_name = 't2'") | ||
| self.assertEqual(c.fetchall(), [[old_version]]) | ||
| if int(path.from_version.split('.')[1]) >= 9: |
There was a problem hiding this comment.
After all nodes are upgraded, newly added partitions' versions follow the newer version, starting 5.9.
tests/bwc/test_rolling_upgrade.py
Outdated
| @@ -28,14 +30,6 @@ | |||
| ) | |||
|
|
|||
| ROLLING_UPGRADES_V5 = ( | |||
| UpgradePath('5.0.x', '5.1.x'), | |||
There was a problem hiding this comment.
LGTM. I'd kindly ask for @mfussenegger to take a look at it as well, though
Summary of the changes / Why this is an improvement
Originate from #341. Adds metadata rolling upgrade tests - privileges, views, default expressions, generated columns and constraints. Also partitioned table with aforementioned columns and table/partition versions.
Checklist