feat!: support table ddl for custom storage

[NiwakaDev]

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

This PR closes #919.

How to use

We first need to configure as follows:

            [storage]
            type = "File"
            ~
            [[storage.providers]]
            type = "S3"
            ~
            [[storage.providers]]
            type = "Gcs"
            bucket = "foo"

Then, we can use this feature like:

CREATE TABLE foo_table (
  ~
  TIME INDEX (ts),
)
PARTITION BY RANGE COLUMNS (id) (
    ~
    PARTITION r2 VALUES LESS THAN (MAXVALUE),
)
ENGINE=mito
WITH(
  storage = 'S3'
);

Checklist

• I have written the necessary rustdoc comments.
• I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

close #919

[codecov]

Codecov Report

Merging #2733 (c835ec4) into develop (9e58bba) will decrease coverage by 0.38%.
Report is 46 commits behind head on develop.
The diff coverage is 85.10%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2733      +/-   ##
===========================================
- Coverage    84.74%   84.37%   -0.38%     
===========================================
  Files          732      749      +17     
  Lines       114657   117998    +3341     
===========================================
+ Hits         97168    99561    +2393     
- Misses       17489    18437     +948     

[killme2008]

Great work! I think we can rename storage.default_store to storage to keep compatible with old versions, what do you think?

[NiwakaDev]

@evenyag @killme2008

Should we consider accepting the default storage as the storage option?

If so, should show create table include the default storage as the storage in its output?

Something like:

create table foo(
  ~, 
  ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP TIME INDEX
) 
with(storage='File') //default storage

show create table foo;

generates

+-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                                                                     |
+-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| demo  | CREATE TABLE IF NOT EXISTS `foo` (
   ~
  `ts` TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(),
  TIME INDEX (`ts`)
)

ENGINE=mito
WITH(
  regions = 1,
  storage="File",
) |
+-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

In this PR implementation,

• If users specify the default storage as storage option, show create table generates storage=~.
• If users don't specify it, show create table doesn't generate storage=~.

I guess we should unify the behavior.

[killme2008]

@NiwakaDev I think we can always show the storage option when executing show create table. We can save this info into table metadata while creating table even if the user doesn't specify it.

[killme2008]

LGTM. It's really cool and useful feature, thanks a lot.

[fengjiachun]

@evenyag PTAL

[evenyag]

The following test failed in the merge queue. Looks like a time zone related issue. '1970-01-01 00:00:00.001+0000' != '1970-01-01 09:00:00.001+0900'

--- TRY 4 STDERR:        tests-integration tests::instance_test::test_custom_storage::case_2_test_with_distributed ---
thread 'tests::instance_test::test_custom_storage::case_2_test_with_distributed' panicked at tests-integration/src/tests/instance_test.rs:1945:13:
assertion failed: `(left == right)`
  left: `"CREATE TABLE IF NOT EXISTS \"test_table\" (\n  \"host\" STRING NULL,\n  \"ts\" TIMESTAMP(3) NOT NULL,\n  TIME INDEX (\"ts\")\n)\nPARTITION BY RANGE COLUMNS (\"ts\") (\n  PARTITION r0 VALUES LESS THAN ('1970-01-01 00:00:00.001+0000'),\n  PARTITION r1 VALUES LESS THAN ('1970-01-01 00:00:00.010+0000'),\n  PARTITION r2 VALUES LESS THAN ('1970-01-01 00:00:00.100+0000'),\n  PARTITION r3 VALUES LESS THAN (MAXVALUE)\n)\nENGINE=mito\nWITH(\n  regions = 4,\n  storage = 'S3'\n)"`,
 right: `"CREATE TABLE IF NOT EXISTS \"test_table\" (\n  \"host\" STRING NULL,\n  \"ts\" TIMESTAMP(3) NOT NULL,\n  TIME INDEX (\"ts\")\n)\nPARTITION BY RANGE COLUMNS (\"ts\") (\n  PARTITION r0 VALUES LESS THAN ('1970-01-01 09:00:00.001+0900'),\n  PARTITION r1 VALUES LESS THAN ('1970-01-01 09:00:00.010+0900'),\n  PARTITION r2 VALUES LESS THAN ('1970-01-01 09:00:00.100+0900'),\n  PARTITION r3 VALUES LESS THAN (MAXVALUE)\n)\nENGINE=mito\nWITH(\n  regions = 4,\n  storage = 'S3'\n)"`

[evenyag]

The following test failed in the merge queue. Looks like a time zone related issue. '1970-01-01 00:00:00.001+0000' != '1970-01-01 09:00:00.001+0900'

We might choose another column (host ?) instead of ts as RANGE COLUMNS. @NiwakaDev

[killme2008]

@NiwakaDev Hi, sorry. There are some conflicts that have to be resolved.

[evenyag]

@killme2008 @NiwakaDev This PR still has a test case that needs to be fixed. Otherwise, it won't pass the merge queue.

--- TRY 4 STDERR:        tests-integration tests::instance_test::test_custom_storage::case_2_test_with_distributed ---

[NiwakaDev]

@NiwakaDev Hi, sorry. There are some conflicts that have to be resolved.

@killme2008 @NiwakaDev This PR still has a test case that needs to be fixed. Otherwise, it won't pass the merge queue.

Sorry for the late response. I'll address these issues by tomorrow.

[evenyag]

Just a reminder. You can do it whenever you have free time.

[evenyag]

Oops, there are some conflicts.

[killme2008]

@NiwakaDev Hi, how are u going? Would you have time to look into these code conflicts? If you're too busy, I'm more than happy to lend a hand.

[NiwakaDev]

@killme2008

I sincerely apologize for not promptly responding in November.

Would you have time to look into these code conflicts?

Yes. I can address this issue from tonight.

[killme2008]

@killme2008

I sincerely apologize for not promptly responding in November.

Would you have time to look into these code conflicts?

Yes. I can address this issue from tonight.

Sorry for disturbing you. I think this feature is really useful and cool, look forward to merging it.

[NiwakaDev]

@killme2008

I don't address this yet: #2733 (comment)

I think meta-srv needs to know default storage so that it can save this info into table metadata.

Right now can we fetch storage configuration corresponding to a region from datanode to meta-srv?

[killme2008]

@killme2008

I don't address this yet: #2733 (comment)

I think meta-srv needs to know default storage so that it can save this info into table metadata.

Right now can we fetch storage configuration corresponding to a region from datanode to meta-srv?

Sorry, I changed my mind. I think it's ok. If the user doesn't specify the storage option, we don't need to generate the storage=~ too. I think the users know what they are doing.

[NiwakaDev]

@killme2008 @evenyag
Thank you for supporting, and I'm sorry for not promptly responding in November. I'll continue to contribute more if you don't mind. Thank you!!

[killme2008]

@killme2008 @evenyag Thank you for supporting, and I'm sorry for not promptly responding in November. I'll continue to contribute more if you don't mind. Thank you!!

Please don't mind, I hope you are doing well. Appreciate your contribution and look forward to you continuing to contribute more! Thank you.