Ali Alsuliman has posted comments on this change. Change subject: [NO ISSUE][OTH] Support log redaction ......................................................................
Patch Set 8: (8 comments) https://asterix-gerrit.ics.uci.edu/#/c/3105/7/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java: Line 225: throw new CompilationException(ErrorCode.COMPILATION_ERROR, a2.getSourceLocation(), > The concern is that topOp.toString() can have user data? I'd just remove to I just removed it. My point was redacting this topOp is useless even though it might contain user data because toString() is not overridden in logical operators and this would just produce the operator class/object name. https://asterix-gerrit.ics.uci.edu/#/c/3105/7/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushFieldAccessRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushFieldAccessRule.java: Line 285: "Field access " + assignOp.getExpressions().get(0).getValue() + " doesn't correspond to any input"); > same comment. remove inputOp and just rely on the source location. or alter Done https://asterix-gerrit.ics.uci.edu/#/c/3105/7/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java: Line 336: + ((AbstractFunctionCallExpression) aggFuncExpr).getFunctionIdentifier().getName() > let's replace with the function name Done https://asterix-gerrit.ics.uci.edu/#/c/3105/7/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SweepIllegalNonfunctionalFunctions.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SweepIllegalNonfunctionalFunctions.java: Line 110: } > let's remove "in op" + op? source location and a function name should be su Done https://asterix-gerrit.ics.uci.edu/#/c/3105/7/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractLogicalOperator.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractLogicalOperator.java: Line 168: if (physicalOperator == null) { > theres no need to print the whole operator here. Source location and Logica Done https://asterix-gerrit.ics.uci.edu/#/c/3105/7/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/jobgen/impl/JobBuilder.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/jobgen/impl/JobBuilder.java: Line 316: throw AlgebricksException.create(DESCRIPTOR_GENERATION_ERROR, op.getSourceLocation(), op.getOperatorTag()); > let's print source location and LogicalOperatorTag instead. Done https://asterix-gerrit.ics.uci.edu/#/c/3105/7/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java: Line 227: throw AlgebricksException.create(ORDER_EXPR_NOT_NORMALIZED, e.getSourceLocation()); > source location should be sufficient instead of the whole expression. It's Done https://asterix-gerrit.ics.uci.edu/#/c/3105/7/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java: Line 120: + opBuild.getOperatorTag() + ": " + v + "\n"); > we're printing the whole operator here, so seem that we need to redact it. I just replaced it with the operator tag since it's almost the same as the toString() method. -- To view, visit https://asterix-gerrit.ics.uci.edu/3105 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I602c833ba2a055da8fbe8782ec62be683ff4581b Gerrit-PatchSet: 8 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Murtadha Hubail <mhub...@apache.org> Gerrit-HasComments: Yes