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

feat: add DictionaryVector DataType #1061

Merged
merged 14 commits into from
Feb 23, 2023

Conversation

waynexia
Copy link
Member

@waynexia waynexia commented Feb 22, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Add a new datatype Dictionary to our type system. But only type definition and conversion are included, without the actual implementation. This is to make schema conversion with arrow work.

This patch also fixes RangeManipulate plan's behavior. It will now included the original timestamp column, and add the "manipulated" timestamp range vector as a new column.

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)

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@killme2008
Copy link
Contributor

Test failed:

thread 'tests::promql_test::sql_insert_tql_query_ceil' panicked at 'assertion failed: `(left == right)`
  left: `"+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n| 1970-01-01T00:00:00 | 67        | 1024         | host1 |\\n| 1970-01-01T00:00:10 | 100       | 20480        | host1 |\\n| 1970-01-01T00:00:20 | 100       | 20480        | host1 |\\n| 1970-01-01T00:00:30 | 32        | 8192         | host1 |\\n| 1970-01-01T00:00:40 | 96        | 334          | host1 |\\n| 1970-01-01T00:00:50 | 12424     | 1334         | host1 |\\n| 1970-01-01T00:01:00 | 12424     | 1334         | host1 |\\n| 1970-01-01T00:01:10 | 12424     | 1334         | host1 |\\n| 1970-01-01T00:01:20 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:30 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:40 | 49        | 3334         | host1 |\\n| ts                  | ceil(cpu) | ceil(memory) | host  |"`,
 right: `"+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n| 1970-01-01T00:00:00 | 67        | 1024         | host1 |\\n| 1970-01-01T00:00:10 | 100       | 2048         | host1 |\\n| 1970-01-01T00:00:20 | 100       | 2048         | host1 |\\n| 1970-01-01T00:00:30 | 32        | 8192         | host1 |\\n| 1970-01-01T00:00:40 | 96        | 334          | host1 |\\n| 1970-01-01T00:00:50 | 1242      | 1334         | host1 |\\n| 1970-01-01T00:01:00 | 1242      | 1334         | host1 |\\n| 1970-01-01T00:01:10 | 1242      | 1334         | host1 |\\n| 1970-01-01T00:01:20 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:30 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:40 | 49        | 3334         | host1 |\\n| ts                  | ceil(cpu) | ceil(memory) | host  |"`', src/datanode/src/tests/test_util.rs:179:5
stack backtrace:

Copy link
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

Mostly LGTM

src/common/grpc/src/select.rs Outdated Show resolved Hide resolved
src/datatypes/src/types/dictionary_type.rs Outdated Show resolved Hide resolved
src/promql/src/planner.rs Outdated Show resolved Hide resolved
@evenyag
Copy link
Contributor

evenyag commented Feb 23, 2023

Test failed:

thread 'tests::promql_test::sql_insert_tql_query_ceil' panicked at 'assertion failed: `(left == right)`
  left: `"+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n| 1970-01-01T00:00:00 | 67        | 1024         | host1 |\\n| 1970-01-01T00:00:10 | 100       | 20480        | host1 |\\n| 1970-01-01T00:00:20 | 100       | 20480        | host1 |\\n| 1970-01-01T00:00:30 | 32        | 8192         | host1 |\\n| 1970-01-01T00:00:40 | 96        | 334          | host1 |\\n| 1970-01-01T00:00:50 | 12424     | 1334         | host1 |\\n| 1970-01-01T00:01:00 | 12424     | 1334         | host1 |\\n| 1970-01-01T00:01:10 | 12424     | 1334         | host1 |\\n| 1970-01-01T00:01:20 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:30 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:40 | 49        | 3334         | host1 |\\n| ts                  | ceil(cpu) | ceil(memory) | host  |"`,
 right: `"+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n+---------------------+-----------+--------------+-------+\\n| 1970-01-01T00:00:00 | 67        | 1024         | host1 |\\n| 1970-01-01T00:00:10 | 100       | 2048         | host1 |\\n| 1970-01-01T00:00:20 | 100       | 2048         | host1 |\\n| 1970-01-01T00:00:30 | 32        | 8192         | host1 |\\n| 1970-01-01T00:00:40 | 96        | 334          | host1 |\\n| 1970-01-01T00:00:50 | 1242      | 1334         | host1 |\\n| 1970-01-01T00:01:00 | 1242      | 1334         | host1 |\\n| 1970-01-01T00:01:10 | 1242      | 1334         | host1 |\\n| 1970-01-01T00:01:20 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:30 | 0         | 2334         | host1 |\\n| 1970-01-01T00:01:40 | 49        | 3334         | host1 |\\n| ts                  | ceil(cpu) | ceil(memory) | host  |"`', src/datanode/src/tests/test_util.rs:179:5
stack backtrace:

But this test case is failed
https://github.com/GreptimeTeam/greptimedb/actions/runs/4241443815/jobs/7371668796

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #1061 (9f698cd) into develop (51641db) will increase coverage by 0.09%.
The diff coverage is 71.67%.

❗ Current head 9f698cd differs from pull request most recent head 39fff84. Consider uploading reports for the commit 39fff84 to get more accurate results

@@             Coverage Diff             @@
##           develop    #1061      +/-   ##
===========================================
+ Coverage    85.70%   85.80%   +0.09%     
===========================================
  Files          450      451       +1     
  Lines        65828    65990     +162     
===========================================
+ Hits         56418    56620     +202     
+ Misses        9410     9370      -40     
Flag Coverage Δ
rust 85.80% <71.67%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/api/src/helper.rs 91.48% <ø> (ø)
src/common/grpc-expr/src/insert.rs 81.28% <0.00%> (+0.14%) ⬆️
src/common/grpc/src/select.rs 87.42% <ø> (ø)
src/common/substrait/src/types.rs 51.92% <0.00%> (ø)
src/datatypes/src/data_type.rs 94.96% <0.00%> (-3.30%) ⬇️
src/datatypes/src/type_id.rs 38.70% <0.00%> (-5.74%) ⬇️
src/datatypes/src/types/dictionary_type.rs 0.00% <0.00%> (ø)
src/datatypes/src/value.rs 94.77% <0.00%> (-0.17%) ⬇️
src/datatypes/src/vectors/eq.rs 99.32% <ø> (ø)
src/script/src/python/vector.rs 70.45% <0.00%> (-0.09%) ⬇️
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

src/datatypes/src/value.rs Outdated Show resolved Hide resolved
src/promql/src/planner.rs Outdated Show resolved Hide resolved
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@waynexia waynexia requested a review from killme2008 February 23, 2023 12:21
Copy link
Contributor

@v0y4g3r v0y4g3r left a comment

Choose a reason for hiding this comment

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

LGTM

@v0y4g3r v0y4g3r merged commit 4cc3ac3 into GreptimeTeam:develop Feb 23, 2023
@waynexia waynexia deleted the prom-scalar-udf branch February 23, 2023 12:37
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* fix stddev and stdvar. try build range function expr

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* feat: add dictionary data type

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* preserve timestamp column in range manipulator

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* plan range functions

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* update test result

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* fix typo

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* update test result

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* update test result

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* resolve CR comments

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* resolve CR comments

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

---------

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
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.

4 participants