Conversation
Document the new Dumpling --filetype parquet support (pingcap/tidb#68345): update the format lists, add an "Export to Parquet files" section with the --parquet-compress/--parquet-page-size/--parquet-row-group-size options, note the --compress incompatibility, and extend the exported-file format and option table.
D3Hunter
added
the
area/dumpling
|
[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 |
ti-chi-bot
Bot
added
missing-translation-status
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe Dumpling overview now documents Parquet as an export format. It adds Parquet export usage, compression and sizing options, file naming details, and updates the command-line option reference. ChangesDumpling Parquet documentation
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Suggested reviewers: Merge Risk: 🔵 Low · up to The Parquet documentation is functionally sound but needs two minor documentation corrections to avoid downstream confusion and CI style findings. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: pingcap/docs/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 5eef9f84-3794-41d9-ad95-f476b3ea2159
📒 Files selected for processing (1)
dumpling-overview.md
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| In the command above: | ||
|
|
||
| - Dumpling exports table data as Parquet files, and still exports the table schema as SQL files. You can use the exported files as the data source of [TiDB Lightning](/tidb-lightning/tidb-lightning-overview.md). | ||
| - You can use the `--parquet-compress` option to specify the compression algorithm for Parquet files. It supports `no-compression`, `snappy`, `gzip`, and `zstd`. The default value is `snappy`. When compression is enabled, the algorithm is added to the file name. For example, with the `snappy` compression, the exported data file is named as `test.t1.000000000.snappy.parquet`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n -C 4 'snappy\.parquet|parquet\.snappy|parquet-compress' --glob '*.go' --glob '*.md'Repository: pingcap/docs
Length of output: 7314
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- tracked relevant files ---'
git ls-files | rg '(^|/)(dumpling|tidb-lightning).*|dumpling-overview\.md'
printf '%s\n' '--- Lightning filename sections ---'
sed -n '300,380p' tidb-lightning/tidb-lightning-data-source.md
printf '%s\n' '--- filename-pattern references ---'
rg -n -C 3 'filename|file name|parquet|compress|schema\}|\{.*csv|sql.*parquet' tidb-lightning/tidb-lightning-data-source.md dumpling-overview.mdRepository: pingcap/docs
Length of output: 35353
🌐 Web query:
official GitHub Dumpling parquet compression filename snappy.parquet tidb-lightning parser
💡 Result:
<source_evidence>
Citations:
- 1: GitHub pull request 68345 in pingcap/tidb (link omitted to avoid creating a cross-reference)
- 2: https://docs.pingcap.com/tidb/stable/tidb-lightning-data-source/
- 3: https://docs.pingcap.com/tidb/dev/tidb-lightning-data-source/
- 4: https://github.com/pingcap/tidb/blob/master/dumpling/export/config.go
🌐 Web query:
site:github.com/pingcap/tidb "snappy.parquet" "parquet-compress"
💡 Result:
<source_evidence>
Citations:
- 1: https://github.com/pingcap/tidb/blob/master/dumpling/export/config.go
- 2: GitHub pull request 68345 in pingcap/tidb (link omitted to avoid creating a cross-reference)
- 3: GitHub pull request 38603 in pingcap/tidb (link omitted to avoid creating a cross-reference)
- 4: GitHub issue 38514 in pingcap/tidb (link omitted to avoid creating a cross-reference)
- 5: GitHub pull request 66949 in pingcap/tidb (link omitted to avoid creating a cross-reference)
- 6: GitHub issue 67849 in pingcap/tidb (link omitted to avoid creating a cross-reference)
🏁 Script executed:
python3 - <<'PY'
import json
import urllib.request
url = "https://api.github.com/repos/pingcap/tidb/pulls/68345/files?per_page=100"
request = urllib.request.Request(url, headers={"Accept": "application/vnd.github+json", "User-Agent": "review-inspection"})
with urllib.request.urlopen(request, timeout=20) as response:
files = json.load(response)
for item in files:
patch = item.get("patch", "")
text = (item.get("filename", "") + "\n" + patch).lower()
if any(term in text for term in ("parquet", "filename", "extension", "compress")):
print(f"--- {item['filename']} ---")
print(patch)
PYRepository: pingcap/docs
Length of output: 2436
🏁 Script executed:
set -e
curl -kfsSL -H 'Accept: application/vnd.github+json' -H 'User-Agent: review-inspection' \
'https://api.github.com/repos/pingcap/tidb/pulls/68345/files?per_page=100' |
python3 -c '
import json, sys
for item in json.load(sys.stdin):
patch = item.get("patch", "")
text = (item.get("filename", "") + "\n" + patch).lower()
if any(term in text for term in ("parquet", "filename", "extension", "compress")):
print("--- " + item["filename"] + " ---")
print(patch)
'Repository: pingcap/docs
Length of output: 33228
Clarify the native Parquet filename exception in the TiDB Lightning documentation.
Dumpling emits test.t1.000000000.snappy.parquet, not test.t1.000000000.parquet.snappy. TiDB Lightning has a dedicated route for native Parquet compression and matches the snappy.parquet form before the generic compressed-file route. Keep the Dumpling examples unchanged.
The generic pattern in tidb-lightning/tidb-lightning-data-source.md:27 conflicts with this behavior and with its note at line 368. Limit that pattern to CSV and SQL files, or document the native Parquet exception explicitly.
Comment on lines
+447
to
+448
| | `--parquet-page-size` | The data page size of Parquet files. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 1MiB | | ||
| | `--parquet-row-group-size` | The row group memory limit of Parquet files, which is the flush threshold by accounted in-memory bytes. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 120MiB | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n --hidden -S 'PingCAP\.Units|Vale|vale|non.?breaking|Units' . --glob '!vendor/**' --glob '!node_modules/**'
sed -n '430,455p' dumpling-overview.mdRepository: pingcap/docs
Length of output: 41016
🤖 get_repo_knowledge executed:
get_repo_knowledge pingcap/docs /tmp/coderabbit-repo-knowledge/pingcap-docs-9c73f70c/conventions /tmp/coderabbit-repo-knowledge/pingcap-docs-9c73f70c/learnings
Length of output: 4452
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- relevant tracked files ---'
git ls-files | rg '(^|/)(\.vale\.ini|vale|Vale|markdownlint|dumpling-overview\.md|ci\.yaml)$|styles/PingCAP/Units\.yml|scripts/' | head -120
printf '%s\n' '--- .vale.ini ---'
if [ -f .vale.ini ]; then cat -n .vale.ini; else printf '%s\n' 'missing'; fi
printf '%s\n' '--- Units rule ---'
if [ -f styles/PingCAP/Units.yml ]; then cat -n styles/PingCAP/Units.yml; else printf '%s\n' 'missing'; fi
printf '%s\n' '--- Vale references in scripts/workflow ---'
rg -n -S 'vale|Vale|PingCAP\.Units|stylesPath|BasedOnStyles|formats|glob|exclude' scripts .github .vale.ini styles 2>/dev/null | head -200
printf '%s\n' '--- target lines and nearby Markdown unit examples ---'
cat -n dumpling-overview.md | sed -n '420,455p'
printf '%s\n' '--- relevant Markdown guidance/config ---'
rg -n -S 'Markdown|Vale|nonbreaking|non-breaking|unit|nbsp' README.md CONTRIBUTING.md .github resources scripts 2>/dev/null | head -200Repository: pingcap/docs
Length of output: 39267
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- CI Vale job ---'
cat -n .github/workflows/ci.yaml | sed -n '55,78p'
printf '%s\n' '--- changed-file Vale plumbing ---'
rg -n -S 'vale-action|vale |Vale|extract-changed-markdown|changed.*md|\.md' .github/workflows .github/scripts scripts 2>/dev/null | head -120
printf '%s\n' '--- repository nonbreaking-space examples ---'
rg -n -S ' |&`#160`;|U\+00A0' --glob '*.md' --glob '*.mdx' . | head -80
printf '%s\n' '--- all target literals in the file ---'
rg -n -F -e '1MiB' -e '120MiB' dumpling-overview.mdRepository: pingcap/docs
Length of output: 15224
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- nearby Parquet section ---'
cat -n dumpling-overview.md | sed -n '155,175p'
printf '%s\n' '--- targeted diff for dumpling-overview.md ---'
git diff --unified=2 -- dumpling-overview.md | rg -n -C 3 'parquet|MiB|@@' | head -180Repository: pingcap/docs
Length of output: 2022
Add nonbreaking spaces to the Parquet size defaults.
PingCAP.Units applies to this Markdown table and flags 1MiB and 120MiB. Use between each number and unit.
Committable replacement
-| `--parquet-page-size` | The data page size of Parquet files. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 1MiB |
-| `--parquet-row-group-size` | The row group memory limit of Parquet files, which is the flush threshold by accounted in-memory bytes. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 120MiB |
+| `--parquet-page-size` | The data page size of Parquet files. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 1 MiB |
+| `--parquet-row-group-size` | The row group memory limit of Parquet files, which is the flush threshold by accounted in-memory bytes. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 120 MiB |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `--parquet-page-size` | The data page size of Parquet files. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 1MiB | | |
| | `--parquet-row-group-size` | The row group memory limit of Parquet files, which is the flush threshold by accounted in-memory bytes. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 120MiB | | |
| | `--parquet-page-size` | The data page size of Parquet files. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 1 MiB | | |
| | `--parquet-row-group-size` | The row group memory limit of Parquet files, which is the flush threshold by accounted in-memory bytes. The unit must be specified such as `128B`, `64KiB`, `32MiB`, and `1.5GiB`. | 120 MiB | |
🧰 Tools
🪛 GitHub Check: vale
[failure] 448-448:
[vale] reported by reviewdog 🐶
[PingCAP.Units] Put a nonbreaking space between the number and the unit in '120MiB'.
Raw Output:
{"message":"[PingCAP.Units] Put a nonbreaking space between the number and the unit in '120MiB'.","location":{"path":"dumpling-overview.md","range":{"start":{"line":448,"column":213},"end":{"line":448,"column":219}}},"severity":"ERROR","code":{"value":"PingCAP.Units","url":"https://developers.google.com/style/units-of-measure"}}
[failure] 447-447:
[vale] reported by reviewdog 🐶
[PingCAP.Units] Put a nonbreaking space between the number and the unit in '1MiB'.
Raw Output:
{"message":"[PingCAP.Units] Put a nonbreaking space between the number and the unit in '1MiB'.","location":{"path":"dumpling-overview.md","range":{"start":{"line":447,"column":141},"end":{"line":447,"column":145}}},"severity":"ERROR","code":{"value":"PingCAP.Units","url":"https://developers.google.com/style/units-of-measure"}}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is changed, added, or deleted? (Required)
Document the new Dumpling feature that exports table data in Parquet format (implemented in pingcap/tidb#68345) in
dumpling-overview.md:--filetype parquetexample, and document the related options:--parquet-compress(defaultsnappy; supportsno-compression,snappy,gzip, andzstd)--parquet-page-size(default1MiB)--parquet-row-group-size(default120MiB)--compressoption is not supported for Parquet files.{schema}.{table}.{0001}.{sql|csv|parquet}, including the compression suffix in Parquet file names (for example,test.t1.000000000.snappy.parquet).--filetypedescription to(csv/sql/parquet).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)?
AI agent involvement
Do your changes match any of the following descriptions?
Summary by CodeRabbit