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.

Sign up for GitHub

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

Jump to bottom

Add documentation for TiDB specific SQL functions #5699

Merged
merged 2 commits into from
Jun 3, 2021
Merged

Add documentation for TiDB specific SQL functions #5699

merged 2 commits into from
Jun 3, 2021

Conversation

dveeden
Copy link
Contributor

@dveeden dveeden commented May 26, 2021 •
edited
Loading

What is changed, added or deleted? (Required)

Add documentation for SQL functions TIDB_PARSE_TSO and TIDB_BOUNDED_STALENESS.

Which TiDB version(s) do your changes apply to? (Required)

  • master (the latest development version)
  • v5.0 (TiDB 5.0 versions)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

Related:

@ti-chi-bot ti-chi-bot requested a review from TomShawn May 26, 2021 09:51
@ti-chi-bot ti-chi-bot added missing-translation-status This PR does not have translation status info. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 26, 2021
@dveeden dveeden requested a review from XuHuaiyu May 26, 2021 10:07
@TomShawn TomShawn added needs-cherry-pick-release-5.0 translation/doing This PR's assignee is translating this PR. labels May 26, 2021
@ti-chi-bot ti-chi-bot removed the missing-translation-status This PR does not have translation status info. label May 26, 2021
@dveeden
Copy link
Contributor Author

dveeden commented May 27, 2021

pingcap/tidb#24813 may also be related. @JmPotato Is the TIDB_BOUNDED_STALENESS() function going to be removed?

@JmPotato
Copy link
Member

pingcap/tidb#24813 may also be related. @JmPotato Is the TIDB_BOUNDED_STALENESS() function going to be removed?

No, these syntax cleanups are not related to TIDB_BOUNDED_STALENESS(). TIDB_BOUNDED_STALENESS() is a newly added function that will be used in the Stale Read feature. I think the detailed usage of it will be added to the documentation later with the release of the Stale Read feature.

@dveeden
Copy link
Contributor Author

dveeden commented May 27, 2021

pingcap/tidb#24813 may also be related. @JmPotato Is the TIDB_BOUNDED_STALENESS() function going to be removed?

No, these syntax cleanups are not related to TIDB_BOUNDED_STALENESS(). TIDB_BOUNDED_STALENESS() is a newly added function that will be used in the Stale Read feature. I think the detailed usage of it will be added to the documentation later with the release of the Stale Read feature.

So for now this PR makes sense? Then later the docs for TIDB_BOUND_STALENESS can be extended?

@ti-chi-bot
Copy link
Member

@JmPotato: Thanks for your review. The bot only counts /lgtm from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

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.

@JmPotato
Copy link
Member

@nolouch @Yisaer PTAL.

@JmPotato
Copy link
Member

pingcap/tidb#24813 may also be related. @JmPotato Is the TIDB_BOUNDED_STALENESS() function going to be removed?

No, these syntax cleanups are not related to TIDB_BOUNDED_STALENESS(). TIDB_BOUNDED_STALENESS() is a newly added function that will be used in the Stale Read feature. I think the detailed usage of it will be added to the documentation later with the release of the Stale Read feature.

So for now this PR makes sense? Then later the docs for TIDB_BOUND_STALENESS can be extended?

This doc LGTM, and we can extend it later.

@TomShawn TomShawn requested review from nolouch and Yisaer May 27, 2021 09:34
@dveeden
Copy link
Contributor Author

dveeden commented May 27, 2021

It actually looks like there are more functions that are not documented.

5.7.25-TiDB-v5.1.0-alpha-13-gd5e0ed0aa-dirty 127.0.0.1:4000  test  SQL  \pager grep tidb
Pager has been set to 'grep tidb'.
5.7.25-TiDB-v5.1.0-alpha-13-gd5e0ed0aa-dirty 127.0.0.1:4000  test  SQL  show builtins;
| tidb_bounded_staleness      |
| tidb_decode_key             |
| tidb_decode_plan            |
| tidb_is_ddl_owner           |
| tidb_parse_tso              |
| tidb_version                |
5.7.25-TiDB-v5.1.0-alpha-13-gd5e0ed0aa-dirty 127.0.0.1:4000  test  SQL  \pager
Pager has been disabled.

@dveeden
Copy link
Contributor Author

dveeden commented May 27, 2021 •
edited
Loading

