joshelser commented on code in PR #1353:
URL: https://github.com/apache/phoenix/pull/1353#discussion_r843315104
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/SaltingUtil.java:
##########
@@ -123,4 +130,29 @@ public static void
addRegionStartKeyToScanStartAndStopRows(byte[] startKey, byte
scan.setStopRow(newStopRow);
}
}
+
+ public static void checkTableIsSalted(Admin admin, TableName tableName,
int buckets) throws IOException, SQLException {
+ // Heuristics to check if and existing HBase table can be a salted
Phoenix table
+ // with the specified number of buckets.
+ // May give false negatives.
+ byte[] maxStartKey = new byte[] {0} ;
+ List<RegionInfo> regionInfos =
+ admin.getRegions(tableName);
+ for (RegionInfo regionInfo : regionInfos) {
+ if (Bytes.compareTo(regionInfo.getStartKey(), maxStartKey) > 0) {
+ maxStartKey = regionInfo.getStartKey();
+ }
+ }
+ byte lastRegionStartSaltKey = maxStartKey[0];
+ if (lastRegionStartSaltKey == (byte) (buckets - 1)) {
Review Comment:
I like the vision of what you are doing here. It's 100% OK to me to try to
catch the obvious case(s) that we can and not try to catch all possibilities
for messed up region boundaries + salt.
Is it possible to add an "escape hatch" to this method for an experienced
user to give a "I know what I'm doing" override? I can't think of a case where
your logic is insufficient, but could envision a scenario where something
unexpected happens ;)
##########
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();
- }
Review Comment:
I'm trying to double-check that it's intentional that you dropped throwing
this exception
> found that merging the regions will break Phoenix, irrespective of the
value of phoenix.query.force.rowkeyorder
Did you mean merging salt split points with user-provided split points (like
I think this method was doing) or did you just mean general region merging?
--
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]
