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 concat & improve strip_nulls #42

Merged
merged 2 commits into from
Jan 3, 2024

Conversation

akoshchiy
Copy link
Contributor

@akoshchiy akoshchiy commented Jan 1, 2024

  1. Added a builder api, which allows a json modification without the full deserialisation;
  2. Added concat for Tracking: JSON functions and operators databend#11270;
  3. Moved strip_nulls to the new builder;

Also, added a benchmark for comparing the strip_nulls implementations.

Results for m1 pro are:

strip_nulls_deser[./data/canada.json]
                        time:   [5.5970 ms 5.6163 ms 5.6352 ms]

strip_nulls_fast[./data/canada.json]
                        time:   [2.8134 ms 2.8249 ms 2.8363 ms]

Benchmarking strip_nulls_deser[./data/twitter.json]: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.8s, enable flat sampling, or reduce sample count to 50.
strip_nulls_deser[./data/twitter.json]
                        time:   [1.7488 ms 1.7705 ms 1.8024 ms]
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) high mild
  3 (3.00%) high severe

strip_nulls_fast[./data/twitter.json]
                        time:   [812.35 µs 815.72 µs 819.60 µs]
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) high mild
  3 (3.00%) high severe

strip_nulls_deser[./data/citm_catalog.json]
                        time:   [4.4845 ms 4.5077 ms 4.5306 ms]

strip_nulls_fast[./data/citm_catalog.json]
                        time:   [2.6602 ms 2.6698 ms 2.6797 ms]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

@akoshchiy akoshchiy marked this pull request as ready for review January 1, 2024 13:50
@b41sh b41sh self-requested a review January 2, 2024 09:58
Copy link
Member

@b41sh b41sh left a comment

Choose a reason for hiding this comment

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

LGTM
Great job on the task, builder looks very useful and we can use it for other functions
to simplify the code. Happy New Year to you by the way! @akoshchiy

@b41sh b41sh merged commit 5a43132 into databendlabs:main Jan 3, 2024
1 check passed
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