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

Fix db2 options #661

Merged
merged 1 commit into from
Nov 11, 2023
Merged

Conversation

chrjorgensen
Copy link
Contributor

@chrjorgensen chrjorgensen commented Nov 11, 2023

A fix to DB2 formatter:

  • ALTER TABLE RENAME COLUMN is not supported in DB2i

@nene
Copy link
Collaborator

nene commented Nov 11, 2023

Sorry, in the mean time I had already fixed the U&'' strings difference between DB2 and DB2i. Just had forgotten to push my changes.

See: 3a7a53a

@nene
Copy link
Collaborator

nene commented Nov 11, 2023

Also, if you're further interested in improving DB2 support, you might want to review the wiki pages which describe the basic syntax of various dialects.

I updated the first section (identifiers, strings, etc) there with references to DB2 LUW-version and DB2i. With the LOW-version of DB2 I can test out the syntax in dbfiddle. But for DB2i I have no other sources besides the IBM documentation. The "Queries" sections is also mostly updated, the rest is still in todo status.

@chrjorgensen
Copy link
Contributor Author

Sorry, in the mean time I had already fixed the U&'' strings difference between DB2 and DB2i. Just had forgotten to push my changes.

That's alright - it's better that we are two thinking the same than none of us is thinking about this. 👍
I'll remove the change and force push the `RENAME´ change.

@nene
Copy link
Collaborator

nene commented Nov 11, 2023

Thanks.

@nene nene merged commit 533aa67 into sql-formatter-org:master Nov 11, 2023
2 checks passed
@chrjorgensen
Copy link
Contributor Author

Thank you for doing all this great work on the DB2 dialects!

@chrjorgensen chrjorgensen deleted the fix/db2-options branch November 11, 2023 18:41
supportsAlterTable(format, {
addColumn: true,
dropColumn: true,
renameColumn: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI: In general instead of specifying false here, better to just skip the option altogether. At least that's the style that the rest of the tests are following for now. Keeps the tests shorter, as like supportsAlterTable() takes an entire 5 boolean flags.

This concrete case I already cleaned up: 91e4be0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted - only did it this way to clarify that it was on purpose in contrast to the DB2, which has true in renameColumn.

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.

2 participants