You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by dz...@apache.org on 2022/07/20 17:13:53 UTC

[drill] 10/10: DRILL-8266: Number narrowing issues (#2608)

This is an automated email from the ASF dual-hosted git repository.

dzamo pushed a commit to branch 1.20
in repository https://gitbox.apache.org/repos/asf/drill.git

commit 162bc851052deabd2134bfa22916bf17ebadd74c
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Wed Jul 20 06:15:49 2022 +0100

    DRILL-8266: Number narrowing issues (#2608)
---
 .../org/apache/drill/hbase/TestTableGenerator.java |  4 +--
 .../apache/drill/exec/udfs/NetworkFunctions.java   |  4 +--
 .../base/AbstractGroupScanWithMetadata.java        |  2 +-
 .../physical/impl/common/HashTableTemplate.java    |  2 +-
 .../apache/drill/exec/record/RecordBatchSizer.java |  4 +--
 .../drill/exec/record/VectorInitializer.java       |  2 +-
 .../columnreaders/NullableColumnReader.java        |  4 +--
 .../filereader/BufferedDirectBufInputStream.java   | 16 +++++------
 .../drill/exec/DrillSeparatePlanningTest.java      |  4 +--
 .../exec/physical/impl/writer/TestWriter.java      |  2 +-
 .../java/org/apache/drill/test/ProfileParser.java  | 33 +++++++++++-----------
 11 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestTableGenerator.java b/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestTableGenerator.java
index 5e14e09706..b66d584d32 100644
--- a/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestTableGenerator.java
+++ b/contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestTableGenerator.java
@@ -462,7 +462,7 @@ public class TestTableGenerator {
 
     BufferedMutator table = conn.getBufferedMutator(tableName);
 
-    for (float i = (float)0.5; i <= 100.00; i += 0.75) {
+    for (float i = 0.5f; i <= 100.00; i += 0.75f) {
       byte[] bytes = new byte[5];
       PositionedByteRange br = new SimplePositionedMutableByteRange(bytes, 0, 5);
       OrderedBytes.encodeFloat32(br, i,Order.ASCENDING);
@@ -586,7 +586,7 @@ public class TestTableGenerator {
 
     BufferedMutator table = conn.getBufferedMutator(tableName);
 
-    for (float i = (float)0.5; i <= 100.00; i += 0.75) {
+    for (float i = 0.5f; i <= 100.00; i += 0.75f) {
       byte[] bytes = new byte[5];
       PositionedByteRange br = new SimplePositionedMutableByteRange(bytes, 0, 5);
       OrderedBytes.encodeFloat32(br, i, Order.DESCENDING);
diff --git a/contrib/udfs/src/main/java/org/apache/drill/exec/udfs/NetworkFunctions.java b/contrib/udfs/src/main/java/org/apache/drill/exec/udfs/NetworkFunctions.java
index 0dbaf87a1d..fcbd39f5fa 100644
--- a/contrib/udfs/src/main/java/org/apache/drill/exec/udfs/NetworkFunctions.java
+++ b/contrib/udfs/src/main/java/org/apache/drill/exec/udfs/NetworkFunctions.java
@@ -431,7 +431,7 @@ public class NetworkFunctions {
         int power = 3 - i;
         try {
           int ip = Integer.parseInt(ipAddressInArray[i]);
-          result += ip * Math.pow(256, power);
+          result += Math.round(ip * Math.pow(256, power));
         } catch (NumberFormatException e) {
           // should not happen since we validated the address
           // but if does, return null
@@ -509,4 +509,4 @@ public class NetworkFunctions {
       out.value = validator.isValidInet6Address(ipString) ? 1 : 0;
     }
   }
-}
\ No newline at end of file
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScanWithMetadata.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScanWithMetadata.java
index eb7d4da3ae..a11f834761 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScanWithMetadata.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScanWithMetadata.java
@@ -545,7 +545,7 @@ public abstract class AbstractGroupScanWithMetadata<P extends TableMetadataProvi
    */
   protected <T extends BaseMetadata> List<T> limitMetadata(Collection<T> metadataList, int maxRecords) {
     List<T> qualifiedMetadata = new ArrayList<>();
-    int currentRowCount = 0;
+    long currentRowCount = 0;
     for (T metadata : metadataList) {
       long rowCount = TableStatisticsKind.ROW_COUNT.getValue(metadata);
       if (rowCount == Statistic.NO_COLUMN_STATS) {
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
index c93de9ef47..26ffde908a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
@@ -862,7 +862,7 @@ public abstract class HashTableTemplate implements HashTable {
         bh.dump(startIdx);
       }
     }
-    resizingTime += System.currentTimeMillis() - t0;
+    resizingTime += Math.toIntExact(System.currentTimeMillis() - t0);
     numResizing++;
   }
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchSizer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchSizer.java
index 4f5f02c578..70f874dfab 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchSizer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchSizer.java
@@ -466,7 +466,7 @@ public class RecordBatchSizer {
     private void allocateMap(AbstractMapVector map, int recordCount) {
       if (map instanceof AbstractRepeatedMapVector) {
         ((AbstractRepeatedMapVector) map).allocateOffsetsNew(recordCount);
-          recordCount *= getEntryCardinalityForAlloc();
+          recordCount *= Math.round(getEntryCardinalityForAlloc());
         }
 
       for (ValueVector vector : map) {
@@ -476,7 +476,7 @@ public class RecordBatchSizer {
 
     private void allocateRepeatedList(RepeatedListVector vector, int recordCount) {
       vector.allocateOffsetsNew(recordCount);
-      recordCount *= getEntryCardinalityForAlloc();
+      recordCount *= Math.round(getEntryCardinalityForAlloc());
       ColumnSize child = children.get(vector.getField().getName());
       if (vector.getDataVector() != null) {
         child.allocateVector(vector.getDataVector(), recordCount);
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorInitializer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorInitializer.java
index 83c0142158..9dc15c1a7f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorInitializer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorInitializer.java
@@ -140,7 +140,7 @@ public class VectorInitializer {
       if (hint == null) {
         recordCount *= 10;
       } else {
-        recordCount *= hint.elementCount;
+        recordCount *= Math.round(hint.elementCount);
       }
     }
     prefix += map.getField().getName() + ".";
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableColumnReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableColumnReader.java
index 4e9121fd22..67531f2cf0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableColumnReader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableColumnReader.java
@@ -173,7 +173,7 @@ abstract class NullableColumnReader<V extends ValueVector> extends ColumnReader<
         pageReader.readPosInBytes = readStartInBytes + readLength;
       }
 
-      pageReader.valuesRead += recordsReadInThisIteration;
+      pageReader.valuesRead += Math.toIntExact(recordsReadInThisIteration);
 
       totalValuesRead += runLength + nullRunLength;
 
@@ -287,7 +287,7 @@ abstract class NullableColumnReader<V extends ValueVector> extends ColumnReader<
         pageReader.readPosInBytes = readStartInBytes + readLength;
       }
 
-      pageReader.valuesRead += recordsReadInThisIteration;
+      pageReader.valuesRead += Math.toIntExact(recordsReadInThisIteration);
       totalValuesRead += numNonNullValues + numNullValues;
       currPageValuesProcessed += numNonNullValues + numNullValues;
       valueCount += numNonNullValues + numNullValues;
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/util/filereader/BufferedDirectBufInputStream.java b/exec/java-exec/src/main/java/org/apache/drill/exec/util/filereader/BufferedDirectBufInputStream.java
index cfcb073580..c32ef55ebe 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/util/filereader/BufferedDirectBufInputStream.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/util/filereader/BufferedDirectBufInputStream.java
@@ -104,8 +104,8 @@ public class BufferedDirectBufInputStream extends DirectBufInputStream implement
     super(in, allocator, id, startOffset, totalByteSize, enforceTotalByteSize, enableHints);
     Preconditions.checkArgument(bufSize >= 0);
     // We make the buffer size the smaller of the buffer Size parameter or the total Byte Size
-    // rounded to next highest pwoer of two
-    int bSize = bufSize < (int) totalByteSize ? bufSize : (int) totalByteSize;
+    // rounded to next highest power of two
+    int bSize = Math.min(bufSize, Math.toIntExact(totalByteSize));
     // round up to next power of 2
     bSize--;
     bSize |= bSize >>> 1;
@@ -216,7 +216,7 @@ public class BufferedDirectBufInputStream extends DirectBufInputStream implement
     }
     bytesAvailable = this.count - this.curPosInBuffer;
     //copy into output buffer
-    int copyBytes = bytesAvailable < len ? bytesAvailable : len;
+    int copyBytes = Math.min(bytesAvailable, len);
     getBuf().getBytes(curPosInBuffer, buf, off, copyBytes);
     buf.writerIndex(off + copyBytes);
     this.curPosInBuffer += copyBytes;
@@ -241,7 +241,7 @@ public class BufferedDirectBufInputStream extends DirectBufInputStream implement
     }
     bytesAvailable = this.count - this.curPosInBuffer;
     // return a slice as the  output
-    int bytesToRead = bytesAvailable < len ? bytesAvailable : len;
+    int bytesToRead = Math.min(bytesAvailable, len);
     DrillBuf newBuf = this.getBuf().slice(off, bytesToRead);
     newBuf.retain();
     return newBuf;
@@ -297,7 +297,7 @@ public class BufferedDirectBufInputStream extends DirectBufInputStream implement
 
 
   @Override public int read(byte[] b) throws IOException {
-    return b.length == 1 ? read() : read(b, (int) 0, b.length);
+    return b.length == 1 ? read() : read(b, 0, b.length);
   }
 
 
@@ -358,8 +358,8 @@ public class BufferedDirectBufInputStream extends DirectBufInputStream implement
         return 0;
       }
     }
-    bytesSkipped = bytesAvailable < n ? bytesAvailable : n;
-    this.curPosInBuffer += bytesSkipped;
+    bytesSkipped = Math.min(bytesAvailable, n);
+    this.curPosInBuffer += Math.toIntExact(bytesSkipped);
 
     return bytesSkipped;
   }
@@ -404,8 +404,6 @@ public class BufferedDirectBufInputStream extends DirectBufInputStream implement
           in = null;
           inp.close();
         }
-      } catch (IOException e) {
-        throw e;
       } finally {
         if ((buffer = this.internalBuffer) != null) {
           this.internalBuffer = null;
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/DrillSeparatePlanningTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/DrillSeparatePlanningTest.java
index 90e9e0054b..5ee2435a71 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/DrillSeparatePlanningTest.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/DrillSeparatePlanningTest.java
@@ -176,7 +176,7 @@ public class DrillSeparatePlanningTest extends ClusterTest {
   }
 
   private int getResultsHelper(final QueryPlanFragments planFragments) throws Exception {
-    int totalRows = 0;
+    long totalRows = 0;
     for (PlanFragment fragment : planFragments.getFragmentsList()) {
       DrillbitEndpoint assignedNode = fragment.getAssignment();
       ClientFixture fragmentClient = cluster.client(assignedNode.getAddress(), assignedNode.getUserPort());
@@ -198,6 +198,6 @@ public class DrillSeparatePlanningTest extends ClusterTest {
       totalRows += summary.recordCount();
       fragmentClient.close();
     }
-    return totalRows;
+    return Math.toIntExact(totalRows);
   }
 }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestWriter.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestWriter.java
index 2a6e1d56b5..5881ad540e 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestWriter.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestWriter.java
@@ -149,7 +149,7 @@ public class TestWriter extends BaseTestQuery {
 
     RecordBatchLoader batchLoader = new RecordBatchLoader(getAllocator());
 
-    int recordsWritten = 0;
+    long recordsWritten = 0;
     for (QueryDataBatch batch : results) {
       batchLoader.load(batch.getHeader().getDef(), batch.getData());
 
diff --git a/exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java b/exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java
index 88ba9a08c7..e7d4f037b0 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java
@@ -129,7 +129,7 @@ public class ProfileParser {
 
     public void parsePlans(String plan) {
       plans = new ArrayList<>();
-      String parts[] = plan.split("\n");
+      String[] parts = plan.split("\n");
       for (String part : parts) {
         plans.add(part);
         OperatorSummary opDef = new OperatorSummary(part);
@@ -206,10 +206,10 @@ public class ProfileParser {
   }
 
   private static List<FieldDef> parseCols(String cols) {
-    String parts[] = cols.split(", ");
+    String[] parts = cols.split(", ");
     List<FieldDef> fields = new ArrayList<>();
     for (String part : parts) {
-      String halves[] = part.split(" ");
+      String[] halves = part.split(" ");
       fields.add(new FieldDef(halves[1], halves[0]));
     }
     return fields;
@@ -241,7 +241,7 @@ public class ProfileParser {
   private void aggregateOpers() {
     for (FragInfo major : fragments.values()) {
       for (OperatorSummary opDef : major.ops) {
-        int sumPeak = 0;
+        long sumPeak = 0;
         opDef.execCount = opDef.opExecs.size();
         for (OperatorProfile op : opDef.opExecs) {
           Preconditions.checkState(major.id == op.majorFragId);
@@ -263,7 +263,7 @@ public class ProfileParser {
 
   public void buildTree() {
     int currentLevel = 0;
-    OperatorSummary opStack[] = new OperatorSummary[topoOrder.size()];
+    OperatorSummary[] opStack = new OperatorSummary[topoOrder.size()];
     for (OperatorSummary opDef : topoOrder) {
       currentLevel = opDef.globalLevel;
       opStack[currentLevel] = opDef;
@@ -307,10 +307,10 @@ public class ProfileParser {
     Matcher m = p.matcher(plan);
     if (! m.find()) { return null; }
     String frag = m.group(1);
-    String parts[] = frag.split(", ");
+    String[] parts = frag.split(", ");
     List<FieldDef> fields = new ArrayList<>();
     for (String part : parts) {
-      String halves[] = part.split(" ");
+      String[] halves = part.split(" ");
       fields.add(new FieldDef(halves[1], halves[0]));
     }
     return fields;
@@ -463,10 +463,11 @@ public class ProfileParser {
     }
 
     public long getMetric(int id) {
-      JsonValue value = metrics.get(id);
+      JsonNumber value = metrics.get(id);
       if (value == null) {
-        return 0; }
-      return ((JsonNumber) value).longValue();
+        return 0;
+      }
+      return value.longValue();
     }
 
     @Override
@@ -673,9 +674,9 @@ public class ProfileParser {
       final StringBuilder nodeBuilder = new StringBuilder();
       nodeBuilder.append(String.format("%02d-%02d ", node.majorId, node.stepId));
       String indent = indentString(indentLevel, ". ");
-      nodeBuilder.append(indent + node.name);
+      nodeBuilder.append(indent).append(node.name);
       if (node.opName != null) {
-        nodeBuilder.append(" (" + node.opName + ")");
+        nodeBuilder.append(" (").append(node.opName).append(")");
       }
       logger.info(nodeBuilder.toString());
 
@@ -835,7 +836,7 @@ public class ProfileParser {
       }
       sb.append(node.name);
       if (node.opName != null) {
-        sb.append(" (" + node.opName + ")");
+        sb.append(" (").append(node.opName).append(")");
       }
       logger.info(sb.toString());
       printTimes(node, "  ");
@@ -854,9 +855,9 @@ public class ProfileParser {
       final StringBuilder sb = new StringBuilder();
       sb.append(String.format("%02d-%02d ", node.majorId, node.stepId));
       String indent = indentString(indentLevel, ". ");
-      sb.append(indent + node.name);
+      sb.append(indent).append(node.name);
       if (node.opName != null) {
-        sb.append(" (" + node.opName + ")");
+        sb.append(" (").append(node.opName).append(")");
       }
       logger.info(sb.toString());
       indent = indentString(15);
@@ -911,7 +912,7 @@ public class ProfileParser {
   public static long percent(long value, long total) {
     if (total == 0) {
       return 0; }
-    return Math.round(value * 100 / total);
+    return Math.round(value * 100 / (double)total);
   }
 
   public List<OperatorSummary> getOpDefn(String target) {