Skip to content

Add ALTER TYPE statements for postgres#42

Merged
tyt2y3 merged 11 commits intoSeaQL:postgres-typesfrom
MozarellaMan:master
May 4, 2021
Merged

Add ALTER TYPE statements for postgres#42
tyt2y3 merged 11 commits intoSeaQL:postgres-typesfrom
MozarellaMan:master

Conversation

@MozarellaMan
Copy link
Copy Markdown
Contributor

For #38

This PR isn't ready. This is a draft so I can check if I am doing things right.

I have implemented ADD VALUE and RENAME TO so far. Before I start on OWNER, SET SCHEMA, ADD ATTRIBUTE, and the RESTRICT and CASCADE options - I wanted to some feedback to see if I'm on the right track with the builder and sql generation code.

If everything is fine I will continue with the PR and add documentation, if I'm doing anything wrong or there is a better way let me know!

@tyt2y3
Copy link
Copy Markdown
Member

tyt2y3 commented May 3, 2021

Thank you for the PR, nicely done! If you intend to format an existing piece of code, please do so in a standalone Format code commit, such that feature additions will not be mixed together.

@MozarellaMan
Copy link
Copy Markdown
Contributor Author

Thank you for the PR, nicely done! If you intend to format an existing piece of code, please do so in a standalone Format code commit, such that feature additions will not be mixed together.

Yep, that makes sense. Will avoid just running cargo fmt in future. I've added the RENAME VALUE statement as well. I just realised I accidentally PR'd from my fork's master instead of the add_alter_type feature branch, sorry about that!

@tyt2y3
Copy link
Copy Markdown
Member

tyt2y3 commented May 3, 2021

---- src/extension/postgres/types.rs - extension::postgres::types::TypeAlterStatement::name (line 308) stdout ----
Test executable failed (exit code 101).

stderr:
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"ALTER TYPE \"font_family\" ADD VALUE \'cursive\'"`,
 right: `"ALTER TYPE \"font_family\" ADD VALUE \'cursive\' "`', src/extension/postgres/types.rs:24:1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The test fails because of the extra space at the end of the assert.
You can run all test cases (including doc tests by running)

cargo test --doc --all-features

Fix typo

Co-authored-by: Billy Chan <[email protected]>
@MozarellaMan
Copy link
Copy Markdown
Contributor Author

I missed that! Thanks

@tyt2y3
Copy link
Copy Markdown
Member

tyt2y3 commented May 4, 2021

Looks good to me! Ready for merging?

@tyt2y3 tyt2y3 changed the base branch from master to postgres-types May 4, 2021 10:11
@tyt2y3 tyt2y3 merged commit 19043f8 into SeaQL:postgres-types May 4, 2021
@tyt2y3
Copy link
Copy Markdown
Member

tyt2y3 commented May 4, 2021

I intentionally ran cargo fmt on my branch to 'subtract out' those lines.

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.

3 participants