delete description about special non-reserved keywords that might still require special treatment (#21116)#21121
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot
added
do-not-merge/hold
|
@hfxsd This PR has conflicts, I have hold it. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.gh.lumenfield.work/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @ti-chi-bot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Gemini here, providing a summary of this pull request. This PR, authored by ti-chi-bot, appears to be an automated cherry-pick aimed at updating the TiDB documentation regarding SQL keywords. Specifically, it removes mentions and descriptions of a category previously referred to as "special non-reserved keywords that might still require special treatment". This suggests that this distinction or the required special treatment is no longer relevant or accurate in the current state of TiDB's SQL parser, aligning with the referenced parser and tidb commits.
Highlights
- Documentation Update: Removes outdated information about a specific category of non-reserved keywords in the
keywords.mddocumentation file. - Keyword Classification: Deletes the description of the "(S)" marker previously used to denote these special non-reserved keywords in the keyword list.
Changelog
- keywords.md
- Removed the sentence describing "special non-reserved keywords that might still require special treatment" and the recommendation to treat them as reserved (around line 11-13).
- Removed the description of the "(S)" marker from the introductory text for the keyword list (around line 66).
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This PR aims to simplify the documentation around keywords by removing the concept of 'special non-reserved keywords' and the associated (S) marker, aligning with recent parser changes (as indicated in linked PRs like parser#1322). This simplification improves clarity and accuracy. Thank you for this update!
However, there's a critical merge conflict in keywords.md that needs to be resolved. I've provided a specific comment and suggestion to fix this. The other change in the PR (removing the (S) marker description) is correct and aligns with the overall goal.
This review adheres to the Documentation Review Style Guide. The feedback focuses on ensuring technical accuracy and document integrity by identifying and providing a solution for a critical merge conflict [^1]. The aim is to provide ready-to-commit suggestions [^2] and to add comments directly rather than formally requesting changes, to facilitate a smoother PR process [^3].
Summary of Findings
- Critical Merge Conflict: A merge conflict exists in
keywords.md(lines 10-14 on the right side of the diff). This conflict includes markers like<<<<<<< HEADand prevents the document from being correctly rendered. It must be resolved. - Accuracy of Change: Once the merge conflict is resolved, the intended change to remove the description of 'special non-reserved keywords' and the
(S)marker aligns with underlying parser changes and simplifies the documentation correctly.
Merge Readiness
This PR introduces a valuable simplification to the keyword documentation. However, it cannot be merged in its current state due to a critical merge conflict in keywords.md. The provided review comment includes a suggestion to resolve this conflict.
As per the Documentation Review Style Guide (line 9), I am setting the review status to COMMENT to avoid procedurally blocking the PR. However, please understand that addressing this merge conflict is essential and mandatory before the PR can be considered ready for merge.
I am unable to approve the pull request. It will require further review and approval from other maintainers after the conflict is fixed and the file is correctly formatted.
| <<<<<<< HEAD | ||
| Keywords are words that have special meanings in SQL statements, such as `SELECT`, `UPDATE`, and `DELETE`. Some of them can be used as identifiers directly, which are called **non-reserved keywords**. Some of them require special treatment before being used as identifiers, which are called **reserved keywords**. However, there are special non-reserved keywords that might still require special treatment. It is recommended that you treat them as reserved keywords. | ||
| ======= | ||
| Keywords are words that have special meanings in SQL statements, such as [`SELECT`](/sql-statements/sql-statement-select.md), [`UPDATE`](/sql-statements/sql-statement-update.md), and [`DELETE`](/sql-statements/sql-statement-delete.md). Some of them can be used as identifiers directly, which are called **non-reserved keywords**. Some of them require special treatment before being used as identifiers, which are called **reserved keywords**. | ||
| >>>>>>> 3dc620c9c9 (delete description about special non-reserved keywords that might still require special treatment (#21116)) |
There was a problem hiding this comment.
It appears there's a merge conflict in this section, indicated by the <<<<<<< HEAD, =======, and >>>>>>> markers. This makes the document incorrectly formatted and unreadable here.
Based on the PR's intent (to delete the description about special non-reserved keywords and use the updated text from the cherry-picked commit, which also includes helpful links for SELECT, UPDATE, and DELETE), the content introduced by the cherry-pick (the part after =======) should be retained.
Could you please resolve this conflict? The suggestion below provides the intended clean text for this paragraph.
Style Guide References
| <<<<<<< HEAD | |
| Keywords are words that have special meanings in SQL statements, such as `SELECT`, `UPDATE`, and `DELETE`. Some of them can be used as identifiers directly, which are called **non-reserved keywords**. Some of them require special treatment before being used as identifiers, which are called **reserved keywords**. However, there are special non-reserved keywords that might still require special treatment. It is recommended that you treat them as reserved keywords. | |
| ======= | |
| Keywords are words that have special meanings in SQL statements, such as [`SELECT`](/sql-statements/sql-statement-select.md), [`UPDATE`](/sql-statements/sql-statement-update.md), and [`DELETE`](/sql-statements/sql-statement-delete.md). Some of them can be used as identifiers directly, which are called **non-reserved keywords**. Some of them require special treatment before being used as identifiers, which are called **reserved keywords**. | |
| >>>>>>> 3dc620c9c9 (delete description about special non-reserved keywords that might still require special treatment (#21116)) | |
| Keywords are words that have special meanings in SQL statements, such as [`SELECT`](/sql-statements/sql-statement-select.md), [`UPDATE`](/sql-statements/sql-statement-update.md), and [`DELETE`](/sql-statements/sql-statement-delete.md). Some of them can be used as identifiers directly, which are called **non-reserved keywords**. Some of them require special treatment before being used as identifiers, which are called **reserved keywords**. |
|
@gemini-code-assist[bot]: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn 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 kubernetes-sigs/prow repository. |
|
@ti-chi-bot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
2 participants
This is an automated cherry-pick of #21116
First-time contributors' checklist
What is changed, added or deleted? (Required)
Which TiDB version(s) do your changes apply to? (Required)
Tips for choosing the affected version(s):
By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.
For details, see tips for choosing the affected versions.
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?