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

Reply via email to