chore: support [<schema>.]<pipeline_name> for cross reference pipeline
#6141
Conversation
|
Do we really want to make pipeline isolated by schema? Do you think it's a good idea to make pipeline namespace agnostic? Any pipeline in the catalog can be accessed by any schema/namespace |
| @@ -54,12 +54,11 @@ impl PipelineHandler for Instance { | |||
|
|
|||
| async fn get_pipeline( | |||
| &self, | |||
| name: &str, | |||
| version: PipelineVersion, | |||
| pipeline_name: &PipelineName, | |||
There was a problem hiding this comment.
I prefer separating the name and version in code. If a combined representation is necessary, consider using the format schema.pipeline:{version}.
| /// The optional schema is for cross-schema reference. | ||
| /// Note the version can be None while query, which means the latest version of the pipeline. | ||
| #[derive(Debug, Clone)] | ||
| pub struct PipelineName { |
There was a problem hiding this comment.
PipelineIdentity is the more appropriate term.
| } | ||
| ); | ||
|
|
||
| let parts = name.split('.').collect::<Vec<&str>>(); |
There was a problem hiding this comment.
Do we need to consider quoted names such as "test.db"."test pipeline"?
There was a problem hiding this comment.
Looks like we need to process it like
greptimedb/src/session/src/table_name.rs
Line 43 in 56e696b
Use sql parser to parse the name into ObjectName at first.
| Ok(()) | ||
| } | ||
|
|
||
| pub fn check_schema(&self, ctx_schema: &str) -> Result<()> { |
There was a problem hiding this comment.
Adds a document for public functions.
Notice that it is only used when adding pipelines.
| compiled_pipeline.clone(), | ||
| self.original_pipelines.insert( | ||
| generate_pipeline_cache_key(&pipeline_name), | ||
| (pipeline.to_owned(), version), |
There was a problem hiding this comment.
Prefer clone over to_owned. Apply this suggestion throughout the file.
| assert_eq!(res.status(), StatusCode::BAD_REQUEST); | ||
|
|
||
| // cross ref using public's pipeline | ||
| let res = client |
There was a problem hiding this comment.
Should pipeline permissions for cross-database access be checked, or does the database access plugin already handle this? If not, should we intercept the request?
|
Adopt removing the schema in pipeline info instead of using the schema and allowing cross-schema reference. |
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
Introduce
PipelineNameto hold and reference the pipeline using name, schema, and version.Now we can use
schema.pipeline_nameto refer to pipelines from other databases while ingesting logs.PR Checklist
Please convert it to a draft if some of the following conditions are not met.