Let's try to fix #5699 as part of this.

@dveeden
Copy link
Contributor Author

dveeden commented May 27, 2021

This PR has a small problem: These are not statements, these are functions.

One of these (TIDB_VERSION())is already documented here: https://docs.pingcap.com/tidb/stable/information-functions

These really could use some examples instead of a one-line description.

@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 27, 2021
@dveeden dveeden force-pushed the tidb_functions branch 3 times, most recently from 8017af5 to ba2991e Compare May 27, 2021 17:03
morgo
morgo reviewed May 31, 2021
View reviewed changes
Copy link
Contributor

@morgo morgo left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for working on this!

TOC.md Outdated Show resolved Hide resolved
functions-and-operators/tidb-functions.md Outdated Show resolved Hide resolved
functions-and-operators/tidb-functions.md Outdated Show resolved Hide resolved
@dveeden dveeden requested a review from morgo June 1, 2021 05:27
TomShawn
TomShawn approved these changes Jun 3, 2021
View reviewed changes
Copy link
Contributor

@TomShawn TomShawn left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • TomShawn

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.

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

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 3, 2021
@TomShawn TomShawn added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 3, 2021
@TomShawn
Copy link
Contributor

TomShawn commented Jun 3, 2021

@dveeden @morgo @JmPotato Is this PR ready to merge?

@morgo
Copy link
Contributor

morgo commented Jun 3, 2021

/LGTM

@ti-chi-bot
Copy link
Member

@morgo: Please use GitHub review feature instead of /lgtm [cancel] when you want to submit review to the pull request.
For how to use GitHub review feature, see also this document provided by GitHub.

For the reason we drop support to the commands, see also this page.
This reply is being used as a temporary reply during the migration of review process and will be removed on July 1st.

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.

@dveeden
Copy link
Contributor Author

dveeden commented Jun 3, 2021

/LGTM

@ti-chi-bot
Copy link
Member

@dveeden: Please use GitHub review feature instead of /lgtm [cancel] when you want to submit review to the pull request.
For how to use GitHub review feature, see also this document provided by GitHub.

For the reason we drop support to the commands, see also this page.
This reply is being used as a temporary reply during the migration of review process and will be removed on July 1st.

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.

@TomShawn
Copy link
Contributor

TomShawn commented Jun 3, 2021

/remove-status LGT1
/status LGT2

@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 Jun 3, 2021
@TomShawn
Copy link
Contributor

TomShawn commented Jun 3, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 2aaae5e

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 3, 2021
@ti-chi-bot ti-chi-bot merged commit 92aea64 into pingcap:master Jun 3, 2021
@ti-chi-bot ti-chi-bot mentioned this pull request Jun 3, 2021
Merged
6 tasks
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #5751.

ti-chi-bot added a commit that referenced this pull request Jun 3, 2021
@ti-chi-bot
Add documentation for TiDB specific SQL functions (#5699) (#5751)
cf10263
@JmPotato JmPotato mentioned this pull request Jun 8, 2021
Merged
11 tasks
@Liuxiaozhen12 Liuxiaozhen12 mentioned this pull request Jun 24, 2021
Merged
11 tasks
@Liuxiaozhen12
Copy link
Contributor

/remove-translation doing
/translation done

@ti-chi-bot ti-chi-bot added translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. and removed translation/doing This PR's assignee is translating this PR. labels Jun 24, 2021
@dveeden dveeden mentioned this pull request Jun 24, 2021
Closed
@TomShawn TomShawn added the requires-followup This PR requires a follow-up task after being merged. label Jul 5, 2021
This was referenced Jul 5, 2021
Merged
Merged
@TomShawn TomShawn mentioned this pull request Apr 20, 2022
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomShawn TomShawn TomShawn approved these changes

@JmPotato JmPotato JmPotato approved these changes

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

@nolouch nolouch Awaiting requested review from nolouch

@Yisaer Yisaer Awaiting requested review from Yisaer

@morgo morgo Awaiting requested review from morgo

Assignees

@Liuxiaozhen12 Liuxiaozhen12

Labels
requires-followup This PR requires a follow-up task after being merged. size/L Denotes a PR that changes 100-499 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. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dveeden @JmPotato @ti-chi-bot @morgo @TomShawn @Liuxiaozhen12