Skip to content

Commit 9f26405

Browse files
committed
PLUGIN-1925: Add detailed comments to the code
1 parent 25f3c54 commit 9f26405

File tree

3 files changed

+22
-10
lines changed

3 files changed

+22
-10
lines changed

database-commons/src/main/java/io/cdap/plugin/db/source/DataDrivenETLDBInputFormat.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,9 @@ public Connection createConnection() {
132132

133133
@Override
134134
protected DBSplitter getSplitter(int sqlDataType) {
135+
// Use SafeBigDecimalSplitter for columns having high precision decimal or numeric columns
135136
if (sqlDataType == Types.NUMERIC || sqlDataType == Types.DECIMAL) {
136-
return new CustomBigDecimalSplitter();
137+
return new SafeBigDecimalSplitter();
137138
}
138139
return super.getSplitter(sqlDataType);
139140
}

database-commons/src/main/java/io/cdap/plugin/db/source/CustomBigDecimalSplitter.java renamed to database-commons/src/main/java/io/cdap/plugin/db/source/SafeBigDecimalSplitter.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,22 @@
2121
import java.math.RoundingMode;
2222

2323
/**
24-
* Custom implementation of {@link BigDecimalSplitter} to ensures safe and precise division of BigDecimal values while
25-
* calculating split points for NUMERIC and DECIMAL types.
24+
* Safe implementation of {@link BigDecimalSplitter} to ensure precise division of BigDecimal values while calculating
25+
* split points for NUMERIC and DECIMAL types.
26+
*
27+
* <p>Problem: The default {@link BigDecimalSplitter} implementation may return 0 when the numerator is smaller than the
28+
* denominator (e.g., 1 / 4 = 0), due to the lack of a defined scale for division. Since the result (0) is smaller than
29+
* {@link BigDecimalSplitter#MIN_INCREMENT} (i.e. {@code 10000 * Double.MIN_VALUE}), the split size defaults to
30+
* {@code MIN_INCREMENT}, leading to an excessive number of splits (~10M) and potential OOM errors.</p>
31+
*
32+
* <p>Fix: This implementation derives scale from column metadata, adds a buffer of 5 decimal places, and uses
33+
* {@link RoundingMode#HALF_UP} as the rounding mode.</p
34+
*
35+
* <p>Note: This class is used by {@link DataDrivenETLDBInputFormat}.</p>
2636
*/
27-
public class CustomBigDecimalSplitter extends BigDecimalSplitter {
37+
public class SafeBigDecimalSplitter extends BigDecimalSplitter {
2838

39+
/* An additional buffer of +5 digits is applied to preserve accuracy during division. */
2940
public static final int SCALE_BUFFER = 5;
3041
/**
3142
* Performs safe division with correct scale handling.
@@ -37,7 +48,7 @@ public class CustomBigDecimalSplitter extends BigDecimalSplitter {
3748
*/
3849
@Override
3950
protected BigDecimal tryDivide(BigDecimal numerator, BigDecimal denominator) {
40-
// Derive scale from numerator/denominator + buffer
51+
// Determine the required scale for the division and add a buffer to ensure accuracy
4152
int effectiveScale = Math.max(numerator.scale(), denominator.scale()) + SCALE_BUFFER;
4253
return numerator.divide(denominator, effectiveScale, RoundingMode.HALF_UP);
4354
}

database-commons/src/test/java/io/cdap/plugin/db/source/CustomBigDecimalSplitterTest.java renamed to database-commons/src/test/java/io/cdap/plugin/db/source/SafeBigDecimalSplitterTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@
3333
import static org.mockito.Mockito.when;
3434

3535
/**
36-
* Test class for {@link CustomBigDecimalSplitter}
36+
* Test class for {@link SafeBigDecimalSplitter}
3737
*/
38-
public class CustomBigDecimalSplitterTest {
39-
private final CustomBigDecimalSplitter splitter = new CustomBigDecimalSplitter();
38+
public class SafeBigDecimalSplitterTest {
39+
private final SafeBigDecimalSplitter splitter = new SafeBigDecimalSplitter();
4040

4141
@Test
4242
public void testSmallRangeDivision() {
@@ -75,7 +75,7 @@ public void testSplits() throws SQLException {
7575
when(conf.getInt("mapreduce.job.maps", 1)).thenReturn(numSplits);
7676
when(resultSet.getBigDecimal(1)).thenReturn(minVal);
7777
when(resultSet.getBigDecimal(2)).thenReturn(maxVal);
78-
BigDecimalSplitter bigDecimalSplitter = new CustomBigDecimalSplitter();
78+
BigDecimalSplitter bigDecimalSplitter = new SafeBigDecimalSplitter();
7979
List<InputSplit> actualSplits = bigDecimalSplitter.split(conf, resultSet, "id");
8080
assertEquals(numSplits, actualSplits.size());
8181
}
@@ -91,7 +91,7 @@ public void testSplitsWithMinValueEqualToMaxValue() throws SQLException {
9191
when(conf.getInt("mapreduce.job.maps", 1)).thenReturn(numSplits);
9292
when(resultSet.getBigDecimal(1)).thenReturn(minVal);
9393
when(resultSet.getBigDecimal(2)).thenReturn(maxVal);
94-
BigDecimalSplitter bigDecimalSplitter = new CustomBigDecimalSplitter();
94+
BigDecimalSplitter bigDecimalSplitter = new SafeBigDecimalSplitter();
9595
List<InputSplit> actualSplits = bigDecimalSplitter.split(conf, resultSet, "id");
9696
assertEquals(numSplits, actualSplits.size());
9797
}

0 commit comments

Comments
 (0)