[
https://issues.apache.org/jira/browse/PHOENIX-6587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17518436#comment-17518436
]
ASF GitHub Bot commented on PHOENIX-6587:
-----------------------------------------
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.
> Handle explicit pre-splits for new salted tables and validate splits when
> creating salted tables on existing HBase tables
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: PHOENIX-6587
> URL: https://issues.apache.org/jira/browse/PHOENIX-6587
> Project: Phoenix
> Issue Type: Bug
> Components: core
> Affects Versions: 5.2.0
> Reporter: Istvan Toth
> Assignee: Istvan Toth
> Priority: Minor
>
> Specifying the split points manually on salted tables can break them.
> The easiest thing to do is to disallow it, which ensures that the table is
> created with the default per bucket presplit.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
