include compatibility notice for tidb_remove_orderby_in_subquery (#20684) #20979
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Grace Cai <qqzczy@126.com>
ti-chi-bot
added
lgtm
size/S
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/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
Hello! Gemini here, providing a summary of this pull request for a quick overview.
This PR, titled "include compatibility notice for tidb_remove_orderby_in_subquery (#20684)", is an automated cherry-pick aimed at updating the documentation for the 7.2.0 release notes. The primary goal is to add a crucial compatibility notice regarding the change in the default value of the tidb_remove_orderby_in_subquery system variable. Additionally, it includes minor updates to several links within the same document, directing them to the archived documentation site.
Highlights
- Documentation Update: Adds a new entry under the 'Behavior changes' section in the
releases/release-7.2.0.mdfile to document the change in the default value of thetidb_remove_orderby_in_subquerysystem variable fromOFFtoON. This change, introduced in v6.1.0 but documented here for 7.2.0, affects how the optimizer handlesORDER BYclauses in subqueries, potentially changing the order of results if not explicitly handled in the outer query. - Link Updates: Updates several internal links within the
releases/release-7.2.0.mdfile to point to thedocs-archive.pingcap.comdomain instead of the maindocs.pingcap.comdomain. This is likely to ensure links in older release notes point to the correct, potentially static, archived versions of the documentation.
Changelog
- releases/release-7.2.0.md
- Added a new bullet point under 'Behavior changes' to document the default value change of
tidb_remove_orderby_in_subqueryand its implications (Diff 3, lines 176-177). - Updated the 'Quick access' link to point to
docs-archive.pingcap.com(Diff 1, line 12). - Updated links for 'managing runaway queries', 'pipeline execution model', 'IMPORT INTO', and 'pause and resume operations' to point to
docs-archive.pingcap.com(Diff 2, lines 27, 32, 37, 42). - Updated the link for
tidb_enable_tiflash_pipeline_modelto point todocs-archive.pingcap.com(Diff 4, line 188).
- Added a new bullet point under 'Behavior changes' to document the default value change of
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 pull request correctly updates URLs to point to docs-archive for the v7.2.0 release notes, which is standard practice for older versions. More importantly, it adds a clear and comprehensive compatibility notice regarding the change in the default value for tidb_remove_orderby_in_subquery.
The added notice is well-structured, explaining the change, its implications, the rationale (SQL standard compliance), and guidance for users who need to maintain previous behavior or ensure specific ordering. The information about how upgrades are handled is also crucial and well-stated.
Overall, the changes enhance the accuracy and completeness of the release notes. I have one minor suggestion for phrasing to improve flow slightly.
Summary of Findings
- URL Updates: Links in the v7.2.0 release notes have been correctly updated to point to
docs-archive.pingcap.com, reflecting that this is an archived version. - Compatibility Notice for
tidb_remove_orderby_in_subquery: A new behavior change notice has been added fortidb_remove_orderby_in_subquery. The notice clearly explains the change in default value, its impact, rationale, and how users can manage it. This is a valuable addition for users. - Clarity of Documentation: The added documentation is clear, concise, and provides necessary details for users to understand the behavior change. A minor phrasing suggestion was made to potentially improve flow.
Merge Readiness
The pull request makes necessary updates to URLs and adds an important compatibility notice. The changes are well-implemented and clear. With the minor phrasing suggestion considered, the PR appears to be in good shape for merging. As per the project's style guide, I am providing comments rather than requesting changes for minor issues to avoid blocking the PR. Other reviewers should make the final approval decision.
| @@ -173,6 +173,7 @@ Quick access: [Quick start](https://docs.pingcap.com/tidb/v7.2/quick-start-with- | |||
| ### Behavior changes | |||
|
|
|||
| - When processing update event, TiCDC splits an event into delete and insert events if the primary key or non-null unique index value is modified in the event. For more information, see [documentation](/ticdc/ticdc-split-update-behavior.md#transactions-containing-a-single-update-change). | |||
| - Changes the default value of [`tidb_remove_orderby_in_subquery`](/system-variables.md#tidb_remove_orderby_in_subquery-new-in-v610) from `OFF` to `ON`, meaning that the optimizer removes `ORDER BY` clauses from subqueries to avoid unnecessary sorting operations. This change might result in a different order of rows in query results. The ISO/IEC SQL standard does not require query results to follow the `ORDER BY` sorting specified in subqueries. If you need to ensure a specific order in the final result, add an `ORDER BY` clause to the outer query. If your applications rely on subquery sorting, you can set this variable to `OFF`. Clusters upgraded from earlier versions retain the previous behavior by default. | |||
There was a problem hiding this comment.
The explanation for the behavior change is clear and informative. Consider a slight rephrasing of the initial consequence for improved flow and emphasis. The current phrasing "meaning that the optimizer removes" is acceptable, but could be a bit more direct.
For example, explicitly stating that this is the new default behavior and using "This means" can make the sentence flow a bit better. 1
Also, ensure that the link /system-variables.md#tidb_remove_orderby_in_subquery-new-in-v610 correctly resolves within the context of the v7.2 documentation. Relative links like this are generally fine if they point to a page within the same version's documentation set.
Style Guide References
| - Changes the default value of [`tidb_remove_orderby_in_subquery`](/system-variables.md#tidb_remove_orderby_in_subquery-new-in-v610) from `OFF` to `ON`, meaning that the optimizer removes `ORDER BY` clauses from subqueries to avoid unnecessary sorting operations. This change might result in a different order of rows in query results. The ISO/IEC SQL standard does not require query results to follow the `ORDER BY` sorting specified in subqueries. If you need to ensure a specific order in the final result, add an `ORDER BY` clause to the outer query. If your applications rely on subquery sorting, you can set this variable to `OFF`. Clusters upgraded from earlier versions retain the previous behavior by default. | |
| - Changes the default value of [`tidb_remove_orderby_in_subquery`](/system-variables.md#tidb_remove_orderby_in_subquery-new-in-v610) from `OFF` to `ON`. This means the optimizer now, by default, removes `ORDER BY` clauses from subqueries to avoid unnecessary sorting operations. This change might result in a different order of rows in query results. The ISO/IEC SQL standard does not require query results to follow the `ORDER BY` sorting specified in subqueries. If you need to ensure a specific order in the final result, add an `ORDER BY` clause to the outer query. If your applications rely on subquery sorting, you can set this variable to `OFF`. Clusters upgraded from earlier versions retain the previous behavior by default. |
Footnotes
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hfxsd The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is an automated cherry-pick of #20684
What is changed, added or deleted? (Required)
Please refer to pingcap/docs-cn#20139
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?