[ https://issues.apache.org/jira/browse/DRILL-6613?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16547993#comment-16547993 ]
ASF GitHub Bot commented on DRILL-6613: --------------------------------------- vrozov commented on a change in pull request #1383: DRILL-6613: Refactor MaterializedField URL: https://github.com/apache/drill/pull/1383#discussion_r203432621 ########## File path: exec/vector/src/main/java/org/apache/drill/exec/record/MaterializedField.java ########## @@ -115,56 +175,17 @@ public void removeChild(MaterializedField field) { */ public void replaceType(MajorType newType) { - assert type.getMinorType() == newType.getMinorType(); - assert type.getMode() == newType.getMode(); + Preconditions.checkArgument(type.getMinorType() == newType.getMinorType()); + Preconditions.checkArgument(type.getMode() == newType.getMode()); Review comment: I would be concerned with the cost only in case `replaceType()` is called for every row (tuple) (note that in java there is no test-time assert, asserts are optimized out only when bytecode is compiled by the hotspot) and I have no idea what it means to replace a type. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on 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 MaterializedField > -------------------------- > > Key: DRILL-6613 > URL: https://issues.apache.org/jira/browse/DRILL-6613 > Project: Apache Drill > Issue Type: Improvement > Reporter: Vlad Rozov > Assignee: Vlad Rozov > Priority: Minor > > {{MaterializedField}} does not need to implement {{clone()}} and should use > constructor. -- This message was sent by Atlassian JIRA (v7.6.3#76005)