Move pg_dist_object to pg_catalog #5765
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thanodnl
force-pushed
the
move-pg-dist-object
branch
from
150f1c8
to
563ee98
Compare
thanodnl
force-pushed
the
move-pg-dist-object
branch
from
563ee98
to
959934c
Compare
thanodnl
commented
Mar 4, 2022
Comment on lines
+495
to
+505
| -- TODO: This test should be moved to a valid downgrade testing suite where the downgrade is done, both on the schema and the binaries. Later changes in Citus made a C vs Schema discrepancy error here | ||
| -- BEGIN; | ||
| -- SET citus.enable_metadata_sync TO on; | ||
| -- SELECT master_add_node('localhost', :master_port, groupId=>0); | ||
| -- CREATE TABLE citus_local_table (a int); | ||
| -- SELECT create_citus_local_table('citus_local_table'); | ||
| -- RESET citus.enable_metadata_sync; | ||
| -- | ||
| -- -- downgrade from 9.5-1 to 9.4-1 should fail as we have a citus local table | ||
| -- ALTER EXTENSION citus UPDATE TO '9.4-1'; | ||
| -- ROLLBACK; |
There was a problem hiding this comment.
This test runs into schema vs binary incompatibility.
Have disabled it for now, would be great to have more robust downgrade tests where binary and schema actually do conform.
marcocitus
approved these changes
Mar 4, 2022
| -- downgrade from 9.5-1 to 9.4-1 should fail as we have a citus local table | ||
| ALTER EXTENSION citus UPDATE TO '9.4-1'; | ||
| ROLLBACK; | ||
| -- TODO: This test should be moved to a valid downgrade testing suite where the downgrade is done, both on the schema and the binaries. Later changes in Citus made a C vs Schema discrepancy error here |
There was a problem hiding this comment.
It's probably also quite well-covered by the regression tests of previous releases and no longer that relevant.
hanefi
added a commit
that referenced
this pull request
Oct 6, 2022
Citus 11 moved `pg_dist_object` to `pg_catalog`. When doing a downgrade, for a brief period of time, Citus 10.2 expects to have this table in `citus` schema, but fails. This can potentially cause failures in downgrade. We now search for the relation in the `citus` schema and if that fails, we search in the `pg_catalog` schema. Related: #5765
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DESCRIPTION: Move pg_dist_object to pg_catalog
Historically
pg_dist_objecthad been created in thecitusschema as an experiment to understand if we could move our catalog tables to a branded schema. We quickly realised that this interfered with the UX on our managed services and other environments, where users connected via a user with the name ofcitus.By default postgres put the username on the search_path. To be able to read the catalog in the
citusschema we would need to grant access permissions to the schema. This caused newly created objects like tables etc, to default to this schema for creation. This failed due to the write permissions to that schema.With this change we move the
pg_dist_objectcatalog table to thepg_catalogschema, where our other schema's are also located. This makes the catalog table visible and readable by any user, like our other catalog tables, for debugging purposes.Note: due to the change of schema, we had to disable 1 test that was running into a discrepancy between the schema and binary. Secondly, we needed to make the lookup functions for the
pg_dist_objectrelation and their indexes less strict on the fallback of the naming due to an other test that, due to an unfortunate cache invalidation, needed to lookup the relation again. This makes that we won't default to only resolving frompg_catalogoutside of upgrades.