Move accessor permission check to InterpreterFactory::get, make the denied query won't insert into the query_log

Author: BohuTANG

src/query/service/src/interpreters/access/accessor.rs

@@ -42,16 +42,16 @@ impl Accessor {
         Accessor { accessors }
     }
 
-    pub fn check(&self, new_plan: &Option<Plan>, plan: &PlanNode) -> Result<()> {
+    pub fn check(&self, plan: &PlanNode) -> Result<()> {
         for accessor in self.accessors.values() {
-            match new_plan {
-                None => {
-                    accessor.check(plan)?;
-                }
-                Some(new) => {
-                    accessor.check_new(new)?;
-                }
-            }
+            accessor.check(plan)?;
+        }
+        Ok(())
+    }
+
+    pub fn check_new(&self, plan: &Plan) -> Result<()> {
+        for accessor in self.accessors.values() {
+            accessor.check_new(plan)?;
         }
         Ok(())
     }

src/query/service/src/interpreters/interpreter_factory.rs

@@ -18,6 +18,7 @@ use common_exception::ErrorCode;
 use common_exception::Result;
 use common_legacy_planners::PlanNode;
 
+use crate::interpreters::access::Accessor;
 use crate::interpreters::DeleteInterpreter;
 use crate::interpreters::EmptyInterpreter;
 use crate::interpreters::ExplainInterpreter;
@@ -35,10 +36,14 @@ pub struct InterpreterFactory;
 /// Such as: SelectPlan -> SelectInterpreter, ExplainPlan -> ExplainInterpreter, ...
 impl InterpreterFactory {
     pub fn get(ctx: Arc<QueryContext>, plan: PlanNode) -> Result<Arc<dyn Interpreter>> {
+        // Check the access permission.
+        let access_checker = Accessor::create(ctx.clone());
+        access_checker.check(&plan)?;
+
         let inner = Self::create_interpreter(ctx.clone(), &plan)?;
         let query_kind = plan.name().to_string();
         Ok(Arc::new(InterceptorInterpreter::create(
-            ctx, inner, plan, None, query_kind,
+            ctx, inner, query_kind,
         )))
     }
 

src/query/service/src/interpreters/interpreter_factory_interceptor.rs

@@ -17,13 +17,11 @@ use std::time::SystemTime;
 
 use common_datavalues::DataSchemaRef;
 use common_exception::Result;
-use common_legacy_planners::PlanNode;
 use common_streams::ErrorStream;
 use common_streams::ProgressStream;
 use common_streams::SendableDataBlockStream;
 use parking_lot::Mutex;
 
-use crate::interpreters::access::Accessor;
 use crate::interpreters::Interpreter;
 use crate::interpreters::InterpreterPtr;
 use crate::interpreters::InterpreterQueryLog;
@@ -32,34 +30,21 @@ use crate::pipelines::SourcePipeBuilder;
 use crate::sessions::QueryContext;
 use crate::sessions::SessionManager;
 use crate::sessions::TableContext;
-use crate::sql::plans::Plan;
 
 pub struct InterceptorInterpreter {
     ctx: Arc<QueryContext>,
-    plan: PlanNode,
-    new_plan: Option<Plan>,
     inner: InterpreterPtr,
     query_log: InterpreterQueryLog,
     source_pipe_builder: Mutex<Option<SourcePipeBuilder>>,
-    accessor_checker: Accessor,
 }
 
 impl InterceptorInterpreter {
-    pub fn create(
-        ctx: Arc<QueryContext>,
-        inner: InterpreterPtr,
-        plan: PlanNode,
-        new_plan: Option<Plan>,
-        query_kind: String,
-    ) -> Self {
+    pub fn create(ctx: Arc<QueryContext>, inner: InterpreterPtr, query_kind: String) -> Self {
         InterceptorInterpreter {
             ctx: ctx.clone(),
-            plan,
-            new_plan,
             inner,
-            query_log: InterpreterQueryLog::create(ctx.clone(), query_kind),
+            query_log: InterpreterQueryLog::create(ctx, query_kind),
             source_pipe_builder: Mutex::new(None),
-            accessor_checker: Accessor::create(ctx),
         }
     }
 }
@@ -75,9 +60,6 @@ impl Interpreter for InterceptorInterpreter {
     }
 
     async fn execute(&self, ctx: Arc<QueryContext>) -> Result<SendableDataBlockStream> {
-        // Access check.
-        self.accessor_checker.check(&self.new_plan, &self.plan)?;
-
         let _ = self
             .inner
             .set_source_pipe_builder((*self.source_pipe_builder.lock()).clone());

src/query/service/src/interpreters/interpreter_factory_v2.rs

@@ -15,12 +15,11 @@
 use std::sync::Arc;
 
 use common_exception::Result;
-use common_legacy_planners::EmptyPlan;
-use common_legacy_planners::PlanNode;
 
 use super::interpreter_share_desc::DescShareInterpreter;
 use super::interpreter_user_stage_drop::DropUserStageInterpreter;
 use super::*;
+use crate::interpreters::access::Accessor;
 use crate::interpreters::interpreter_copy_v2::CopyInterpreterV2;
 use crate::interpreters::interpreter_presign::PresignInterpreter;
 use crate::interpreters::interpreter_table_create_v2::CreateTableInterpreterV2;
@@ -44,13 +43,15 @@ impl InterpreterFactoryV2 {
     }
 
     pub fn get(ctx: Arc<QueryContext>, plan: &Plan) -> Result<InterpreterPtr> {
+        // Check the access permission.
+        let access_checker = Accessor::create(ctx.clone());
+        access_checker.check_new(plan)?;
+
         let inner = InterpreterFactoryV2::create_interpreter(ctx.clone(), plan)?;
 
         Ok(Arc::new(InterceptorInterpreter::create(
             ctx,
             inner,
-            PlanNode::Empty(EmptyPlan::create()),
-            Some(plan.clone()),
             plan.to_string(),
         )))
     }