ankitsinghal commented on code in PR #1353:
URL: https://github.com/apache/phoenix/pull/1353#discussion_r844322557
##########
phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java:
##########
@@ -595,25 +594,35 @@ public static boolean isSystemTable(byte[] fullTableName)
{
if (QueryConstants.SYSTEM_SCHEMA_NAME.equals(schemaName)) return true;
return false;
}
-
- // Given the splits and the rowKeySchema, find out the keys that
- public static byte[][] processSplits(byte[][] splits,
LinkedHashSet<PColumn> pkColumns, Integer saltBucketNum, boolean
defaultRowKeyOrder) throws SQLException {
- // FIXME: shouldn't this return if splits.length == 0?
- if (splits == null) return null;
- // We do not accept user specified splits if the table is salted and
we specify defaultRowKeyOrder. In this case,
- // throw an exception.
- if (splits.length > 0 && saltBucketNum != null && defaultRowKeyOrder) {
- throw new
SQLExceptionInfo.Builder(SQLExceptionCode.NO_SPLITS_ON_SALTED_TABLE).build().buildException();
- }
- // If the splits are not specified and table is salted, pre-split the
table.
- if (splits.length == 0 && saltBucketNum != null) {
- splits = SaltingUtil.getSalteByteSplitPoints(saltBucketNum);
- }
- byte[][] newSplits = new byte[splits.length][];
+
+ // Given the splits and the rowKeySchema, add the split points based on
saltBucketNum, as well
+ // as the ones passed in the parameter, call processSplit on both, and
return the union.
+ public static byte[][] processSplits(byte[][] splits,
LinkedHashSet<PColumn> pkColumns, Integer saltBucketNum) throws SQLException {
+ if (splits == null) {
+ splits = new byte[0][];
+ }
+
+ TreeSet<byte[]> allSplits = new TreeSet<>(new
Bytes.ByteArrayComparator());
+
+ // Add the salted split points, if any
+ if (saltBucketNum != null) {
+ byte[][] saltSplits =
SaltingUtil.getSaltedByteSplitPoints(saltBucketNum);
+ for (int i=0; i<saltSplits.length; i++) {
+ allSplits.add(processSplit(saltSplits[i], pkColumns));
+ }
+ }
+
+ //Add the specified split points
for (int i=0; i<splits.length; i++) {
- newSplits[i] = processSplit(splits[i], pkColumns);
+ allSplits.add(processSplit(splits[i], pkColumns));
Review Comment:
How we are ensuring that the explicit split will always start with the SALT
< buckets?
##########
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java:
##########
@@ -289,8 +289,11 @@ public MetaDataMutationResult getTable(PName tenantId,
byte[] schemaBytes, byte[
}
private static List<HRegionLocation> generateRegionLocations(byte[]
physicalName, byte[][] splits) {
- byte[] startKey = HConstants.EMPTY_START_ROW;
List<HRegionLocation> regions =
Lists.newArrayListWithExpectedSize(splits.length);
+ if (splits.length == 0) {
+ return regions;
+ }
+ byte[] startKey = HConstants.EMPTY_START_ROW;
Review Comment:
nit:
```suggestion
if (splits.length == 0) {
return regions;
}
List<HRegionLocation> regions =
Lists.newArrayListWithExpectedSize(splits.length);
byte[] startKey = HConstants.EMPTY_START_ROW;
```
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/salted/SaltedTableIT.java:
##########
@@ -63,14 +65,43 @@ public void testTableWithInvalidBucketNumber() throws
Exception {
}
@Test
- public void testTableWithSplit() throws Exception {
- try {
- createTestTable(getUrl(), "create table " + generateUniqueName() +
" (a_integer integer not null primary key) SALT_BUCKETS = 4",
- new byte[][] {{1}, {2,3}, {2,5}, {3}}, null);
+ public void testTableWithExplicitSplit() throws Exception {
+ byte[][] expectedStartKeys =
+ new byte[][] { {},
+ {1, 0, 0, 0, 0},
+ {2, 0, 0, 0, 0},
+ {2, 3, 0, 0, 0},
+ {2, 5, 0, 0, 0},
+ {3, 0, 0, 0, 0}};
+ String tableName = generateUniqueName();
+ createTestTable(getUrl(), "create table " + tableName + " (a_integer
integer not null primary key) SALT_BUCKETS = 4",
+ new byte[][] {{1}, {2,3}, {2,5}, {3}}, null);
+ Admin admin = utility.getAdmin();
+ ArrayList<byte[]> startKeys = new ArrayList<>();
+ List<RegionInfo> regionInfos =
+ admin.getRegions(TableName.valueOf(tableName));
+ for (RegionInfo regionInfo : regionInfos) {
+ startKeys.add(regionInfo.getStartKey());
+ }
+ assertTrue(Arrays.deepEquals(expectedStartKeys, startKeys.toArray()));
Review Comment:
This makes sense, we are ensuring that we never overlap two buckets.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
