gjacoby126 commented on code in PR #1497:
URL: https://github.com/apache/phoenix/pull/1497#discussion_r959960626
##########
phoenix-core/src/main/java/org/apache/hadoop/hbase/regionserver/ScannerContextUtil.java:
##########
@@ -31,12 +32,8 @@
public class ScannerContextUtil {
public static void incrementSizeProgress(ScannerContext sc, List<Cell>
cells) {
for (Cell cell : cells) {
- sc.incrementSizeProgress(CellUtil.estimatedSerializedSizeOf(cell),
- CellUtil.estimatedHeapSizeOf(cell));
+
sc.incrementSizeProgress(PrivateCellUtil.estimatedSerializedSizeOf(cell),
Review Comment:
This is also IA.Private, but I don't offhand see an IA.Public or
IA.LimitedPrivate alternative.
##########
phoenix-core/src/main/java/org/apache/phoenix/hbase/index/Indexer.java:
##########
@@ -445,7 +445,7 @@ public void
preBatchMutateWithExceptions(ObserverContext<RegionCoprocessorEnviro
// inconsistencies as this case isn't handled correctly
currently).
for (List<Cell> cells : m.getFamilyCellMap().values()) {
for (Cell cell : cells) {
- CellUtil.setTimestamp(cell, now);
+ PrivateCellUtil.setTimestamp(cell, now);
Review Comment:
ditto: this can stay CellUtil.setTimestamp
##########
phoenix-core/src/test/java/org/apache/phoenix/util/ScanUtilTest.java:
##########
@@ -515,9 +522,15 @@ public void testPhoenixTTLUtilMethods() throws
SQLException {
KeyValue.Type type43 = KeyValue.Type.Put;
String value43 = "test.value.43";
long seqId43 = 1043L;
- Cell cell43 = CellUtil.createCell(Bytes.toBytes(row),
- emptyColumnFamilyName, Bytes.toBytes(columnName),
- timestamp43, type43.getCode(), Bytes.toBytes(value43),
seqId43);
+ Cell cell43 =
ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY)
+ .setRow(Bytes.toBytes(row))
+ .setFamily(emptyColumnFamilyName)
+ .setQualifier(Bytes.toBytes(columnName))
+ .setTimestamp(timestamp43)
Review Comment:
These two in this file do need the ExtendedCellBuilderFactory because of the
sequence id. Outside of the scope of this PR, but in the future we might want
to figure out an IA.LimitedPrivate/Public way to do this, if possible.
##########
phoenix-core/src/test/java/org/apache/phoenix/expression/OrExpressionTest.java:
##########
@@ -136,13 +138,14 @@ private KeyValueColumnExpression kvExpr(final String
name) {
private Cell createCell(String name, Boolean value) {
byte[] valueBytes = value == null ? null : value ? PBoolean.TRUE_BYTES
: PBoolean.FALSE_BYTES;
- return CellUtil.createCell(
- Bytes.toBytes("row"),
- QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES,
- Bytes.toBytes(name),
- 1,
- KeyValue.Type.Put.getCode(),
- valueBytes);
+ return ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY)
+ .setRow(Bytes.toBytes("row"))
Review Comment:
This can also be CellBuilderFactory/CellBuilder, I think
##########
phoenix-core/src/test/java/org/apache/phoenix/index/PrepareIndexMutationsForRebuildTest.java:
##########
@@ -751,14 +753,28 @@ byte[] generateIndexRowKey(String indexVal) {
void addCellToPutMutation(Put put, byte[] family, byte[] column, long ts,
byte[] value) throws Exception {
byte[] rowKey = put.getRow();
- Cell cell = CellUtil.createCell(rowKey, family, column, ts,
KeyValue.Type.Put.getCode(), value);
+ Cell cell =
ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY)
+ .setRow(rowKey)
+ .setFamily(family)
+ .setQualifier(column)
+ .setTimestamp(ts)
+ .setType(Cell.Type.Put)
+ .setValue(value)
+ .build();
put.add(cell);
}
- void addCellToDelMutation(Delete del, byte[] family, byte[] column, long
ts, KeyValue.Type type) throws Exception {
+ void addCellToDelMutation(Delete del, byte[] family, byte[] column, long
ts, Cell.Type type) throws Exception {
byte[] rowKey = del.getRow();
- Cell cell = CellUtil.createCell(rowKey, family, column, ts,
type.getCode(), null);
- del.addDeleteMarker(cell);
+ Cell cell =
ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY)
+ .setRow(rowKey)
+ .setFamily(family)
+ .setQualifier(column)
Review Comment:
This can also be CellBuilderFactory/CellBuilder, I think
##########
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/MultiHfileOutputFormat.java:
##########
@@ -195,7 +196,7 @@ public void write(TableRowkeyPair row, V cell)
// we now have the proper WAL writer. full steam ahead
if (cell.getTimestamp() == HConstants.LATEST_TIMESTAMP) {
- CellUtil.setTimestamp(cell, this.now);
+ PrivateCellUtil.setTimestamp(cell, this.now);
Review Comment:
We can use CellUtil.setTimestamp for this
##########
phoenix-core/src/main/java/org/apache/phoenix/util/TransactionUtil.java:
##########
@@ -64,11 +66,25 @@ public static boolean isDeleteFamily(Cell cell) {
}
private static Cell newDeleteFamilyMarker(byte[] row, byte[] family, long
timestamp) {
- return CellUtil.createCell(row, family, FAMILY_DELETE_MARKER,
timestamp, KeyValue.Type.Put.getCode(), HConstants.EMPTY_BYTE_ARRAY);
+ return ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY)
Review Comment:
Looks like both of these can use the normal IA.Public
CellBuilder/CellBuilderFactory
##########
phoenix-core/src/test/java/org/apache/phoenix/index/PrepareIndexMutationsForRebuildTest.java:
##########
@@ -751,14 +753,28 @@ byte[] generateIndexRowKey(String indexVal) {
void addCellToPutMutation(Put put, byte[] family, byte[] column, long ts,
byte[] value) throws Exception {
byte[] rowKey = put.getRow();
- Cell cell = CellUtil.createCell(rowKey, family, column, ts,
KeyValue.Type.Put.getCode(), value);
+ Cell cell =
ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY)
+ .setRow(rowKey)
+ .setFamily(family)
+ .setQualifier(column)
Review Comment:
This can also be CellBuilderFactory/CellBuilder, I think
##########
phoenix-core/src/it/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilterIT.java:
##########
@@ -120,7 +122,12 @@ public void testOtherTablesAutoPass() throws Exception {
//Cell is nonsense but we should auto pass because the table name's not
System.Catalog
WAL.Entry entry = new WAL.Entry(new WALKeyImpl(REGION,
TableName.valueOf(TestUtil.ENTITY_HISTORY_TABLE_NAME),
System.currentTimeMillis()), new WALEdit());
- entry.getEdit().add(CellUtil.createCell(Bytes.toBytes("foo")));
+ entry.getEdit().add(
+ ExtendedCellBuilderFactory.create(
Review Comment:
ExtendedCellBuilderFactory is IA.Private -- looks like this could use
CellBuilderFactory, which is IA.Public.
##########
phoenix-core/src/main/java/org/apache/phoenix/hbase/index/IndexRegionObserver.java:
##########
@@ -637,7 +640,7 @@ public static void
setTimestamps(MiniBatchOperationInProgress<Mutation> miniBatc
private static void setTimestampOnMutation(Mutation m, long ts) throws
IOException {
for (List<Cell> cells : m.getFamilyCellMap().values()) {
for (Cell cell : cells) {
- CellUtil.setTimestamp(cell, ts);
+ PrivateCellUtil.setTimestamp(cell, ts);
Review Comment:
PrivateCellUtil is IA.Private.
The Deprecation notes on CellUtil.setTimestamp says that it's deprecated "As
of HBase-2.0. Will be a LimitedPrivate API in HBase-3.0." I checked in
3.0-alpha and it's IA.LimitedPrivate(COPROC), which means it's still OK for us
to use CellUtil.setTimestamp
##########
phoenix-core/src/test/java/org/apache/phoenix/expression/AndExpressionTest.java:
##########
@@ -136,13 +138,14 @@ private KeyValueColumnExpression kvExpr(final String
name) {
private Cell createCell(String name, Boolean value) {
byte[] valueBytes = value == null ? null : value ? PBoolean.TRUE_BYTES
: PBoolean.FALSE_BYTES;
- return CellUtil.createCell(
- Bytes.toBytes("row"),
- QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES,
- Bytes.toBytes(name),
- 1,
- KeyValue.Type.Put.getCode(),
- valueBytes);
+ return ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY)
+ .setRow(Bytes.toBytes("row"))
Review Comment:
This can also be CellBuilderFactory/CellBuilder, I think
--
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]
