virajjasani commented on code in PR #2134:
URL: https://github.com/apache/phoenix/pull/2134#discussion_r2067030853
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/CompactionScanner.java:
##########
@@ -180,13 +181,13 @@ public CompactionScanner(RegionCoprocessorEnvironment env,
// Empty column family and qualifier are always needed to compute
which all empty cells to retain
// even during minor compactions. If required empty cells are not
retained during
// minor compactions then we can run into the risk of partial row
expiry on next major compaction.
- this.emptyCF = SchemaUtil.getEmptyColumnFamily(table);
- this.emptyCQ = SchemaUtil.getEmptyColumnQualifier(table);
+ this.emptyCF = table != null ? SchemaUtil.getEmptyColumnFamily(table)
: EMPTY_BYTE_ARRAY;
+ this.emptyCQ = table != null ?
SchemaUtil.getEmptyColumnQualifier(table) : EMPTY_BYTE_ARRAY;
Review Comment:
Why not keep emptyCF and emptyCQ as null if PTable is null, so that we can
also incorporate this logic?
Instead of
```
if (ScanUtil.isEmptyColumn(cell, emptyCF, emptyCQ)) {
index = addEmptyColumn(result, currentColumnCell,
index, emptyColumn);
} else {
index = skipColumn(result, currentColumnCell,
retainedCells, index);
}
```
this
```
if (emptyCF != null && emptyCQ != null &&
ScanUtil.isEmptyColumn(cell, emptyCF,
emptyCQ)) {
index = addEmptyColumn(result, currentColumnCell,
index, emptyColumn);
} else {
index = skipColumn(result, currentColumnCell,
retainedCells, index);
}
```
and similarly, `if (emptyCQ == EMPTY_BYTE_ARRAY)` too will be simple null
check.
I don't think EMPTY_BYTE_ARRAY is allowed as CF:CQ, but while debugging,
null check will be more readable rather than using incorrect values of
`emptyCF` and `emptyCQ` for `ScanUtil.isEmptyColumn`?
--
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]
