kevinrr888 commented on PR #4852:
URL: https://github.com/apache/accumulo/pull/4852#issuecomment-2327280245
### List of my changes so it's easy to track the differences b/w my impl and
the prev impl
These notes may be verbose, but I figure it is better to have as much detail
as possible about differences in my impl vs the prev impl so it is easier to
verify no existing functionality is lost or changed.
- Validation of the tablet row now done in method `validateTabletRow()`
- Previously had logic:
`
if (containsSemiC) {
if (row.length == 0) {
violations = addIfNotPresent(violations, 4);
}
}
`
If containsSemiC is true, row length can't be 0. Removed statement
- Validation of TabletColumnFamily now done in `validateTabletFamily()`
- Check
`
if (colValLen > 0 && (violations == null ||
!violations.contains((short) 4))) {
`
Changed to
`
if (colValLen > 0 && !violations.contains((short) 4)) {
`
- Moved check
`
if (columnUpdate.getValue().length == 0 &&
!(columnFamily.equals(ScanFileColumnFamily.NAME)
|| columnFamily.equals(LogColumnFamily.NAME))) {
violations = addViolation(violations, 6);
}
`
to method `validateColValLen()`
- Validation of BulkFileColumnFamily
- Assumed
`
try {
StoredTabletFile.validate(new
String(columnUpdate.getColumnQualifier(), UTF_8));
} catch (RuntimeException e) {
violations = addViolation(violations, 9);
}
`
is equivalent to
`
try {
StoredTabletFile.of(new Text(update.getColumnQualifier()));
} catch (RuntimeException e) {
violations = addViolation(violations, 9);
}
`
There were places that did one or the other, changed to just check
one way since they seemed equivalent. If these are not equivalent, will need to
change this.
- No longer need check
`
if (!columnUpdate.isDeleted() && !checkedBulk) {
`
checkedBulk no longer needed since this is handled automatically
with the new way BulkFileColumnFamily is validated. Checking if the column
update is deleted here was never needed so removed.
It is always the case that !columnUpdate.isDeleted() from earlier
check in the loop.
- Instead of the innefficient nested loop, validation of this
column is deferred until after the other columns are validated
- Created new class `BulkFileColData` which stores the
data needed to validate a BulkFileColumn update.
- The check:
`
if (!isValidColumn(columnUpdate)) {
violations = addViolation(violations, 2);
}
`
was confusingly placed in original impl, but I believe my impl is equivalent.
- Violations are no longer added to and returned, only added to. For this,
initialized violations at the start as a list instead of null. Null added extra
unnecessary complexity.
- Not sure if there was a specific reason for using null before; in the
case of MetadataConstraints it appeared unnecessary. The other implementations
of `Constraint` still return null for `check()`.
- Removed comments referencing issue#3505 since they have now been addressed
--
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]
