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

[#1632] fix(jdbc-mysql): Supporting create not null column for MySQL table. #1637

Merged
merged 6 commits into from
Jan 23, 2024

Conversation

yuqi1129
Copy link
Contributor

What changes were proposed in this pull request?

Add NOT NULL key word in create table sentence if column type is NOT NULL.

Why are the changes needed?

We should support creating a table with not null columns.

Fix: #1632

Does this PR introduce any user-facing change?

N/A.

How was this patch tested?

Add some ITs.

@yuqi1129 yuqi1129 requested review from diqiu50 and Clearvive and removed request for diqiu50 January 22, 2024 09:55
@yuqi1129 yuqi1129 self-assigned this Jan 22, 2024
@yuqi1129
Copy link
Contributor Author

PG has no such problem.

@yuqi1129
Copy link
Contributor Author

@diqiu50
Please take another look.

Copy link
Contributor

@diqiu50 diqiu50 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -435,8 +434,7 @@ private String updateColumnCommentFieldDefinition(
return "MODIFY COLUMN " + col + appendColumnDefinition(updateColumn, new StringBuilder());
}

private String addColumnFieldDefinition(
TableChange.AddColumn addColumn, CreateTable createTable) {
private String addColumnFieldDefinition(TableChange.AddColumn addColumn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

besides trino test, could you add a mysql test?

@jerryshao
Copy link
Contributor

Do we also need to support this for PG?

@yuqi1129
Copy link
Contributor Author

Do we also need to support this for PG?

No, I have checked PG has no such issue.
image

image

@jerryshao jerryshao merged commit 28cb2f7 into apache:main Jan 23, 2024
12 checks passed
mchades pushed a commit to mchades/gravitino that referenced this pull request Jan 24, 2024
…MySQL table. (apache#1637)

### What changes were proposed in this pull request?

Add `NOT NULL` key word in create table sentence if column type is NOT
NULL.

### Why are the changes needed?

We should support creating a table with not null columns.

Fix: apache#1632

### Does this PR introduce _any_ user-facing change?

N/A.

### How was this patch tested?

Add some ITs.
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.

[Bug report] Can add a not null column by Trino
4 participants