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.

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

stmt-reference, sysvars: Document dml-batch-size #3838

Merged
merged 6 commits into from Sep 9, 2020
Merged

stmt-reference, sysvars: Document dml-batch-size #3838

merged 6 commits into from Sep 9, 2020

Conversation

ghost
Copy link

@ghost ghost commented Sep 4, 2020

What is changed, added or deleted? (Required)

Part of #3155

Only relevant to master.

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

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

What is the related PR or file link(s)?

  • This PR is translated from:
  • Other reference link(s):

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Have version specific changes
  • Might cause conflicts

@ghost ghost requested review from kolbe, cfzjywxk and jackysp September 4, 2020 17:17
@ghost
Copy link
Author

ghost commented Sep 4, 2020

It looks like this was removed in #1806 , but it needs to be added back because of the upgrade issue mentioned here :(

@jackysp
Copy link
Member

jackysp commented Sep 5, 2020

lgtm

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 5, 2020
@cfzjywxk
Copy link
Contributor

cfzjywxk commented Sep 5, 2020

LGTM

@ti-srebot
Copy link
Contributor

@cfzjywxk,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIG: docs(slack).

@ghost ghost mentioned this pull request Sep 5, 2020
22 tasks
@TomShawn TomShawn added size/small Changes of a small size. translation/doing This PR's assignee is translating this PR. labels Sep 7, 2020
@TomShawn TomShawn self-requested a review September 7, 2020 02:27
@TomShawn TomShawn self-assigned this Sep 7, 2020
sql-statements/sql-statement-load-data.md Outdated Show resolved Hide resolved
system-variables.md Outdated Show resolved Hide resolved
sql-statements/sql-statement-load-data.md Outdated Show resolved Hide resolved
sql-statements/sql-statement-load-data.md Outdated Show resolved Hide resolved
Null not nil and others added 4 commits September 9, 2020 00:37
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
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-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 9, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 9, 2020
@TomShawn TomShawn merged commit 3cb18ef into pingcap:master Sep 9, 2020
@TomShawn TomShawn assigned ireneontheway and unassigned TomShawn Sep 21, 2020
@ireneontheway ireneontheway 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 Sep 24, 2020

- Scope: SESSION
- Default value: 0
- Example value: 20000
Copy link
Contributor

Choose a reason for hiding this comment

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

@nullnotnil Could you explain what example value is? I find it confusing when reviewing the translation of this PR. From the context, 20000 does not appear in this document but in sql-statement-load-data.md . Did you mean the example in sql-statement-load-data.md ?

Copy link
Author

Choose a reason for hiding this comment

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

I picked 20000 as an example value because this is the prior batch size for LOAD DATA before it was changed to not be batched in master.

I think it is potentially useful for other sysvars, because the min/max values may be a very wide range.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we say In earlier releases of TiDB, the default batch size for LOAD DATA was 20000? Here the example seems a little confusing.

Copy link
Author

Choose a reason for hiding this comment

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

Let's just remove it. I don't think people will know to look up this value when they get an error, they will look up the reference for LOAD DATA, which mentions the previous default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed in #3977.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/small Changes of a small size. 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
Development

Successfully merging this pull request may close these issues.

5 participants