Skip to content

Avoid checking permission of Babelfish temp tables on parallel worker#3644

Merged
Deepesh125 merged 17 commits intobabelfish-for-postgresql:BABEL_5_X_DEVfrom
amazon-aurora:jira-babel-5703-3
Apr 10, 2025
Merged

Avoid checking permission of Babelfish temp tables on parallel worker#3644
Deepesh125 merged 17 commits intobabelfish-for-postgresql:BABEL_5_X_DEVfrom
amazon-aurora:jira-babel-5703-3

Conversation

@Deepesh125
Copy link
Copy Markdown
Contributor

@Deepesh125 Deepesh125 commented Apr 1, 2025

Description

Consider following facts,

Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel worker
tries to check permissions on Babelfish then it will fail.

Any user should be able to access Babelfish temp tables under given session.

Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set of
temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Issues Resolved

BABEL-5703

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@Deepesh125 Deepesh125 changed the title Empty-Commit Avoid checking permission of Babelfish temp tables on parallel worker Apr 4, 2025
CREATE LOGIN l1 WITH PASSWORD = '12345678'
GO

CREATE USER l1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should have a test where
accessing temp table inside parallel worker should give error
accessing temp table as outer scan should not give an error
accessing PG temp table inside parallel worker should give error
accessing PG temp table as outer scan should not give an error (but this will not skip the check, rather check will pass)
accessing PG temp table as outer scan should give an error (due to perm issue)
accessing PG regular table in both case should always give an error
mix of all three tables in same query

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

CREATE USER l1
GO

CREATE TABLE pq_t (id INT)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear if all test cases are covered, can you put one line comment for each test to describe what use case it covers like
-- Skip permission check for temp table in BBF on parallel worker
-- Skip permission check for PG temp table in BBF on parallel worker
-- Permission check error for PG temp table without parallel worker and so on

kuntalghosh
kuntalghosh previously approved these changes Apr 9, 2025
@Deepesh125 Deepesh125 merged commit 6da7ff0 into babelfish-for-postgresql:BABEL_5_X_DEV Apr 10, 2025
47 checks passed
@Deepesh125 Deepesh125 deleted the jira-babel-5703-3 branch April 10, 2025 03:42
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 10, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 10, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 10, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 10, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 10, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 10, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 10, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Apr 14, 2025
…babelfish-for-postgresql#3644)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.
2. Any user should be able to access Babelfish temp tables under given session.
3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader
does required permission check on other tables. This commits achieves this behaviour by implementating following three
hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check relation
if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Deepesh125 added a commit that referenced this pull request Apr 14, 2025
…#3644) (#3671)

Consider following facts,

1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel
worker tries to check permissions on Babelfish then it will fail.

2. Any user should be able to access Babelfish temp tables under given session.

3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error.

Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that
leader does required permission check on other tables. This commits achieves this behaviour by implementing
following three hooks,

bbf_ParallelQueryMain_hook -- implements ParallelQueryMain_hook. It constructs Bitmapset (list of temp table oids)
by looping through es_range_table checking if given relation is temp table, checks permission if it is and add it to the
set. It will then pass this set of temp table defined under current with Parallel workers.

bbf_ExecInitParallelPlan -- implements ExecInitParallelPlan_hook. Parallel workers will gather additional details (oid set
of temp table) passed by Leader node. This set will later be used to avoid checking permissions on temp tables.

bbf_ExecCheckOneRelPerms_hook -- implements ExecCheckOneRelPerms_hook. It will avoid permission check
relation if relid is part of the set of oids constructed using bbf_ExecInitParallelPlan.

Task: BABEL-5703
Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>
(cherry picked from commit 6da7ff0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants