Skip to content
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

Block cross-db 'SELECT ... INTO table FROM ...' statement #2744

Conversation

HarshLunagariya
Copy link
Contributor

@HarshLunagariya HarshLunagariya commented Jul 16, 2024

Description

Before this commit, Cross-db 'SELECT ... INTO table FROM ...' was allowed to be executed unintentionally since it was missed to be blocked when cross-db DMLs got supported. Due to this unblocked behaviour, It creates a table whose owner is current session's login since internally cross-db statement execution sets current role to current session's login.

This commit blocks cross-db 'SELECT ... INTO table FROM ...' statement by adding relevant check on ANTLR parser.

Task: BABEL-4934
Signed-off-by: Harsh Lunagariya [email protected]

Issues Resolved

BABEL-4934

Test Scenarios Covered

  • Use case based -
    SELECT INTO query with table within same-db/cross-db
    SELECT INTO temp table query

  • Boundary conditions -
    NA

  • Arbitrary inputs -
    NA

  • Negative test cases -
    SELECT INTO query with table within same-db

  • Minor version upgrade tests -
    NA

  • Major version upgrade tests -
    NA

  • Performance tests -
    NA

  • Tooling impact -
    NA

  • Client tests -

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.

HarshLunagariya and others added 2 commits July 16, 2024 09:41
Before this commit, Cross-db 'SELECT ... INTO table FROM ...' was
allowed to be executed unintentionally since it was missed to be blocked
when cross-db DMLs got supported. Due to this unblocked behaviour, It
creates a table whose owner is current session's login since internally
cross-db statement execution sets current role to current session's
login.

This commit blocks cross-db 'SELECT ... INTO table FROM ...' statement
by adding relavant check on ANTLR parser.

Signed-off-by: Harsh Lunagariya <[email protected]>
@coveralls
Copy link
Collaborator

coveralls commented Jul 16, 2024

Pull Request Test Coverage Report for Build 10056627748

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • 188 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.008%) to 73.218%

Files with Coverage Reduction New Missed Lines %
contrib/babelfishpg_tsql/src/pltsql_bulkcopy.c 23 88.33%
contrib/babelfishpg_tsql/src/hooks.c 165 79.69%
Totals Coverage Status
Change from base Build 9952949997: 0.008%
Covered Lines: 42905
Relevant Lines: 58599

💛 - Coveralls

forestkeeper
forestkeeper previously approved these changes Jul 18, 2024
Copy link
Contributor

@forestkeeper forestkeeper left a comment

Choose a reason for hiding this comment

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

LGTM

{
throw PGErrorWrapperException(ERROR,
ERRCODE_FEATURE_NOT_SUPPORTED,
"cross-db 'SELECT INTO' statement is not supported in Babelfish",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should we suggest a alternative in error hint?

Copy link
Contributor

Choose a reason for hiding this comment

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

error hint will only be logged in logfile. not sure how will it help in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Included workaround in comment message itself as suggested by Alex.

Deepesh125
Deepesh125 previously approved these changes Jul 19, 2024
Signed-off-by: Harsh Lunagariya <[email protected]>
@HarshLunagariya HarshLunagariya dismissed stale reviews from Deepesh125 and forestkeeper via dfbc593 July 23, 2024 09:18
Signed-off-by: Harsh Lunagariya <[email protected]>
@shardgupta shardgupta merged commit 1570cc6 into babelfish-for-postgresql:BABEL_4_X_DEV Jul 24, 2024
43 checks passed
@shardgupta shardgupta deleted the babel_4934_sel_into branch July 24, 2024 06:43
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jul 25, 2024
…for-postgresql#2744)

Before this commit, Cross-db 'SELECT ... INTO table FROM ...' was allowed to be executed unintentionally since it was missed to be blocked when cross-db DMLs got supported. Due to this unblocked behaviour, It creates a table whose owner is current session's login since internally cross-db statement execution sets current role to current session's login.

This commit blocks cross-db 'SELECT ... INTO table FROM ...' statement by adding relevant check on ANTLR parser.

Task: BABEL-4934

Signed-off-by: Harsh Lunagariya [email protected]
shardgupta pushed a commit that referenced this pull request Jul 31, 2024
)

Before this commit, Cross-db 'SELECT ... INTO table FROM ...' was allowed to be executed unintentionally since it was missed to be blocked when cross-db DMLs got supported. Due to this unblocked behaviour, It creates a table whose owner is current session's login since internally cross-db statement execution sets current role to current session's login.

This commit blocks cross-db 'SELECT ... INTO table FROM ...' statement by adding relevant check on ANTLR parser.

Task: BABEL-4934

Signed-off-by: Harsh Lunagariya [email protected]
sharathbp pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Aug 20, 2024
…for-postgresql#2744)

Before this commit, Cross-db 'SELECT ... INTO table FROM ...' was allowed to be executed unintentionally since it was missed to be blocked when cross-db DMLs got supported. Due to this unblocked behaviour, It creates a table whose owner is current session's login since internally cross-db statement execution sets current role to current session's login.

This commit blocks cross-db 'SELECT ... INTO table FROM ...' statement by adding relevant check on ANTLR parser.

Task: BABEL-4934

Signed-off-by: Harsh Lunagariya [email protected]
staticlibs pushed a commit to wiltondb/babelfish_extensions that referenced this pull request Oct 20, 2024
…for-postgresql#2744) (babelfish-for-postgresql#2780)

Before this commit, Cross-db 'SELECT ... INTO table FROM ...' was allowed to be executed unintentionally since it was missed to be blocked when cross-db DMLs got supported. Due to this unblocked behaviour, It creates a table whose owner is current session's login since internally cross-db statement execution sets current role to current session's login.

This commit blocks cross-db 'SELECT ... INTO table FROM ...' statement by adding relevant check on ANTLR parser.

Task: BABEL-4934

Signed-off-by: Harsh Lunagariya [email protected]
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
anju15bharti pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 13, 2024
shardgupta pushed a commit that referenced this pull request Nov 13, 2024
)" (#3109)

This reverts commit 1570cc6.

Task: BABEL-4934 

Co-authored-by: ANJU BHARTI <[email protected]>
shardgupta pushed a commit that referenced this pull request Nov 13, 2024
)" (#3112)

This reverts commit 1570cc6.

Task: BABEL-4934 

Co-authored-by: ANJU BHARTI <[email protected]>
shardgupta pushed a commit that referenced this pull request Nov 13, 2024
)" (#3111)

This reverts commit 1570cc6.

Task: BABEL-4934 

Co-authored-by: ANJU BHARTI <[email protected]>
shardgupta pushed a commit that referenced this pull request Nov 13, 2024
) (#2780)" (#3114)

This reverts commit 0b401ee.
Task: BABEL-4934
Signed-off-by: ANJU BHARTI <[email protected]>
shardgupta pushed a commit that referenced this pull request Nov 13, 2024
) (#2780)" (#3113)

This reverts commit 0b401ee.
Task: BABEL-4934
Signed-off-by: ANJU BHARTI <[email protected]>
shardgupta pushed a commit that referenced this pull request Nov 13, 2024
) (#2780)" (#3110)

This reverts commit 1570cc6.

Task: BABEL-4934 

Signed-off-by: ANJU BHARTI <[email protected]>
roshan0708 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 20, 2024
roshan0708 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 20, 2024
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 25, 2024
shardgupta pushed a commit that referenced this pull request Nov 25, 2024
)" (#3109)

This reverts commit 1570cc6.

Task: BABEL-4934 

Co-authored-by: ANJU BHARTI <[email protected]>
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.

7 participants