[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17005939#comment-17005939 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944 Breaks the big "setup" function into its own class, and separates out physical vector setup from logical projection planning. No functional change; just rearranging existing code. Testing: reran all unit tests. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007048#comment-17007048 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362507186 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007035#comment-17007035 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362491665 ## 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: + * + * 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. */ 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 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 { Review comment: ```suggestion private enum OutputColumnType { ``` 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007047#comment-17007047 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362494309 ## 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: + * + * 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. */ 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 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; Review comment: please remove the field and enum as suggested previously. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007050#comment-17007050 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362495815 ## 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: + * + * 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. */ 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 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; } Review comment: ```suggestion OutputWidthExpression getOutputExpression() { return outputExpression; } ``` 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 qu
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007052#comment-17007052 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362499191 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007043#comment-17007043 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362495075 ## 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: + * + * 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. */ 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 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; Review comment: Actually the field and getter aren't used, but if you think this could be useful for debugging then it can be left as is. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines th
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007051#comment-17007051 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362493899 ## 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: + * + * 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. */ 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 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; Review comment: The ```width``` is always ```-1``` and getter is unused, so I suggest removing the field and related code. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions,
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007045#comment-17007045 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362530889 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007034#comment-17007034 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362529595 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007049#comment-17007049 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362496036 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007037#comment-17007037 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362495939 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007038#comment-17007038 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362493497 ## 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: + * + * 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. */ 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 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 { Review comment: ```suggestion private static class VariableWidthColumnInfo { ``` 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specifi
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007031#comment-17007031 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362500067 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007040#comment-17007040 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362496478 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007030#comment-17007030 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362497826 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007044#comment-17007044 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362534274 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007046#comment-17007046 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362531195 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007041#comment-17007041 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362496119 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007036#comment-17007036 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362529251 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007025#comment-17007025 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362452176 ## 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: + * + * 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 Review comment: ```suggestion * registers the field with PMM. If the field is a variable-width field, PMM ``` 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007029#comment-17007029 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362506622 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007033#comment-17007033 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362452329 ## 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: + * + * 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, Review comment: ```suggestion * fixed-width fields are just accumulated into a single total. Note: The PMM, ``` 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007026#comment-17007026 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362454960 ## 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: + * + * 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. */ 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); Review comment: ```suggestion private static final Logger logger = LoggerFactory.getLogger(ProjectMemoryManager.class); ``` 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007028#comment-17007028 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362499071 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007024#comment-17007024 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362452251 ## 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: + * + * 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 Review comment: ```suggestion * records the expression that produces the variable-width field. The expression ``` 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007042#comment-17007042 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362530135 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007054#comment-17007054 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362534494 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007039#comment-17007039 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362534333 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007053#comment-17007053 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362501279 ## 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007027#comment-17007027 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on 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: + * + * 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. */ 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 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 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 getOutputColumnTy
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007032#comment-17007032 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362486603 ## 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: + * + * 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. */ 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 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 + } Review comment: After a close look at the code, I believe this enum is unnecessary. All usages of the ```ColumnWidthInfo``` constructor accept ```WidthType.VARIABLE``` and there is a block inside the update method : ```java if (columnWidthInfo.isFixedWidth()) { // fixed width columns are accumulated in totalFixedWidthColumnWidth ShouldNotReachHere(); } else {...} ``` Please remove the enum and related code. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge >
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007229#comment-17007229 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362705846 ## 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: + * + * 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. */ 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 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 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 getOutputColum
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007228#comment-17007228 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362704446 ## 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: + * + * 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. */ 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 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 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 getOutputColum
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007226#comment-17007226 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362702109 ## 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: + * + * 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. */ 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 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 + } Review comment: Done. I tried to limit the "blast radius" by gritting my teeth and leaving this class as it was. This seems to be some offshoot of my "temporary" `BatchSizer`. Just for background, some time ago we realized that not all Drill rows are of the same width. (Who'd have guessed?) So, we added "temporary" code to work out sizes in each operator. This was supposed to be temporary until batches carried their own size info. (Once we figure it out for a vector, we don't have to throw it away and figure it out again for each operator.) In fact, I deliberately chose a goofy name "BatchSizer" to remind folks that this was a temporary hack to so I could focus on the "real work" of fixing the external sort. Sigh... But, the temporary solution seems to have become semi-permanent and has grown odd variations such as this. The "master plan" was to not try to predict batch sizes as we are doing here. Instead, the `ResultSetLoader` is intended to just let the operator write to the batch until we hit the desired limit. All the calcs are already done for the reader case. The goal was to use the same mechanism in other places were we don't know widths ahead of time. Project is the classic case: for all we know, the user is doing some silly function like repeating a big `VARCHAR` 100 times. So,
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007227#comment-17007227 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r362702397 ## 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: + * + * 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. */ 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 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; Review comment: Thanks for looking at this in detail. Fixed. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring bre
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007237#comment-17007237 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-570465345 Longer term, it seems we might want to restructure how we generate code. Today, if we run a query across, say, 20 minor fragments in the same Drillbit, and they all see the same schema (the normal case), then all 20 will generate exactly the same code. Then, down in the compiler layer, the first thread will compile the code and put it into the code cache. Threads 2 through 20 will get the compiled code from the cache. But, today, the process is **very** inefficient. Each thread: * Does the semantic expression analysis (function lookup, type resolution, etc.) * Generate the entire Java code. * Rewrite the Java code to remove the varying bits (the generated class name). * Hash the entire code to get a hash code to look up in the code cache. * If a match is found, compare the entire code block byte-by-byte to verify a match. * If new, generate code, cache it, and use the source code (which can be huge) as the cache key. The only real benefit of this approach is that it has worked all these years. The better approach is to: * Create a parameterized "descriptor" object that holds all the factors needed to generate the code. (Input schema, expressions, relevant session options.) * Use that descriptor as a key into the code lookup table. If found, reuse the compiled code without any code gen. * If not found, only then tell the descriptor to generate the needed code, which will then be shared by all fragments. The work I did back in the managed sort, and that I'm starting here, at least splits code gen from the operator. I suspect one (very long term) improvement would be to introduce another layer of abstraction like we had in Impala. The code gen code tries to do the kind of logical type analysis normally done in a planner. But, because the goal is Java code gen, it tends to mix SQL type analysis with Java implementation details, leading to overly complex code. (That's what I'm fighting with the typeof/UNION issue.). Such an approach would be doubly useful a we roll out the schema improvements your team has been doing. If we know the schema (types) at plan time, we can work out all the type conversion stuff at that time. In fact, we can even play the Spark trick: generate Java once in the planner and send it to the workers. I have only vague ideas here; have not spent much time on it. Sounds like you've looked at this some. What do you suggest we do? 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007236#comment-17007236 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-570465320 @ihuzenko, thanks for your thorough review, as usual. Here is my general philosophy on refactoring: do it in bite-size chunks. The first step here was simply to break code gen out of the operator itself. By leaving the bulk of the code unchanged, one can do a review by diffing the old and new files (not in GitHub, sadly), and see that blocks of code are identical, they have just been moved; in some cases from one huge function to smaller functions. Then, once we're satisfied that this simple refactoring works, we can think about the next step. If you agree with the step-by-step approach, then perhaps we can commit this first step to put us in position to make subsequent changes. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007603#comment-17007603 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-570618977 @paul-rogers thanks for the quick update. I agree that using the already generated and compiled code whenever possible would be very good in the long run. Also, I agree with the step-by-step refactoring approach. With respect to this PR, I propose to extract the ```VectorState``` into a separate class together with all the ```ProjectRecordBatch```'s fields that can be encapsulated. I believe that the extraction of ```VectorState``` won't allow using mixed access to fields in the future and opens more ways for later improvements. In order to better express my thought, I created the sample [commit](https://github.com/ihuzenko/drill/commit/80fb0065cf4d11b836383b031dffc07b92a5ad91). Sorry for the late reply. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17007935#comment-17007935 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-570763354 @ihuzenko, good suggestion on `VectorState`: my original thought was to make it a static class, then move it to its own file. Then I thought better of it. You'll notice that, at present, it is kind of a "shim" class to expose certain operations on the vector objects to the "materializer" without making the entire project operator visible. In particular, it is an inner class so it can tinker with project operator state. We could accomplish the same thing by defining an interface which `ProjectRecordBatch` would implement. But, in that case, all the complex setup logic would be back in the project operator, undoing the separation we just accomplished. So, the interface approach feels not quite right. I agree that this can be improved, but I don't yet see an obvious move that leaves the code simpler. So, I thought to just leave it as is for now. Maybe it can evolve to own the allocation vectors, complex writers and so on. These are the kinds of questions we can ask now that we can start to see the pieces rather than just a big mess. Suggestions? 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17008590#comment-17008590 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-571034273 Hello @paul-rogers , if you don't want to extract the ```VectorState``` class then at least please put related fields into it, so it'll be visible which state is managed by the class. Although extraction is better way for me, since it won't allow anybody to easily mix access to related fields (```transfers, allocationVectors, complexWriters, complexFieldReferences etc...```). PS. Here there is no quick solution without serious rewriting to make everything look good:) 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17009046#comment-17009046 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-571235610 @ihuzenko, the goal of this refactoring was simply to pull out the code gen part. The `VectorState` is an admitted hack to create a clean interface on the CG side, though it is messy on the operator side. Since we don't like this approach, for now, I'll remove `VectorState` and simply move its methods onto the project operator, and pass the project operator to the CG component. Not ideal, but we can revisit the issue later when we want to do further cleanup. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17009058#comment-17009058 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-571241220 One more thought about the CG. In working on another issue, it slowly dawned on me a couple of other worthwhile goals in addition to those mentioned earlier. We mentioned above that CD combines logical planning and code gen, creating a very complex bit of code. Again, some of us are not smart enough to be able to quickly understand the combined logic, so it would make sense to separate out the two steps so that average folks can more easily understand and modify them. Then, we need to think about debug and testing. As originally implemented, CG was direct path from operator to running byte codes. The original authors were smart enough to be able to debug the result by stepping through the compiled byte codes. Most of us are not that smart. So, few years back we added the "plain Java" and "save code for debugging" modes which made it possible for us newbies to view and step through the Java code. Still, in order to debug, we need to run Drill, find a useful unit test, run the query, and then debug CG in the context of running a query. We are limited in what we can test based on what queries we can find or create to set up the scenarios of interest. Better would be to apply unit testing principles: separate out CG so we can set up a set of inputs, run CG, and diff the resulting code against a golden copy. This is how we debugged the Impala planner and it worked very well. This way, we can easily set up every data type without needing to use a zillion different input file formats. That is: ``` (operator config, schema, options) --> CG --> (Java code, exec plan) ``` Where the "exec plan" would be things like the knick-lacks that the `VectorState` currently wrap. That is, rather than having CG set up vectors, maybe CG creates a plan that says, "here are the vectors you will need", then the operator creates the vectors. Still pretty hand-wavey at this point. This refactoring can be seen as a naive grasping toward that goal. We need to pull out CG, but it is not clear yet the final form. The same can be said of the earlier refactoring of the external sort to separate out CG. Sometimes, just by taking a step, we can get a bit better understanding of where we need to go. All that said, the goal here is just to take a step, not to implement a final solution. I realize, however, that makes this PR hard to review since it is incomplete: it takes a step, but does not clearly state "a step to where?" 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17009066#comment-17009066 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-571235610 @ihuzenko, the goal of this refactoring was simply to pull out the code gen part. The `VectorState` is an admitted hack to create a clean interface on the CG side, though it is messy on the operator side. I was really hoping to not change the execution part of the project operator in this PR in order to limit the scope of changes. Maybe I'll go with the interface route (discussed in an earlier note), which will help with the longer term goal outlined below. Let me play with the code to see how that approach might work. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17009333#comment-17009333 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-571414414 @ihuzenko, went ahead and adopted the interface solution. The interface is probably a better idea: if we try to isolate code gen for testing, we can create a mock implementation that skips the actual vector work. Please let me know if this satisfies your concerns. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17009376#comment-17009376 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-571437864 @ihuzenko, refactored some additional steps to adopt the solution you suggested. This version still uses the interface, with an implementation in a class other than the project record batch. This turns out to be handy because, oddly, the project operator generates code for two separate incoming batches: the "real" one and a "fake" empty one. The `ProjectRecordBatchBuilder` holds onto the input batch so we don't have to pass it into the materializer and back out. This version tries to eliminate all references to the incoming batch in the materializer, and instead work with the batch schema. Annoyingly, the `ExpressionTreeMaterializer` needs the input batch so it can iterate over the vectors to get their schemas. If all we need is the schema, we don't need actual vectors. So, if we can pass in a schema, we can completely separate code gen from physical vectors. The next refactoring move is to change this code to work with a schema (or interface to obtain the schema) rather than the actual vectors. Now, as it turns out, the batch schema has limitations for complex types, which is one of the reasons we created the `TupleMetadata` family of classes. So, perhaps we can convert the incoming batch to a `TupleMetadata` schema and use that. (The code to do that already exists in the `RowSet` classes.) Or, we can just pass an interface which will return the `TypedFieldId` for each column. Or, do that conversion ahead of time, and pass in the results. Will have to play with it some to see which solution is the simplest. Since the required work will be rather large; I propose we do that as a separate PR. Have we done enough for this one PR? 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010732#comment-17010732 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364210835 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/OutputWidthVisitor.java ## @@ -43,238 +43,213 @@ import java.util.ArrayList; -public class OutputWidthVisitor extends AbstractExecExprVisitor { - -@Override -public OutputWidthExpression visitVarDecimalConstant(VarDecimalExpression varDecimalExpression, - OutputWidthVisitorState state) throws RuntimeException { - Preconditions.checkArgument(varDecimalExpression.getMajorType().hasPrecision()); -return new FixedLenExpr(varDecimalExpression.getMajorType().getPrecision()); -} - - -/** - * - * Records the {@link IfExpression} as a {@link IfElseWidthExpr}. IfElseWidthExpr will be reduced to - * a {@link FixedLenExpr} by taking the max of the if-expr-width and the else-expr-width. - * - * @param ifExpression - * @param state - * @return IfElseWidthExpr - * @throws RuntimeException - */ -@Override -public OutputWidthExpression visitIfExpression(IfExpression ifExpression, OutputWidthVisitorState state) -throws RuntimeException { -IfExpression.IfCondition condition = ifExpression.ifCondition; -LogicalExpression ifExpr = condition.expression; -LogicalExpression elseExpr = ifExpression.elseExpression; - -OutputWidthExpression ifWidthExpr = ifExpr.accept(this, state); -OutputWidthExpression elseWidthExpr = null; -if (elseExpr != null) { -elseWidthExpr = elseExpr.accept(this, state); -} -return new IfElseWidthExpr(ifWidthExpr, elseWidthExpr); +public class OutputWidthVisitor +extends AbstractExecExprVisitor { + + @Override + public OutputWidthExpression visitVarDecimalConstant(VarDecimalExpression varDecimalExpression, + OutputWidthVisitorState state) throws RuntimeException { + Preconditions.checkArgument(varDecimalExpression.getMajorType().hasPrecision()); +return new FixedLenExpr(varDecimalExpression.getMajorType().getPrecision()); + } + + /** + * Review comment: ```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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010731#comment-17010731 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364208438 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/OutputWidthVisitor.java ## @@ -43,238 +43,213 @@ import java.util.ArrayList; -public class OutputWidthVisitor extends AbstractExecExprVisitor { - -@Override -public OutputWidthExpression visitVarDecimalConstant(VarDecimalExpression varDecimalExpression, - OutputWidthVisitorState state) throws RuntimeException { - Preconditions.checkArgument(varDecimalExpression.getMajorType().hasPrecision()); -return new FixedLenExpr(varDecimalExpression.getMajorType().getPrecision()); -} - - -/** - * - * Records the {@link IfExpression} as a {@link IfElseWidthExpr}. IfElseWidthExpr will be reduced to - * a {@link FixedLenExpr} by taking the max of the if-expr-width and the else-expr-width. - * - * @param ifExpression - * @param state - * @return IfElseWidthExpr - * @throws RuntimeException - */ -@Override -public OutputWidthExpression visitIfExpression(IfExpression ifExpression, OutputWidthVisitorState state) -throws RuntimeException { -IfExpression.IfCondition condition = ifExpression.ifCondition; -LogicalExpression ifExpr = condition.expression; -LogicalExpression elseExpr = ifExpression.elseExpression; - -OutputWidthExpression ifWidthExpr = ifExpr.accept(this, state); -OutputWidthExpression elseWidthExpr = null; -if (elseExpr != null) { -elseWidthExpr = elseExpr.accept(this, state); -} -return new IfElseWidthExpr(ifWidthExpr, elseWidthExpr); +public class OutputWidthVisitor +extends AbstractExecExprVisitor { Review comment: Please reformat to: ```java public class OutputWidthVisitor extends AbstractExecExprVisitor { ``` 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010735#comment-17010735 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364234843 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectionMaterializer.java ## @@ -0,0 +1,625 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.physical.impl.project; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; + +import org.apache.commons.collections.map.CaseInsensitiveMap; +import org.apache.drill.common.expression.ConvertExpression; +import org.apache.drill.common.expression.ErrorCollector; +import org.apache.drill.common.expression.ErrorCollectorImpl; +import org.apache.drill.common.expression.ExpressionPosition; +import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.FunctionCall; +import org.apache.drill.common.expression.FunctionCallFactory; +import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.ValueExpressions; +import org.apache.drill.common.expression.PathSegment.NameSegment; +import org.apache.drill.common.expression.fn.FunctionReplacementUtils; +import org.apache.drill.common.logical.data.NamedExpression; +import org.apache.drill.common.types.Types; +import org.apache.drill.common.types.TypeProtos.MinorType; +import org.apache.drill.exec.exception.ClassTransformationException; +import org.apache.drill.exec.exception.SchemaChangeException; +import org.apache.drill.exec.expr.ClassGenerator; +import org.apache.drill.exec.expr.CodeGenerator; +import org.apache.drill.exec.expr.DrillFuncHolderExpr; +import org.apache.drill.exec.expr.ExpressionTreeMaterializer; +import org.apache.drill.exec.expr.ValueVectorReadExpression; +import org.apache.drill.exec.expr.ValueVectorWriteExpression; +import org.apache.drill.exec.expr.fn.FunctionLookupContext; +import org.apache.drill.exec.ops.FragmentContext; +import org.apache.drill.exec.planner.StarColumnHelper; +import org.apache.drill.exec.record.MaterializedField; +import org.apache.drill.exec.record.VectorAccessible; +import org.apache.drill.exec.record.VectorWrapper; +import org.apache.drill.exec.record.BatchSchema; +import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode; +import org.apache.drill.exec.server.options.OptionManager; +import org.apache.drill.exec.store.ColumnExplorer; +import org.apache.drill.exec.vector.ValueVector; +import org.apache.drill.shaded.guava.com.google.common.collect.Lists; +import org.apache.drill.shaded.guava.com.google.common.collect.Maps; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.carrotsearch.hppc.IntHashSet; + +/** + * Plans the projection given the incoming and requested outgoing schemas. Works + * with the {@link VectorState} to create required vectors, writers and so on. Review comment: please update the comment since ```VectorState``` is gone. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combine
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010733#comment-17010733 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364218502 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchBuilder.java ## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.physical.impl.project; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.exec.expr.ValueVectorReadExpression; +import org.apache.drill.exec.expr.ValueVectorWriteExpression; +import org.apache.drill.exec.record.MaterializedField; +import org.apache.drill.exec.record.RecordBatch; +import org.apache.drill.exec.record.TransferPair; +import org.apache.drill.exec.record.TypedFieldId; +import org.apache.drill.exec.record.VectorContainer; +import org.apache.drill.exec.vector.FixedWidthVector; +import org.apache.drill.exec.vector.SchemaChangeCallBack; +import org.apache.drill.exec.vector.ValueVector; +import org.apache.drill.shaded.guava.com.google.common.base.Preconditions; +import org.apache.drill.shaded.guava.com.google.common.collect.Lists; + +public class ProjectBatchBuilder implements ProjectionMaterializer.BatchBuilder { + private final ProjectRecordBatch projectBatch; + private final VectorContainer container; + private final SchemaChangeCallBack callBack; + private final RecordBatch incomingBatch; + final List transfers = new ArrayList<>(); Review comment: please make also private + add getter + move ```= new ArrayList<>()``` into constructor. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010734#comment-17010734 ] ASF GitHub Bot commented on DRILL-7503: --- ihuzenko commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364223072 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchBuilder.java ## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.physical.impl.project; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.exec.expr.ValueVectorReadExpression; +import org.apache.drill.exec.expr.ValueVectorWriteExpression; +import org.apache.drill.exec.record.MaterializedField; +import org.apache.drill.exec.record.RecordBatch; +import org.apache.drill.exec.record.TransferPair; +import org.apache.drill.exec.record.TypedFieldId; +import org.apache.drill.exec.record.VectorContainer; +import org.apache.drill.exec.vector.FixedWidthVector; +import org.apache.drill.exec.vector.SchemaChangeCallBack; +import org.apache.drill.exec.vector.ValueVector; +import org.apache.drill.shaded.guava.com.google.common.base.Preconditions; +import org.apache.drill.shaded.guava.com.google.common.collect.Lists; + +public class ProjectBatchBuilder implements ProjectionMaterializer.BatchBuilder { + private final ProjectRecordBatch projectBatch; + private final VectorContainer container; + private final SchemaChangeCallBack callBack; + private final RecordBatch incomingBatch; + final List transfers = new ArrayList<>(); + + public ProjectBatchBuilder(ProjectRecordBatch projectBatch, VectorContainer container, + SchemaChangeCallBack callBack, RecordBatch incomingBatch) { +this.projectBatch = projectBatch; +this.container = container; +this.callBack = callBack; +this.incomingBatch = incomingBatch; + } + + @Override + public void addTransferField(String name, ValueVector vvIn) { +FieldReference ref = new FieldReference(name); +ValueVector vvOut = container.addOrGet(MaterializedField.create(ref.getAsNamePart().getName(), + vvIn.getField().getType()), callBack); +projectBatch.memoryManager.addTransferField(vvIn, vvIn.getField().getName(), vvOut.getField().getName()); +transfers.add(vvIn.makeTransferPair(vvOut)); + } + + @Override + public int addDirectTransfer(FieldReference ref, ValueVectorReadExpression vectorRead) { +TypedFieldId id = vectorRead.getFieldId(); +ValueVector vvIn = incomingBatch.getValueAccessorById(id.getIntermediateClass(), id.getFieldIds()).getValueVector(); +Preconditions.checkNotNull(incomingBatch); + +ValueVector vvOut = + container.addOrGet(MaterializedField.create(ref.getLastSegment().getNameSegment().getPath(), +vectorRead.getMajorType()), callBack); +TransferPair tp = vvIn.makeTransferPair(vvOut); +projectBatch.memoryManager.addTransferField(vvIn, TypedFieldId.getPath(id, incomingBatch), vvOut.getField().getName()); +transfers.add(tp); +return vectorRead.getFieldId().getFieldIds()[0]; + } + + @Override + public ValueVectorWriteExpression addOutputVector(String name, LogicalExpression expr) { +MaterializedField outputField = MaterializedField.create(name, expr.getMajorType()); +ValueVector vv = container.addOrGet(outputField, callBack); +projectBatch.allocationVectors.add(vv); +TypedFieldId fid = container.getValueVectorId(SchemaPath.getSimplePath(outputField.getName())); +ValueVectorWriteExpression write = new ValueVectorWriteExpression(fid, expr, true); +projectBatch.memoryManager.addNewField(vv, write); +return write; + } + + @Override + public void addComplexField(FieldReference ref) { +initComplexWriters(); +if (projectBatch.complexFieldReferencesList == null) { + projectBatch.complexFieldReferencesList =
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010769#comment-17010769 ] ASF GitHub Bot commented on DRILL-7503: --- arina-ielchiieva commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364288964 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/record/TypedFieldId.java ## @@ -241,7 +241,7 @@ public TypedFieldId build() { secondaryFinal = finalType; } - MajorType actualFinalType = finalType; + //MajorType actualFinalType = finalType; Review comment: Should we keep this commented code? 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010775#comment-17010775 ] ASF GitHub Bot commented on DRILL-7503: --- arina-ielchiieva commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364290284 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectionMaterializer.java ## @@ -0,0 +1,625 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.physical.impl.project; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; + +import org.apache.commons.collections.map.CaseInsensitiveMap; +import org.apache.drill.common.expression.ConvertExpression; +import org.apache.drill.common.expression.ErrorCollector; +import org.apache.drill.common.expression.ErrorCollectorImpl; +import org.apache.drill.common.expression.ExpressionPosition; +import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.FunctionCall; +import org.apache.drill.common.expression.FunctionCallFactory; +import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.ValueExpressions; +import org.apache.drill.common.expression.PathSegment.NameSegment; +import org.apache.drill.common.expression.fn.FunctionReplacementUtils; +import org.apache.drill.common.logical.data.NamedExpression; +import org.apache.drill.common.types.Types; +import org.apache.drill.common.types.TypeProtos.MinorType; +import org.apache.drill.exec.exception.ClassTransformationException; +import org.apache.drill.exec.exception.SchemaChangeException; +import org.apache.drill.exec.expr.ClassGenerator; +import org.apache.drill.exec.expr.CodeGenerator; +import org.apache.drill.exec.expr.DrillFuncHolderExpr; +import org.apache.drill.exec.expr.ExpressionTreeMaterializer; +import org.apache.drill.exec.expr.ValueVectorReadExpression; +import org.apache.drill.exec.expr.ValueVectorWriteExpression; +import org.apache.drill.exec.expr.fn.FunctionLookupContext; +import org.apache.drill.exec.ops.FragmentContext; +import org.apache.drill.exec.planner.StarColumnHelper; +import org.apache.drill.exec.record.MaterializedField; +import org.apache.drill.exec.record.VectorAccessible; +import org.apache.drill.exec.record.VectorWrapper; +import org.apache.drill.exec.record.BatchSchema; +import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode; +import org.apache.drill.exec.server.options.OptionManager; +import org.apache.drill.exec.store.ColumnExplorer; +import org.apache.drill.exec.vector.ValueVector; +import org.apache.drill.shaded.guava.com.google.common.collect.Lists; +import org.apache.drill.shaded.guava.com.google.common.collect.Maps; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.carrotsearch.hppc.IntHashSet; + +/** + * Plans the projection given the incoming and requested outgoing schemas. Works + * with the {@link VectorState} to create required vectors, writers and so on. + * Populates the code generator with the "projector" expressions. + */ +class ProjectionMaterializer { + private static final Logger logger = LoggerFactory.getLogger(ProjectionMaterializer.class); + private static final String EMPTY_STRING = ""; + + /** + * Abstracts the physical vector setup operations to separate + * the physical setup, in ProjectRecordBatch, from the + * logical setup in the materializer class. + */ + public interface BatchBuilder { +void addTransferField(String name, ValueVector vvIn); +ValueVectorWriteExpression addOutputVector(String name, LogicalExpression expr); +int addDirectTransfer(FieldReference ref, ValueVectorReadExpression vectorRead); +void addComplexField(FieldReference ref); +ValueVectorWriteExpression addEvalVector(String outputName, +LogicalExpression expr); + } + + private static class ClassifierResult { +private boolean isStar; +private List outputNames; +private String p
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17010774#comment-17010774 ] ASF GitHub Bot commented on DRILL-7503: --- arina-ielchiieva commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364289943 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectionMaterializer.java ## @@ -0,0 +1,625 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.physical.impl.project; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; + +import org.apache.commons.collections.map.CaseInsensitiveMap; +import org.apache.drill.common.expression.ConvertExpression; +import org.apache.drill.common.expression.ErrorCollector; +import org.apache.drill.common.expression.ErrorCollectorImpl; +import org.apache.drill.common.expression.ExpressionPosition; +import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.FunctionCall; +import org.apache.drill.common.expression.FunctionCallFactory; +import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.expression.ValueExpressions; +import org.apache.drill.common.expression.PathSegment.NameSegment; +import org.apache.drill.common.expression.fn.FunctionReplacementUtils; +import org.apache.drill.common.logical.data.NamedExpression; +import org.apache.drill.common.types.Types; +import org.apache.drill.common.types.TypeProtos.MinorType; +import org.apache.drill.exec.exception.ClassTransformationException; +import org.apache.drill.exec.exception.SchemaChangeException; +import org.apache.drill.exec.expr.ClassGenerator; +import org.apache.drill.exec.expr.CodeGenerator; +import org.apache.drill.exec.expr.DrillFuncHolderExpr; +import org.apache.drill.exec.expr.ExpressionTreeMaterializer; +import org.apache.drill.exec.expr.ValueVectorReadExpression; +import org.apache.drill.exec.expr.ValueVectorWriteExpression; +import org.apache.drill.exec.expr.fn.FunctionLookupContext; +import org.apache.drill.exec.ops.FragmentContext; +import org.apache.drill.exec.planner.StarColumnHelper; +import org.apache.drill.exec.record.MaterializedField; +import org.apache.drill.exec.record.VectorAccessible; +import org.apache.drill.exec.record.VectorWrapper; +import org.apache.drill.exec.record.BatchSchema; +import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode; +import org.apache.drill.exec.server.options.OptionManager; +import org.apache.drill.exec.store.ColumnExplorer; +import org.apache.drill.exec.vector.ValueVector; +import org.apache.drill.shaded.guava.com.google.common.collect.Lists; +import org.apache.drill.shaded.guava.com.google.common.collect.Maps; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.carrotsearch.hppc.IntHashSet; + +/** + * Plans the projection given the incoming and requested outgoing schemas. Works + * with the {@link VectorState} to create required vectors, writers and so on. + * Populates the code generator with the "projector" expressions. + */ +class ProjectionMaterializer { + private static final Logger logger = LoggerFactory.getLogger(ProjectionMaterializer.class); + private static final String EMPTY_STRING = ""; + + /** + * Abstracts the physical vector setup operations to separate + * the physical setup, in ProjectRecordBatch, from the + * logical setup in the materializer class. + */ + public interface BatchBuilder { +void addTransferField(String name, ValueVector vvIn); +ValueVectorWriteExpression addOutputVector(String name, LogicalExpression expr); +int addDirectTransfer(FieldReference ref, ValueVectorReadExpression vectorRead); +void addComplexField(FieldReference ref); +ValueVectorWriteExpression addEvalVector(String outputName, +LogicalExpression expr); + } + + private static class ClassifierResult { +private boolean isStar; +private List outputNames; +private String p
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17011399#comment-17011399 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364554333 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchBuilder.java ## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.physical.impl.project; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.LogicalExpression; +import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.exec.expr.ValueVectorReadExpression; +import org.apache.drill.exec.expr.ValueVectorWriteExpression; +import org.apache.drill.exec.record.MaterializedField; +import org.apache.drill.exec.record.RecordBatch; +import org.apache.drill.exec.record.TransferPair; +import org.apache.drill.exec.record.TypedFieldId; +import org.apache.drill.exec.record.VectorContainer; +import org.apache.drill.exec.vector.FixedWidthVector; +import org.apache.drill.exec.vector.SchemaChangeCallBack; +import org.apache.drill.exec.vector.ValueVector; +import org.apache.drill.shaded.guava.com.google.common.base.Preconditions; +import org.apache.drill.shaded.guava.com.google.common.collect.Lists; + +public class ProjectBatchBuilder implements ProjectionMaterializer.BatchBuilder { + private final ProjectRecordBatch projectBatch; + private final VectorContainer container; + private final SchemaChangeCallBack callBack; + private final RecordBatch incomingBatch; + final List transfers = new ArrayList<>(); Review comment: Made private. Added getter. But. left initializer with the field since this is a final member and we often initialize such objects this way as a way of saying that the field does not depend on constructor input. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17011402#comment-17011402 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#discussion_r364554593 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/record/TypedFieldId.java ## @@ -241,7 +241,7 @@ public TypedFieldId build() { secondaryFinal = finalType; } - MajorType actualFinalType = finalType; + //MajorType actualFinalType = finalType; Review comment: Not sure. Not sure what the comments here are about, so I thought I'd leave them. Just commenting out an unused variable. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17011406#comment-17011406 ] ASF GitHub Bot commented on DRILL-7503: --- paul-rogers commented on issue #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944#issuecomment-572380826 Thank you @ihuzenko and @arina-ielchiieva for your reviews. Addressed remaining minor issues. Squashed commits. @ihuzenko, your many suggestions made this a much better solution. It is almost, but not quite, clean enough that I could write code gen unit tests. Need to clean up that pesky `ExpressionTreeMaterializer` issue, then we'll be able to write such tests. 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7503) Refactor project operator
[ https://issues.apache.org/jira/browse/DRILL-7503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17011982#comment-17011982 ] ASF GitHub Bot commented on DRILL-7503: --- asfgit commented on pull request #1944: DRILL-7503: Refactor the project operator URL: https://github.com/apache/drill/pull/1944 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 > Refactor project operator > - > > Key: DRILL-7503 > URL: https://issues.apache.org/jira/browse/DRILL-7503 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Paul Rogers >Assignee: Paul Rogers >Priority: Minor > Labels: ready-to-commit > Fix For: 1.18.0 > > > Work on another ticket revealed that the Project operator ("record batch") > has grown quite complex. The setup phase lives in the operator as one huge > function. The function combines the "logical" tasks of working out the > projection expressions and types, the code gen for those expressions, and the > physical setup of vectors. > The refactoring breaks up the logic so that it is easier to focus on the > specific bits of interest. -- This message was sent by Atlassian Jira (v8.3.4#803005)