chore: add code about hack and debug#9
Conversation
58dc382
into
feat/nested-projection-support-epic
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a17e49a6d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn scan_region(&self, region_id: RegionId, request: ScanRequest) -> Result<ScanRegion> { | ||
| fn scan_region(&self, region_id: RegionId, mut request: ScanRequest) -> Result<ScanRegion> { | ||
| let query_start = Instant::now(); | ||
| Self::hack_scan_req(&mut request); |
There was a problem hiding this comment.
Calling hack_scan_req on every scan_region mutates request.projection_input for all scans whenever GREPTIME_PROJECTION_NESTED_PATHS is present, so one debug env setting can silently replace the planner-provided projection for unrelated queries. In that mode, the engine can return a different column set/schema than the query planned for, which can produce incorrect results or runtime schema-mismatch failures; this should be gated to explicit debug builds/paths instead of the default scan path.
Useful? React with 👍 / 👎.
| let parquet_read_cols = read_format.parquet_read_columns(); | ||
| let leaf_indices = build_parquet_leaves_indices(parquet_schema_desc, &parquet_read_cols); | ||
| // TODO(fys): remove it later. | ||
| common_telemetry::info!("leaf_indices: {:?}", leaf_indices); |
There was a problem hiding this comment.
This adds an unconditional info! log for leaf_indices during parquet reader construction, which runs for normal scans and compactions; on wide schemas and many files this emits large per-file logs and adds avoidable CPU/I/O overhead in production. Because this is debug-only data, it should be removed or downgraded behind a debug/trace guard.
Useful? React with 👍 / 👎.
1 participant
I hereby agree to the terms of the GreptimeDB CLA
What's changed and what's your intention?
PLEASE DO NOT LEAVE THIS EMPTY !!!
Please explain IN DETAIL what the changes are in this PR and why they are needed:
Checklist
Refer to a related PR or issue link (optional)