You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/08/16 21:41:06 UTC
[GitHub] [phoenix] gjacoby126 commented on a change in pull request #1286: PHOENIX-6528 Fix view index read repair for the pks with variable length
gjacoby126 commented on a change in pull request #1286:
URL: https://github.com/apache/phoenix/pull/1286#discussion_r689876158
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.java
##########
@@ -522,6 +527,9 @@ private void createTableForRowKeyTestsAndVerify(Connection conn, String viewPkCo
String actualExplainPlan = QueryUtil.getExplainPlan(rs1);
assertTrue(actualExplainPlan.contains("_IDX_" + fullTableName));
+ SingleCellIndexIT.dumpTable("_IDX_" + fullTableName);
Review comment:
Please remove dumpTable statements before merging
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -846,18 +847,19 @@ public void setDataEncodingScheme(QualifierEncodingScheme sc) {
// Write separator byte if variable length unless it's the last field in the schema
// (but we still need to write it if it's DESC to ensure sort order is correct).
byte sepByte = SchemaUtil.getSeparatorByte(rowKeyOrderOptimizable, ptr.getLength() == 0, dataRowKeySchema.getField(i));
- if (!dataRowKeySchema.getField(i).getDataType().isFixedWidth() && (((i+1) != dataRowKeySchema.getFieldCount()) || sepByte == QueryConstants.DESC_SEPARATOR_BYTE)) {
+ if (!dataRowKeySchema.getField(i).getDataType().isFixedWidth()){
Review comment:
Need to fix comment on line 848 which refers to the logic you're changing
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
##########
@@ -846,18 +847,19 @@ public void setDataEncodingScheme(QualifierEncodingScheme sc) {
// Write separator byte if variable length unless it's the last field in the schema
// (but we still need to write it if it's DESC to ensure sort order is correct).
byte sepByte = SchemaUtil.getSeparatorByte(rowKeyOrderOptimizable, ptr.getLength() == 0, dataRowKeySchema.getField(i));
- if (!dataRowKeySchema.getField(i).getDataType().isFixedWidth() && (((i+1) != dataRowKeySchema.getFieldCount()) || sepByte == QueryConstants.DESC_SEPARATOR_BYTE)) {
+ if (!dataRowKeySchema.getField(i).getDataType().isFixedWidth()){
output.writeByte(sepByte);
+ trailingVariableWidthColumnNum++;
+ } else {
+ trailingVariableWidthColumnNum = 0;
}
}
int length = stream.size();
- int minLength = length - maxTrailingNulls;
byte[] dataRowKey = stream.getBuffer();
// Remove trailing nulls
- int index = dataRowKeySchema.getFieldCount() - 1;
- while (index >= 0 && !dataRowKeySchema.getField(index).getDataType().isFixedWidth() && length > minLength && dataRowKey[length-1] == QueryConstants.SEPARATOR_BYTE) {
+ while (trailingVariableWidthColumnNum > 0 && dataRowKey[length-1] == QueryConstants.SEPARATOR_BYTE) {
Review comment:
What if it's the DESC_SEPARATOR_BYTE? wouldn't we need to continue the loop and decrement trailingVariableWidthColumnNum? The old logic only needed to check the asc SEPARATOR_BYTE but that's because it only wrote a SEPARATOR_BYTE in the asc case, but that's changed now above.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org