Skip to content

Json unquote extract pushdown#6377

Merged
yibin87 merged 33 commits intopingcap:masterfrom
yibin87:json_unquote_extract_pushdown
Dec 2, 2022
Merged

Json unquote extract pushdown#6377
yibin87 merged 33 commits intopingcap:masterfrom
yibin87:json_unquote_extract_pushdown

Conversation

@yibin87
Copy link
Contributor

@yibin87 yibin87 commented Nov 28, 2022 •
edited
Loading

What problem does this PR solve?

Issue Number: close #6376

Problem Summary:

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

yibin87 added 11 commits November 9, 2022 09:45
@yibin87
Move tidb new json definitions to tiflash
5a922ce 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Little change
6ff6618 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Temporary changes
105a024 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Milestore 1
80dde7c 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Basic functionality milestone
2d6948b 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Save changes
f3fa2ed 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Save works
7176f6c 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Add integration tests
3dabf46 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Fix incorrect path expr handling logic
a80a9a7 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Little refact
dc78571 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Reuse memory buffer
b0089eb 
Signed-off-by: yibin <huyibin@pingcap.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Nov 28, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • mengxin9014
  • solotzg

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Nov 28, 2022

@yibin87: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 28, 2022
@yibin87 yibin87 requested a review from mengxin9014 November 29, 2022 02:45
@yibin87
Fix static analyze failure
2b39859 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Fix static analyze errors
62c1f8c 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87 yibin87 removed the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Nov 29, 2022
@yibin87
Fix tidy static analyze issues
a84125e 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Copy link
Contributor Author

yibin87 commented Nov 29, 2022

/run-all-tests

@yibin87
Fix code change issue
1f11fde 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87
Copy link
Contributor Author

yibin87 commented Nov 29, 2022

/run-all-tests

mengxin9014
mengxin9014 reviewed Nov 29, 2022
View reviewed changes
@yibin87
Remove useless header
9e13d8c 
Signed-off-by: yibin <huyibin@pingcap.com>
@yibin87 yibin87 requested a review from mengxin9014 November 29, 2022 08:37
@yibin87
Copy link
Contributor Author

yibin87 commented Dec 1, 2022

/run-all-tests

mengxin9014
mengxin9014 reviewed Dec 1, 2022
View reviewed changes
Copy link
Contributor

@mengxin9014 mengxin9014 left a comment

Choose a reason for hiding this comment

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

Others LGTM

@yibin87 yibin87 requested a review from mengxin9014 December 1, 2022 03:42
solotzg
solotzg reviewed Dec 2, 2022
View reviewed changes
dbms/src/Common/UTF8Helpers.h Outdated
Comment on lines +32 to +33
static const UInt32 UNICODEMax = 0x0010FFFF; // Maximum valid Unicode code point.
static const UInt32 UTF8Error = UNICODEMax + 1; // the "error" code
Copy link
Contributor

@solotzg solotzg Dec 2, 2022

Choose a reason for hiding this comment

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

UNICODEMax -> UNICODE_MAX
UTF8Error -> UTF8_ERROR

Copy link
Contributor Author

@yibin87 yibin87 Dec 2, 2022

Choose a reason for hiding this comment

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

Nice, changed it.

Copy link
Contributor

@solotzg solotzg Dec 2, 2022

Choose a reason for hiding this comment

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

still UNICODE_Max

Copy link
Contributor Author

@yibin87 yibin87 Dec 2, 2022

Choose a reason for hiding this comment

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

Yeah, I didn't notice the upper case, update it now.

solotzg
solotzg reviewed Dec 2, 2022
View reviewed changes
@yibin87
Update to comments
f18245a 
Signed-off-by: yibin <huyibin@pingcap.com>
solotzg
solotzg reviewed Dec 2, 2022
View reviewed changes
@solotzg
Copy link
Contributor

solotzg commented Dec 2, 2022

Is there any code coverage report about your modification ?

@yibin87
Copy link
Contributor Author

yibin87 commented Dec 2, 2022

/run-unit-test

@yibin87
Copy link
Contributor Author

yibin87 commented Dec 2, 2022

Is there any code coverage report about your modification ?

I'll run tests to see.

@yibin87
Copy link
Contributor Author

yibin87 commented Dec 2, 2022

/run-all-tests

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 2, 2022
@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 2, 2022
@yibin87
Copy link
Contributor Author

yibin87 commented Dec 2, 2022

/merge

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 2, 2022

@yibin87: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 2, 2022

This pull request has been accepted and is ready to merge.

DetailsCommit hash: f18245a

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 2, 2022
@yibin87
Copy link
Contributor Author

yibin87 commented Dec 2, 2022

/re-build

@yibin87
Copy link
Contributor Author

yibin87 commented Dec 2, 2022

/re-builds

@yibin87
Copy link
Contributor Author

yibin87 commented Dec 2, 2022

/rebuild

@wuhuizuo
Copy link
Contributor

wuhuizuo commented Dec 2, 2022

/run-unit-test
/run-integration-test

@yibin87 yibin87 merged commit 4bae244 into pingcap:master Dec 2, 2022
@yibin87 yibin87 mentioned this pull request Dec 2, 2022
Closed
@yibin87 yibin87 mentioned this pull request Nov 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@solotzg solotzg solotzg approved these changes

@SeaRise SeaRise Awaiting requested review from SeaRise

@windtalker windtalker Awaiting requested review from windtalker

+1 more reviewer

@mengxin9014 mengxin9014 mengxin9014 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support 'json_extract', '->', '->>' json functions in TiFlash

6 participants

@yibin87 @ti-chi-bot @solotzg @wuhuizuo @windtalker @mengxin9014