[incubator-doris] branch master updated: [Bug] NPE when using unknown function in broker load process (#3225)

[morningman]

This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 4131afe  [Bug] NPE when using unknown function in broker load process 
(#3225)
4131afe is described below

commit 4131afe316f3354bb7f2b9a6b0ff80066db237e0
Author: Mingyu Chen <morningman....@gmail.com>
AuthorDate: Mon Mar 30 18:34:41 2020 +0800

    [Bug] NPE when using unknown function in broker load process (#3225)
    
    This CL fix the bug described in issue #3224 by
    
    1. Forbid UDF in broker load process
    2. Improving the function checking logic to avoid NPE when trying to
       get default database from ConnectionContext.
---
 .../java/org/apache/doris/analysis/FunctionCallExpr.java     |  5 +++--
 .../org/apache/doris/load/loadv2/LoadingTaskPlanner.java     | 12 +++++++++++-
 .../main/java/org/apache/doris/planner/BrokerScanNode.java   |  6 +-----
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java 
b/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
index 437a2a6..a98d996 100644
--- a/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
@@ -558,7 +558,8 @@ public class FunctionCallExpr extends Expr {
             // find user defined functions
             if (fn == null) {
                 if (!analyzer.isUDFAllowed()) {
-                    throw new AnalysisException("Does not support non-builtin 
functions: " + fnName);
+                    throw new AnalysisException(
+                            "Does not support non-builtin functions, or 
function does not exist: " + this.toSqlImpl());
                 }
 
                 String dbName = fnName.analyzeDb(analyzer);
@@ -579,7 +580,7 @@ public class FunctionCallExpr extends Expr {
         }
 
         if (fn == null) {
-            LOG.warn("fn {} not exists", fnName.getFunction());
+            LOG.warn("fn {} not exists", this.toSqlImpl());
             throw new 
AnalysisException(getFunctionNotFoundError(collectChildReturnTypes()));
         }
 
diff --git 
a/fe/src/main/java/org/apache/doris/load/loadv2/LoadingTaskPlanner.java 
b/fe/src/main/java/org/apache/doris/load/loadv2/LoadingTaskPlanner.java
index d05de48..194c2e4 100644
--- a/fe/src/main/java/org/apache/doris/load/loadv2/LoadingTaskPlanner.java
+++ b/fe/src/main/java/org/apache/doris/load/loadv2/LoadingTaskPlanner.java
@@ -38,6 +38,7 @@ import org.apache.doris.planner.PlanFragment;
 import org.apache.doris.planner.PlanFragmentId;
 import org.apache.doris.planner.PlanNodeId;
 import org.apache.doris.planner.ScanNode;
+import org.apache.doris.qe.ConnectContext;
 import org.apache.doris.thrift.TBrokerFileStatus;
 import org.apache.doris.thrift.TUniqueId;
 
@@ -65,7 +66,8 @@ public class LoadingTaskPlanner {
     private final long timeoutS;    // timeout of load job, in second
 
     // Something useful
-    private Analyzer analyzer = new Analyzer(Catalog.getInstance(), null);
+    // ConnectContext here is just a dummy object to avoid some NPE problem, 
like ctx.getDatabase()
+    private Analyzer analyzer = new Analyzer(Catalog.getInstance(), new 
ConnectContext());
     private DescriptorTable descTable = analyzer.getDescTbl();
 
     // Output params
@@ -86,6 +88,14 @@ public class LoadingTaskPlanner {
         this.strictMode = strictMode;
         this.analyzer.setTimezone(timezone);
         this.timeoutS = timeoutS;
+
+        /*
+         * TODO(cmy): UDF currently belongs to a database. Therefore, before 
using UDF,
+         * we need to check whether the user has corresponding permissions on 
this database.
+         * But here we have lost user information and therefore cannot check 
permissions.
+         * So here we first prohibit users from using UDF in load. If 
necessary, improve it later.
+         */
+        this.analyzer.setUDFAllowed(false);
     }
 
     public void plan(TUniqueId loadId, List<List<TBrokerFileStatus>> 
fileStatusesList, int filesAdded)
diff --git a/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java 
b/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java
index fcb558f..4d0db14 100644
--- a/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java
+++ b/fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java
@@ -163,11 +163,7 @@ public class BrokerScanNode extends LoadScanNode {
             ParamCreateContext context = new ParamCreateContext();
             context.fileGroup = fileGroup;
             context.timezone = analyzer.getTimezone();
-            try {
-                initParams(context);
-            } catch (AnalysisException e) {
-                throw new UserException(e.getMessage());
-            }
+            initParams(context);
             paramCreateContexts.add(context);
         }
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org