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.

Sign up for GitHub

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

Jump to bottom

Add support for ENABLE and DISABLE on ALTER TABLE for pg #1077

Merged
merged 4 commits into from
Dec 31, 2023
Merged

Add support for ENABLE and DISABLE on ALTER TABLE for pg #1077

merged 4 commits into from
Dec 31, 2023

Conversation

tobyhede
Copy link
Contributor

Add support for the following pg syntax

# enable
ALTER TABLE tab ENABLE ALWAYS RULE rule_name
ALTER TABLE tab ENABLE ALWAYS TRIGGER trigger_name
ALTER TABLE tab ENABLE REPLICA TRIGGER trigger_name
ALTER TABLE tab ENABLE REPLICA RULE rule_name
ALTER TABLE tab ENABLE ROW LEVEL SECURITY
ALTER TABLE tab ENABLE RULE rule_name
ALTER TABLE tab ENABLE TRIGGER ALL
ALTER TABLE tab ENABLE TRIGGER USER
ALTER TABLE tab ENABLE TRIGGER trigger_name

# disable
ALTER TABLE tab DISABLE ROW LEVEL SECURITY
ALTER TABLE tab DISABLE RULE rule_name
ALTER TABLE tab DISABLE TRIGGER ALL
ALTER TABLE tab DISABLE TRIGGER USER
ALTER TABLE tab DISABLE TRIGGER trigger_name

@coveralls
Copy link

coveralls commented Dec 30, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 7362770487

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 87.839%
Totals Coverage Status
Change from base Build 7314541195: 0.1%
Covered Lines: 18447
Relevant Lines: 21001
💛 - Coveralls

@tobyhede tobyhede force-pushed the pg-alter-table-enable-disable branch 2 times, most recently from 0f43971 to d663b19 Compare December 30, 2023 06:41
@tobyhede
Alphabetise
5b7bd43
@tobyhede
Fix clippy issues
fc35013 
Signed-off-by: Toby Hede <toby@cipherstash.com>
@tobyhede tobyhede force-pushed the pg-alter-table-enable-disable branch from d663b19 to fc35013 Compare December 30, 2023 07:17
tobyhede
tobyhede commented Dec 30, 2023
View reviewed changes
src/tokenizer.rs
chars,
|ch| matches!(ch, '0'..='9' | 'A'..='F' | 'a'..='f'),
);
let s2 = peeking_take_while(chars, |ch| ch.is_ascii_hexdigit());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New clippy rule dropped.

tobyhede
tobyhede commented Dec 30, 2023
View reviewed changes
src/tokenizer.rs
@@ -1077,7 +1074,7 @@ impl<'a> Tokenizer<'a> {
match chars.peek() {
Some('$') => {
chars.next();
for (_, c) in value.chars().enumerate() {
for c in value.chars() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New clippy rule dropped.

@alamb alamb mentioned this pull request Dec 31, 2023
Closed
7 tasks
alamb
alamb approved these changes Dec 31, 2023
View reviewed changes
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks good to me -- thank you for the contribution @tobyhede

src/dialect/mod.rs
@@ -349,6 +349,7 @@ mod tests {
}
}

#[allow(clippy::needless_raw_string_hashes)]
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can fix this in a follow on PR (as in remove the clippy allow and then fix the tests to conform to the new clippy opinions)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, is a good idea. I tried to fix, but without the raw the parser errors with Unterminated string literal.
I wasn't sure enough of the underlying intent to make more a dramatic change.

@alamb alamb merged commit a75778c into apache:main Dec 31, 2023
10 checks passed
@tobyhede tobyhede deleted the pg-alter-table-enable-disable branch August 27, 2024 05:09
@tobyhede tobyhede restored the pg-alter-table-enable-disable branch August 27, 2024 05:10
@tobyhede tobyhede deleted the pg-alter-table-enable-disable branch August 27, 2024 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alamb alamb alamb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tobyhede @coveralls @alamb