Dmitry Lychagin has posted comments on this change. Change subject: [ASTERIXDB-2180][FUN] Prevent dropping of entities used by functions ......................................................................
Patch Set 11: (7 comments) https://asterix-gerrit.ics.uci.edu/#/c/2253/11/asterixdb/LICENSE File asterixdb/LICENSE: Line 276: --- Why license changes are part of this change? Can we revert this file? https://asterix-gerrit.ics.uci.edu/#/c/2253/11/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java: Line 1711: Set<CallExpr> functionCalls = Can we refactor this block into a separate helper method in FunctionUtil? We already have retrieveUsedStoredFunctions() there, at some point we should see whether we can align them Line 1724: if (fid.equals(BuiltinFunctions.DATASET)) { What about other datasource functions ('feed-collect', etc) ? Are we ignoring them here? PS11, Line 1727: dependencies.get(0). Instead of dependencies.get(0) and (1) I'd create two lists 'datasourceDependencies' and 'functionDependencies', populate them in this loop and then create the final 'dependencies' list at the end. Makes it easier to understand what (0) and (1) correspond to. PS11, Line 1727: new ArrayList<> Arrays.asList() already creates a List, why copy it again? Line 1731: CommonFunctionMapUtil.normalizeBuiltinFunctionSignature(signature), false)) { Why CommonFunctionMapUtil.normalizeBuiltinFunctionSignature() is necessary here? The query rewriter was supposed to call it already to normalize built-in function calls (see SqlppQueryRewriter.rewriteFunctionNames()) PS11, Line 1734: new ArrayList<> Arrays.asList() already creates a List, why copy it again? -- To view, visit https://asterix-gerrit.ics.uci.edu/2253 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f08ff150dfd57432b88381c507814ddb57bd67b Gerrit-PatchSet: 11 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Ildar Absalyamov <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
