Repository navigation
[bug](avro) fix avro jni reader coredump with nullptr - #64699
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 11:44 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review completed for PR 64699.
Summary opinion: I found no substantiated blocking issue in this PR. The patch only guards insertion of time_zone in JniConnector::open() when RuntimeState* is null. The null-state path is the Avro schema-only reader, whose constructor sets _is_table_schema; Java Avro schema parsing does not consume time_zone, and normal scan paths still call open(_state, _profile) with a non-null state.
Critical checkpoints:
- Changed files: GitHub and local diff both report only
be/src/exec/connector/jni_connector.cpp. - Existing review context: no existing inline review comments or replies were present.
- Duplicate suppression: no prior inline thread or ledger candidate covered an issue requiring suppression.
- Correctness: no evidence of changed scan semantics outside the null-state Avro schema-fetch crash path.
- Session/config propagation: normal JNI scan paths still receive
time_zone; schema-only Avro parsing does not require it. - Tests/style:
git diff --checkon the changed file passed. I did not run a full build or regression suite because the patch is a narrow null guard.
Subagent conclusions:
optimizer-rewrite: no candidate found; convergence round 1 returnedNO_NEW_VALUABLE_FINDINGS.tests-session-config: no candidate found; convergence round 1 returnedNO_NEW_VALUABLE_FINDINGS.- No subagent candidates became inline comments, and none were dismissed as duplicates. Main dismissals are recorded in
.code-review.r4svrC/subagent_review_findings.md.
User focus: no additional user-provided review focus was supplied.
Review instruction note: no repository-local SKILL.md code-review skill was present in this checkout, so the review followed the supplied .code-review.r4svrC/review_prompt.txt instructions and repository evidence.
|
run buildall |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…4699) (#65512) cherry-pick from master #64699 (comment)
…69849) ### What problem does this PR solve? Issue Number: None Related PR: #69601, #64699 Problem Summary: **In short.** #69601 made FE reject `format=avro` while it parses file format properties. This PR adds end-to-end coverage for that: a regression suite that runs `s3()` and `hdfs()` with `format=avro` and checks that they fail during analysis, and a unit test for the two table-function constructors. Test only, no code change. **Background.** Master no longer reads standalone Avro files: #61141 removed the BE reader, #65733 the scanner module, and #69601 made `FileFormatProperties.createFileFormatProperties` throw `Avro file reads are no longer supported`. Before #69601, FE accepted the format and the query failed later in BE with `Not supported file format in fetch table schema: 12`. branch-4.2 still reads Avro through the JNI scanner (#64699 fixed a crash in it), so users who upgrade from 4.x with Avro table functions or loads get this error. **What was missing.** #69601's unit test calls `createFileFormatProperties` directly. Nothing checks that a table function reaches that check before it validates storage properties or lists files. If a constructor validated the location first, `format=avro` against an unreachable location would report a storage error instead, and no test would notice. **What this PR adds.** - `external_table_p0/tvf/test_tvf_avro_format_rejected`: `s3()` and `hdfs()` with `format` set to `avro` and to `AVRO`, against a made-up bucket, made-up credentials and an HDFS address nothing listens on. Each call must fail with `Avro file reads are no longer supported`. - `UnsupportedExternalReadsTest#testAvroTableFunctionsAreRejectedBeforeStorageAccess`: constructs `S3TableValuedFunction` and `HdfsTableValuedFunction` with `format=avro` / `AVRO` and checks the same message. **Results.** On master `2a4191c1dc9` with this change, all four statements of the suite fail during analysis with `Can not build s3(): Avro file reads are no longer supported` / `Can not build hdfs(): ...`, and the unit test passes. Before #69601 the same `s3()` call would have gone on to validate and list the made-up location, so the suite pins the order as well as the message.
What problem does this PR solve?
Problem Summary:
when fetch_table_schema call by FE rpc, will not pass runtime state object, so it's nullptr.
master branch have been refactor so it's no problem.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)