bharathv commented on a change in pull request #2591:
URL: https://github.com/apache/hbase/pull/2591#discussion_r513757980
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSplit.java
##########
@@ -184,9 +184,24 @@ public TableSplit(final byte [] tableName, byte[]
startRow, byte[] endRow,
* @param startRow The start row of the split.
* @param endRow The end row of the split.
* @param location The location of the region.
+ * @param encodedRegionName The region ID.
+ * @param length Size of region in bytes
*/
public TableSplit(TableName tableName, byte[] startRow, byte[] endRow,
- final String location) {
+ final String location, final String encodedRegionName, long length) {
+ this(tableName, null, startRow, endRow, location, encodedRegionName,
length);
Review comment:
Why do we need a new constructor? Just pass null wherever it is not
needed?
##########
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableInputFormatScanBase.java
##########
@@ -271,6 +272,11 @@ public void testNumOfSplits(int splitsPerRegion, int
expectedNumOfSplits) throws
TableInputFormat tif = new TableInputFormat();
tif.setConf(job.getConfiguration());
List<InputSplit> splits = tif.getSplits(job);
+ for (InputSplit split : splits) {
+ TableSplit tableSplit = (TableSplit) split;
+ Assert.assertEquals(tableSplit.getScan().getStartRow(),
HConstants.EMPTY_START_ROW);
Review comment:
nit: I think these checks are not tight enough since they don't
differentiate between a full table scan object and a no scan object, instead I
think it should be something like
assertTrue(tableSplit.getScanAsString().isEmpty()) or some such...
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSplit.java
##########
@@ -184,9 +184,24 @@ public TableSplit(final byte [] tableName, byte[]
startRow, byte[] endRow,
* @param startRow The start row of the split.
* @param endRow The end row of the split.
* @param location The location of the region.
+ * @param encodedRegionName The region ID.
+ * @param length Size of region in bytes
*/
public TableSplit(TableName tableName, byte[] startRow, byte[] endRow,
- final String location) {
+ final String location, final String encodedRegionName, long length) {
+ this(tableName, null, startRow, endRow, location, encodedRegionName,
length);
+ }
+
+ /**
+ * Creates a new instance without a scanner.
Review comment:
Mind adding a javadoc for "scan" object? What it means to be null..
----------------------------------------------------------------
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
