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

UI: change column's default property to nullable (NULL) #560

Closed
waynexia opened this issue Nov 17, 2022 · 6 comments
Closed

UI: change column's default property to nullable (NULL) #560

waynexia opened this issue Nov 17, 2022 · 6 comments
Assignees
Labels
C-enhancement Category Enhancements

Comments

@waynexia
Copy link
Member

What type of enhancement is this?

API improvement

What does the enhancement do?

If a column's nullability is not specified in CREATE TABLE clause it will be defaulted to NON-NULL. But other system like postgresql is default to NULL.

Ours:

mysql> CREATE TABLE monitor (
    ->   host STRING,
    ->   ts TIMESTAMP DEFAULT current_timestamp,
    ->   cpu DOUBLE DEFAULT 0,
    ->   memory DOUBLE NOT NULL,
    ->   TIME INDEX (ts),
    ->   PRIMARY KEY(host)) ENGINE=mito WITH(regions=1);
Query OK, 1 row affected (0.07 sec)

mysql> desc table monitor;
+--------+-----------+------+---------------------+---------------+
| Field  | Type      | Null | Default             | Semantic Type |
+--------+-----------+------+---------------------+---------------+
| host   | String    | NO   |                     | PRIMARY KEY   |
| ts     | Timestamp | NO   | current_timestamp() | TIME INDEX    |
| cpu    | Float64   | NO   | 0                   | VALUE         |
| memory | Float64   | NO   |                     | VALUE         |
+--------+-----------+------+---------------------+---------------+
4 rows in set (0.01 sec)

A nullable column should be explicit declared:

mysql> CREATE TABLE monitor3 (
    ->   host STRING,
    ->   ts TIMESTAMP DEFAULT current_timestamp,
    ->   cpu DOUBLE DEFAULT 0,
    ->   memory DOUBLE NULL,
    ->   TIME INDEX (ts),
    ->   PRIMARY KEY(host)) ENGINE=mito WITH(regions=1);
Query OK, 1 row affected (0.06 sec)

mysql> desc table monitor3
    -> ;
+--------+-----------+------+---------------------+---------------+
| Field  | Type      | Null | Default             | Semantic Type |
+--------+-----------+------+---------------------+---------------+
| host   | String    | NO   |                     | PRIMARY KEY   |
| ts     | Timestamp | NO   | current_timestamp() | TIME INDEX    |
| cpu    | Float64   | NO   | 0                   | VALUE         |
| memory | Float64   | YES  |                     | VALUE         |
+--------+-----------+------+---------------------+---------------+
4 rows in set (0.01 sec)

Implementation challenges

No response

@waynexia waynexia added the C-enhancement Category Enhancements label Nov 17, 2022
@azhsmesos
Copy link
Contributor

Do you know what caused the mistake?

mysql> desc table monitor;
ERROR 1815 (HY000): Failed to execute query: desc table monitor, source: Failed to parse SQL, source: SQL statement is not supported: desc table monitor, keyword: desc

@waynexia
Copy link
Member Author

Do you know what caused the mistake?

mysql> desc table monitor;
ERROR 1815 (HY000): Failed to execute query: desc table monitor, source: Failed to parse SQL, source: SQL statement is not supported: desc table monitor, keyword: desc

The example I wrote uses an un-merged function (DESC TABLE) in #534 .

@waynexia
Copy link
Member Author

It is now merged. Your SQL should run as expect.

@azhsmesos
Copy link
Contributor

Please describe this issue again. I verified and found that it seems to meet the expectationPlease describe this issue again. I just verified and found that it seems to meet the expectation...

@waynexia
Copy link
Member Author

Please describe this issue again. I verified and found that it seems to meet the expectationPlease describe this issue again. I just verified and found that it seems to meet the expectation...

Sorry for the unclear description. In short, column's nullability property is current default to NOT NULL. And this issue request it to default to NULL.

@azhsmesos
Copy link
Contributor

I have completed the coding, can you assign it to me? I will mention pr later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category Enhancements
Projects
None yet
Development

No branches or pull requests

3 participants