Skip to content

[function](json) Json unquote - #18037

Merged
dataroaring merged 8 commits into
apache:masterfrom
Mellorsssss:json_unquote
Apr 24, 2023
Merged

[function](json) Json unquote#18037
dataroaring merged 8 commits into
apache:masterfrom
Mellorsssss:json_unquote

Conversation

@Mellorsssss

@MellorsssssMellorsssss commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Proposed changes

Issue Number: close#17216

Problem summary

Implement json_unquote.

mysql> SELECT json_unquote('"doris"');
+-------------------------+
| json_unquote('"doris"') |
+-------------------------+
| doris |
+-------------------------+

Checklist(Required)

  • Does it affect the original behavior
  • Has unit tests been added
  • Has document been added or modified
  • Does it need to update dependencies
  • Is this PR support rollback (If NO, please explain WHY)

Further comments

Currently, the NO_BACKSLASH_ESCAPES sql mode specified in the MySQL documentation at does not actually take effect. Therefore, even if a user enables NO_BACKSLASH_ESCAPES, the behavior of json_unquote will not change.

@github-actionsgithub-actionsBot added area/sql/function Issues or PRs related to the SQL functions area/vectorization kind/docs Categorizes issue or PR as related to documentation. kind/test labels Mar 22, 2023
@github-actions

github-actionsBot commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@morningmanmorningman changed the title Json unquote[function](json) Json unquoteMar 23, 2023
Yukang-Lian
Yukang-Lian previously approved these changes Mar 23, 2023
View reviewed changes

@Yukang-LianYukang-Lian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@github-actions

github-actionsBot commented Mar 23, 2023

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@Yukang-Lian

Yukang-Lian commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator

run buildall

1 similar comment
@dataroaring

dataroaring commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

run buildall

@dataroaring

dataroaring commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

add a regression test for normal string and "".

@hello-stephen

hello-stephen commented Mar 25, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 36.08 seconds
stream load tsv: 445 seconds loaded 74807831229 Bytes, about 160 MB/s
stream load json: 23 seconds loaded 2358488459 Bytes, about 97 MB/s
stream load orc: 72 seconds loaded 1101869774 Bytes, about 14 MB/s
stream load parquet: 30 seconds loaded 861443392 Bytes, about 27 MB/s
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230329140811_clickbench_pr_122502.html

@Mellorsssss

Mellorsssss commented Mar 25, 2023

Copy link
Copy Markdown
ContributorAuthor

done

@Mellorsssss

Mellorsssss commented Mar 25, 2023

Copy link
Copy Markdown
ContributorAuthor

run buildall

xiaokang
xiaokang reviewed Mar 25, 2023
View reviewed changes
Comment threadbe/src/vec/functions/function_json.cpp Outdated
}

const auto& data = col_from_string->get_data_at(i);
if (data.size < 2 || data.data[0] != '"' || data.data[data.size - 1] != '"') {

@xiaokangxiaokangMar 25, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If data.size == 0, data[0] should not be used.

@MellorsssssMellorsssssMar 25, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

If the data.size == 0, then data.data[0] will not be evaluated since data.size < 2 will be true. I add a test case for the empty string ''.

Comment threadbe/src/vec/functions/function_json.cpp Outdated
continue;
}

const auto& data = col_from_string->get_data_at(i);

@xiaokangxiaokangMar 25, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The name data.data is strange. Can you change variable data to json_str?

@MellorsssssMellorsssssMar 25, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks xiaokang! I will rename it.

@github-actions

github-actionsBot commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Mellorsssss

Mellorsssss commented Mar 25, 2023

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

github-actionsBot commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Mellorsssss

Mellorsssss commented Mar 25, 2023

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

github-actionsBot commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Mellorsssss

Mellorsssss commented Mar 26, 2023

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

github-actionsBot commented Mar 26, 2023

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@nextdreamblue

nextdreamblue commented Mar 27, 2023

Copy link
Copy Markdown
Contributor

LGTM

@github-actions

github-actionsBot commented Mar 29, 2023

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Mellorsssss

Mellorsssss commented Mar 29, 2023

Copy link
Copy Markdown
ContributorAuthor

run buildall

xiaokang
xiaokang approved these changes Apr 17, 2023
View reviewed changes

@xiaokangxiaokang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Yukang-Lian

Yukang-Lian commented Apr 17, 2023

Copy link
Copy Markdown
Collaborator

run p1

dataroaring
dataroaring approved these changes Apr 24, 2023
View reviewed changes

@dataroaringdataroaring left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dataroaring
dataroaring merged commit ab2a686 into apache:masterApr 24, 2023
gnehil pushed a commit to gnehil/doris that referenced this pull request Apr 25, 2023
@Mellorsssss@gnehil
[function](json) Json unquote (apache#18037)
37b2cd8
Reminiscent pushed a commit to Reminiscent/doris that referenced this pull request May 15, 2023
@Mellorsssss@Reminiscent
[function](json) Json unquote (apache#18037)
fb59e7a
liutang123 pushed a commit to liutang123/doris that referenced this pull request Feb 20, 2025
derenli
[Tencent] trival changes, include audit plugins and some json functions.
37906c7
1. add reload and avoid exception when install plugin again
2. add json_valid function (apache#17247)
3. add json unquote (apache#18037)
4. add json->operator convert to json_extract (apache#19899)
5. add json_contains function (apache#20824)
6. add DLC catalog support (apache#20314)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xiaokangxiaokangxiaokang approved these changes
@dataroaringdataroaringdataroaring approved these changes
@Yukang-LianYukang-LianYukang-Lian left review comments

Assignees

No one assigned

Labels

area/sql/functionIssues or PRs related to the SQL functionsarea/vectorizationkind/docsCategorizes issue or PR as related to documentation.kind/testreviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature] add some new json function

6 participants

@Mellorsssss@Yukang-Lian@dataroaring@hello-stephen@nextdreamblue@xiaokang