You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/21 14:18:57 UTC
[GitHub] [doris] morningman opened a new pull request, #10320: [feature-wip](multi-catalog) refactor catalog interface
morningman opened a new pull request, #10320:
URL: https://github.com/apache/doris/pull/10320
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dujl commented on pull request #10320: [feature-wip](multi-catalog) refactor catalog interface
Posted by GitBox <gi...@apache.org>.
dujl commented on PR #10320:
URL: https://github.com/apache/doris/pull/10320#issuecomment-1163887617
Database db = Catalog.getCurrentCatalog().getDbOrAnalysisException(label.getDbName());
Database db =
Catalog.getCurrentInternalCatalog().getDbOrAnalysisException(label.getDbName());
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] SaintBacchus commented on pull request #10320: [feature-wip](multi-catalog) refactor catalog interface
Posted by GitBox <gi...@apache.org>.
SaintBacchus commented on PR #10320:
URL: https://github.com/apache/doris/pull/10320#issuecomment-1162697485
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10320: [feature-wip](multi-catalog) refactor catalog interface
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10320:
URL: https://github.com/apache/doris/pull/10320#issuecomment-1165410874
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on pull request #10320: [feature-wip](multi-catalog) refactor catalog interface
Posted by GitBox <gi...@apache.org>.
morningman commented on PR #10320:
URL: https://github.com/apache/doris/pull/10320#issuecomment-1165140701
> Why invoke getCurrentInternalCatalog outside the catalog such as in InsertStmt? getCurrentInternalCatalog should be an internal function in catalo. In insertStmt we should get a DatabaseIf or TableIf from catalog and add a datasource checker for check whether is it a supported datasource table.
>
> disadvantage:
>
> 1. Catalog.getCurrentInternalCatalog(). getDbOrDdlException will throw an exception even though the db is exist in some external datasource. This will confuses users. Such as in CreateRoutineLoadStmt.java, When create routineLoad job for an external datasource table, It should throw do not support external datasource table instead of table not exist.
> 2. catalog api caller such analyzer just want to resolve the db or table identifier instead of perceive the catalog internal interface such getCurrentInternalCatalog.
>
> Suggestion:
>
> 1. catalog api caller such analyzer use Catalog. getDbOrDdlException apis to get a DatabaseIf or TableIf object.
> 2. In catalog internal, it can use current datasource or catalog indicator from api parameter to get a Database or table object.
> 3. if we support insert external datasource table in the future, just remove the datasource checker in InsertStmt.
1. First, I deleted the `getDbXXX` methods from `Catalog.java`, because in essence these `getDb` methods should be the methods of DataSourceMgr. The `Catalog` class will be renamed later, such as `Env` or `DorisRuntime`. It is only responsible for managing all Mgrs, and each Mgr is responsible for its own affairs, such as `DataSourceMgr` is responsible for managing `DataSource` (subsequently renamed `Catalog`) and `getDb`.
2. All are currently modified to `getCurrentInternalCatalog`. Mainly because the function of multi catalog is still under development, and there is currently no context of external catalog, so I first use `InternalCatalog` to ensure that the existing functions are completely unaffected.
All subsequent `Catalog.getCurrentInternalCatalog().getDbxxx` will be uniformly changed to, for example, `Env.getCatalog().getDbxxx()`.
Currently this is only an intermediate state, the purpose is to remove some methods in the Catalog.
3. Whether a function supports the external catalog will be determined later in the analyze phase of Stmt. Then, when the switch catalog is developed, you can obtain the context of the catalog through the context for unified judgment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10320: [feature-wip](multi-catalog) refactor catalog interface
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10320:
URL: https://github.com/apache/doris/pull/10320#issuecomment-1166230408
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei merged pull request #10320: [feature-wip](multi-catalog) refactor catalog interface
Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #10320:
URL: https://github.com/apache/doris/pull/10320
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10320: [feature-wip](multi-catalog) refactor catalog interface
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10320:
URL: https://github.com/apache/doris/pull/10320#issuecomment-1165410896
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org