[fix](fe) Improve MaxCompute catalog validation#64119
Open
hubgeter wants to merge 1 commit into
Open
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28829 ms |
Contributor
TPC-DS: Total hot run time: 169519 ms |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run buildall |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29310 ms |
Contributor
TPC-DS: Total hot run time: 169725 ms |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I found one blocking issue in the new MaxCompute namespace-schema validation path. The PR otherwise stays focused on MaxCompute catalog validation and unsupported table/view rejection, with targeted unit tests added, but this validation path can reject valid existing deployments that use private/intranet MaxCompute endpoints.
Critical checkpoint conclusions:
- Goal/test: The goal appears to be earlier MaxCompute validation and explicit rejection of unsupported external tables/logical views. The new tests cover local branching and unsupported table/view checks, but not private/intranet endpoint behavior for the new OpenAPI validation.
- Scope/focus: The code change is mostly focused, though it adds a new SDK dependency and a second MaxCompute client path.
- Concurrency/lifecycle: No new shared mutable concurrency path found. Catalog initialization remains synchronized by ExternalCatalog, but the new remote validation runs during lazy initialization.
- Configuration/compatibility: The new OpenAPI validation does not honor the existing mc.endpoint/private endpoint configuration, which is a compatibility regression for namespace-schema catalogs.
- Parallel paths: Read and write unsupported-table checks were both updated.
- Error handling/observability: Errors are surfaced during catalog initialization with useful context, but the incorrect endpoint selection makes the error misleading for private endpoint users.
- Data correctness/transactions/persistence: No transaction visibility or persisted metadata format issue found in the reviewed diff.
- Performance: No hot-path performance issue found; validation runs at initialization.
- Security model/focus: I read SECURITY.md and threat-model.md because this PR touches external catalog connection/auth material. The finding is an operational correctness/compatibility issue, not a security vulnerability under the model. No additional user-provided review focus was present.
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
hubgeter
force-pushed
the
fix_mc_epic
branch
2 times, most recently
from
afb3991 to
d1e407c
Compare
Contributor
Author
|
run buildall |
Contributor
FE Regression Coverage ReportIncrement line coverage |
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
2 participants
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 problem does this PR solve?
Problem Summary:
Add the
mc.validate_connectionparameter, which defaults to false, to perform ak-sk、 project 、schema validation when creating a catalog.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)