ihuzenko commented on a change in pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362497334
########## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectMemoryManager.java ########## @@ -42,307 +44,310 @@ import java.util.Map; /** - * - * ProjectMemoryManager(PMM) is used to estimate the size of rows produced by ProjectRecordBatch. - * The PMM works as follows: - * - * Setup phase: As and when ProjectRecordBatch creates or transfers a field, it registers the field with PMM. - * If the field is a variable width field, PMM records the expression that produces the variable - * width field. The expression is a tree of LogicalExpressions. The PMM walks this tree of LogicalExpressions - * to produce a tree of OutputWidthExpressions. The widths of Fixed width fields are just accumulated into a single - * total. Note: The PMM, currently, cannot handle new complex fields, it just uses a hard-coded estimate for such fields. - * - * - * Execution phase: Just before a batch is processed by Project, the PMM walks the tree of OutputWidthExpressions - * and converts them to FixedWidthExpressions. It uses the RecordBatchSizer and the function annotations to do this conversion. - * See OutputWidthVisitor for details. + * ProjectMemoryManager(PMM) is used to estimate the size of rows produced by + * ProjectRecordBatch. The PMM works as follows: + * <p> + * Setup phase: As and when ProjectRecordBatch creates or transfers a field, it + * registers the field with PMM. If the field is a variable width field, PMM + * records the expression that produces the variable width field. The expression + * is a tree of LogicalExpressions. The PMM walks this tree of + * LogicalExpressions to produce a tree of OutputWidthExpressions. The widths of + * Fixed width fields are just accumulated into a single total. Note: The PMM, + * currently, cannot handle new complex fields, it just uses a hard-coded + * estimate for such fields. + * <p> + * Execution phase: Just before a batch is processed by Project, the PMM walks + * the tree of OutputWidthExpressions and converts them to + * FixedWidthExpressions. It uses the RecordBatchSizer and the function + * annotations to do this conversion. See OutputWidthVisitor for details. */ public class ProjectMemoryManager extends RecordBatchMemoryManager { - static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ProjectMemoryManager.class); - - public RecordBatch getIncomingBatch() { - return incomingBatch; + static final Logger logger = LoggerFactory.getLogger(ProjectMemoryManager.class); + + private RecordBatch incomingBatch; + private ProjectRecordBatch outgoingBatch; + + private int rowWidth; + private final Map<String, ColumnWidthInfo> outputColumnSizes; + // Number of variable width columns in the batch + private int variableWidthColumnCount; + // Number of fixed width columns in the batch + private int fixedWidthColumnCount; + // Number of complex columns in the batch + private int complexColumnsCount; + + // Holds sum of all fixed width column widths + private int totalFixedWidthColumnWidth; + // Holds sum of all complex column widths + // Currently, this is just a guess + private int totalComplexColumnWidth; + + private enum WidthType { + FIXED, + VARIABLE + } + + public enum OutputColumnType { + TRANSFER, + NEW + } + + public static class ColumnWidthInfo { + private final OutputWidthExpression outputExpression; + private final int width; + private final WidthType widthType; + private final OutputColumnType outputColumnType; + private final ValueVector outputVV; // for transfers, this is the transfer src + + + ColumnWidthInfo(OutputWidthExpression outputWidthExpression, + OutputColumnType outputColumnType, + WidthType widthType, + int fieldWidth, ValueVector outputVV) { + this.outputExpression = outputWidthExpression; + this.width = fieldWidth; + this.outputColumnType = outputColumnType; + this.widthType = widthType; + this.outputVV = outputVV; } - RecordBatch incomingBatch = null; - ProjectRecordBatch outgoingBatch = null; + public OutputWidthExpression getOutputExpression() { return outputExpression; } - int rowWidth = 0; - Map<String, ColumnWidthInfo> outputColumnSizes; - // Number of variable width columns in the batch - int variableWidthColumnCount = 0; - // Number of fixed width columns in the batch - int fixedWidthColumnCount = 0; - // Number of complex columns in the batch - int complexColumnsCount = 0; + public OutputColumnType getOutputColumnType() { return outputColumnType; } + public boolean isFixedWidth() { return widthType == WidthType.FIXED; } - // Holds sum of all fixed width column widths - int totalFixedWidthColumnWidth = 0; - // Holds sum of all complex column widths - // Currently, this is just a guess - int totalComplexColumnWidth = 0; - - enum WidthType { - FIXED, - VARIABLE - } - - enum OutputColumnType { - TRANSFER, - NEW - } + public int getWidth() { return width; } + } - class ColumnWidthInfo { - OutputWidthExpression outputExpression; - int width; - WidthType widthType; - OutputColumnType outputColumnType; - ValueVector outputVV; // for transfers, this is the transfer src + public RecordBatch getIncomingBatch() { + return incomingBatch; + } + void ShouldNotReachHere() { + throw new IllegalStateException(); + } - ColumnWidthInfo(OutputWidthExpression outputWidthExpression, - OutputColumnType outputColumnType, - WidthType widthType, - int fieldWidth, ValueVector outputVV) { - this.outputExpression = outputWidthExpression; - this.width = fieldWidth; - this.outputColumnType = outputColumnType; - this.widthType = widthType; - this.outputVV = outputVV; - } + private void setIncomingBatch(RecordBatch recordBatch) { + incomingBatch = recordBatch; + } - public OutputWidthExpression getOutputExpression() { return outputExpression; } + public RecordBatch incomingBatch() { return incomingBatch; } Review comment: Since ```getIncomingBatch()``` already exists in the class, please remove the method and replace its usages. ```suggestion ``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services