-
Notifications
You must be signed in to change notification settings - Fork 753
Harden columnar public views against cross-session temp relations #8252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
m3hm3t
wants to merge
11
commits into
main
Choose a base branch
from
m3hm3t/pg18_columnar_access_temp_err
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3c9f179
Pg18 beta conf file updated
m3hm3t 9ac3064
Stabilize multi_insert_select expected: accept unqualified columns in…
m3hm3t a827d7e
bump pg versions
m3hm3t 9c303df
Postgres 18: Fix regress tests caused by GROUP RTE. (#8206)
colm-mchugh a1be689
add pg 18.0
m3hm3t d1f2ce4
Removes citus-tools submodule; add pg15-pg18 upgrade test
naisila 356f254
Revert multi_insert_select_0.out addition (#8232)
naisila 041ae15
Pg18 beta conf file updated
m3hm3t 2e4cb36
Add CREATE VIEW statement for columnar.storage with security barrier
m3hm3t f776678
Enhance columnar_relation_storageid to handle temp tables in PG18+
m3hm3t 5fd40f9
Remove citus-tools submodule
m3hm3t File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
42 changes: 42 additions & 0 deletions
42
src/backend/columnar/sql/citus_columnar--13.2-1--14.0-1.sql
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,44 @@ | ||
| -- citus_columnar--13.2-1--14.0-1 | ||
| -- bump version to 14.0-1 | ||
|
|
||
| CREATE OR REPLACE VIEW columnar.storage WITH (security_barrier) AS | ||
| SELECT c.oid::regclass AS relation, | ||
| columnar.get_storage_id(c.oid) AS storage_id | ||
| FROM pg_catalog.pg_class c | ||
| JOIN pg_catalog.pg_am am ON c.relam = am.oid | ||
| WHERE am.amname = 'columnar' | ||
| -- exclude other sessions' temp rels, but keep *my* temp tables | ||
| AND (c.relpersistence <> 't' | ||
| OR c.relnamespace = pg_catalog.pg_my_temp_schema()) | ||
| AND pg_catalog.pg_has_role(c.relowner, 'USAGE'); | ||
| COMMENT ON VIEW columnar.storage IS 'Columnar relation ID to storage ID mapping.'; | ||
| GRANT SELECT ON columnar.storage TO PUBLIC; | ||
|
|
||
| -- re-emit dependent views with OR REPLACE so they stay bound cleanly | ||
| CREATE OR REPLACE VIEW columnar.stripe WITH (security_barrier) AS | ||
| SELECT relation, storage.storage_id, stripe_num, file_offset, data_length, | ||
| column_count, chunk_row_count, row_count, chunk_group_count, first_row_number | ||
| FROM columnar_internal.stripe stripe, columnar.storage storage | ||
| WHERE stripe.storage_id = storage.storage_id; | ||
| COMMENT ON VIEW columnar.stripe | ||
| IS 'Columnar stripe information for tables on which the current user has ownership privileges.'; | ||
| GRANT SELECT ON columnar.stripe TO PUBLIC; | ||
|
|
||
| CREATE OR REPLACE VIEW columnar.chunk_group WITH (security_barrier) AS | ||
| SELECT relation, storage.storage_id, stripe_num, chunk_group_num, row_count | ||
| FROM columnar_internal.chunk_group cg, columnar.storage storage | ||
| WHERE cg.storage_id = storage.storage_id; | ||
| COMMENT ON VIEW columnar.chunk_group | ||
| IS 'Columnar chunk group information for tables on which the current user has ownership privileges.'; | ||
| GRANT SELECT ON columnar.chunk_group TO PUBLIC; | ||
|
|
||
| CREATE OR REPLACE VIEW columnar.chunk WITH (security_barrier) AS | ||
| SELECT relation, storage.storage_id, stripe_num, attr_num, chunk_group_num, | ||
| minimum_value, maximum_value, value_stream_offset, value_stream_length, | ||
| exists_stream_offset, exists_stream_length, value_compression_type, | ||
| value_compression_level, value_decompressed_length, value_count | ||
| FROM columnar_internal.chunk chunk, columnar.storage storage | ||
| WHERE chunk.storage_id = storage.storage_id; | ||
| COMMENT ON VIEW columnar.chunk | ||
| IS 'Columnar chunk information for tables on which the current user has ownership privileges.'; | ||
| GRANT SELECT ON columnar.chunk TO PUBLIC; | ||
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder should create older versions of these views in downgrade path as we usually do, or, as we did in some of the similar bugfixes, should those stay as is even after a downgrade?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose if we keep this view, we’ll only “lose” rows that came from other sessions’ temp columnar tables which nobody should rely on, and which is exactly what bit on PG18.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, I can add a downgrade script to this PR and open a separate issue to decide later whether to keep or remove it.
What do you think on this @onurctirtir ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after discuss will add downgrade path