From dcf845c25eb9bd619a999d16ce9e2f548ce7b491 Mon Sep 17 00:00:00 2001 From: James Taylor Date: Mon, 20 Jul 2015 17:52:53 -0700 Subject: [PATCH] PHOENIX-2120 Padding character is not inverted as required for DESC CHAR columns --- .../phoenix/end2end/LpadFunctionIT.java | 48 +++- .../org/apache/phoenix/compile/KeyPart.java | 8 +- .../phoenix/compile/WhereOptimizer.java | 132 ++++++--- .../UngroupedAggregateRegionObserver.java | 33 ++- .../phoenix/expression/LiteralExpression.java | 3 +- .../expression/function/InvertFunction.java | 8 +- .../expression/function/PrefixFunction.java | 13 +- .../expression/function/RTrimFunction.java | 15 +- .../function/RoundDateExpression.java | 12 +- .../function/RoundDecimalExpression.java | 22 +- .../query/ConnectionQueryServicesImpl.java | 8 +- .../org/apache/phoenix/schema/PTableImpl.java | 25 +- .../apache/phoenix/schema/types/PBinary.java | 28 +- .../apache/phoenix/schema/types/PChar.java | 11 + .../phoenix/schema/types/PDataType.java | 1 + .../apache/phoenix/util/PhoenixRuntime.java | 43 ++- .../org/apache/phoenix/util/StringUtil.java | 7 - .../org/apache/phoenix/util/UpgradeUtil.java | 256 ++++++++++++------ 18 files changed, 495 insertions(+), 178 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/LpadFunctionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/LpadFunctionIT.java index 4aa66c20fda..40701033cb4 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/LpadFunctionIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/LpadFunctionIT.java @@ -16,6 +16,7 @@ */ package org.apache.phoenix.end2end; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -27,8 +28,10 @@ import java.util.ArrayList; import java.util.List; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.phoenix.query.QueryConstants; +import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.TestUtil; -import org.junit.Ignore; import org.junit.Test; import com.google.common.collect.Lists; @@ -107,26 +110,59 @@ private void testLpad(Connection conn, List inputList, int length, List< testLpad(conn, inputList, length, fillStringList, "pk", expectedOutputList); } - @Ignore @Test public void testCharPadding() throws Exception { ResultSet rs; Connection conn = DriverManager.getConnection(getUrl()); + + conn.createStatement().execute("CREATE TABLE t (k CHAR(3) PRIMARY KEY)"); + conn.createStatement().execute("UPSERT INTO t VALUES('a')"); + conn.createStatement().execute("UPSERT INTO t VALUES('ab')"); + conn.commit(); + rs = conn.createStatement().executeQuery("SELECT * FROM t ORDER BY k"); + assertTrue(rs.next()); + assertEquals("a", rs.getString(1)); + assertTrue(rs.next()); + assertEquals("ab", rs.getString(1)); + assertFalse(rs.next()); conn.createStatement().execute("CREATE TABLE tdesc (k CHAR(3) PRIMARY KEY DESC)"); conn.createStatement().execute("UPSERT INTO tdesc VALUES('a')"); + conn.createStatement().execute("UPSERT INTO tdesc VALUES('ab')"); conn.commit(); - rs = conn.createStatement().executeQuery("SELECT * FROM tdesc"); + rs = conn.createStatement().executeQuery("SELECT * FROM tdesc ORDER BY k DESC"); + assertTrue(rs.next()); + assertEquals("ab", rs.getString(1)); assertTrue(rs.next()); assertEquals("a", rs.getString(1)); assertFalse(rs.next()); + } + + @Test + public void testBinaryPadding() throws Exception { + ResultSet rs; + Connection conn = DriverManager.getConnection(getUrl()); - conn.createStatement().execute("CREATE TABLE t (k CHAR(3) PRIMARY KEY)"); + conn.createStatement().execute("CREATE TABLE t (k BINARY(3) PRIMARY KEY)"); conn.createStatement().execute("UPSERT INTO t VALUES('a')"); + conn.createStatement().execute("UPSERT INTO t VALUES('ab')"); conn.commit(); - rs = conn.createStatement().executeQuery("SELECT * FROM t"); + rs = conn.createStatement().executeQuery("SELECT * FROM t ORDER BY k"); assertTrue(rs.next()); - assertEquals("a", rs.getString(1)); + assertArrayEquals(ByteUtil.concat(Bytes.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, QueryConstants.SEPARATOR_BYTE_ARRAY), rs.getBytes(1)); + assertTrue(rs.next()); + assertArrayEquals(ByteUtil.concat(Bytes.toBytes("ab"), QueryConstants.SEPARATOR_BYTE_ARRAY), rs.getBytes(1)); + assertFalse(rs.next()); + + conn.createStatement().execute("CREATE TABLE tdesc (k BINARY(3) PRIMARY KEY DESC)"); + conn.createStatement().execute("UPSERT INTO tdesc VALUES('a')"); + conn.createStatement().execute("UPSERT INTO tdesc VALUES('ab')"); + conn.commit(); + rs = conn.createStatement().executeQuery("SELECT * FROM tdesc ORDER BY k DESC"); + assertTrue(rs.next()); + assertArrayEquals(ByteUtil.concat(Bytes.toBytes("ab"), QueryConstants.SEPARATOR_BYTE_ARRAY), rs.getBytes(1)); + assertTrue(rs.next()); + assertArrayEquals(ByteUtil.concat(Bytes.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, QueryConstants.SEPARATOR_BYTE_ARRAY), rs.getBytes(1)); assertFalse(rs.next()); } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/KeyPart.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/KeyPart.java index 4eb53d39a59..55385f29b51 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/KeyPart.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/KeyPart.java @@ -20,10 +20,10 @@ import java.util.List; import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp; - import org.apache.phoenix.expression.Expression; import org.apache.phoenix.query.KeyRange; import org.apache.phoenix.schema.PColumn; +import org.apache.phoenix.schema.PTable; /** * @@ -74,4 +74,10 @@ public interface KeyPart { * @return the primary key column for this key part */ public PColumn getColumn(); + + /** + * Gets the table metadata object associated with this key part + * @return the table for this key part + */ + public PTable getTable(); } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java index 332f2931658..a270f126cc4 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java @@ -63,7 +63,7 @@ import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.ValueSchema.Field; import org.apache.phoenix.schema.tuple.Tuple; -import org.apache.phoenix.schema.types.PArrayDataType; +import org.apache.phoenix.schema.types.PBinary; import org.apache.phoenix.schema.types.PChar; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PVarbinary; @@ -322,6 +322,14 @@ public static Expression pushKeyExpressionsToScan(StatementContext context, Filt } } + private static int computeUnpaddedLength(byte[] b, byte padByte) { + int len = b.length; + while (len > 0 && b[len - 1] == padByte) { + len--; + } + return len; + } + // Special hack for PHOENIX-2067 to change the constant array to match // the separators used for descending, variable length arrays. // Note that there'd already be a coerce expression around the constant @@ -335,27 +343,60 @@ private static List transformKeyRangesIfNecessary(KeyExpressionVisitor PTable table = context.getCurrentTable().getTable(); // Constants are always build with rowKeyOptimizable as true, using the correct separators // We only need to do this conversion if we have a table that has not yet been converted. - if (type != null && type.isArrayType() && column.getSortOrder() == SortOrder.DESC && !PArrayDataType.arrayBaseType(type).isFixedWidth() && !table.rowKeyOrderOptimizable()) { - ImmutableBytesWritable ptr = context.getTempPtr(); - List newKeyRanges = Lists.newArrayListWithExpectedSize(keyRanges.size()); - for (KeyRange keyRange : keyRanges) { - byte[] lower = keyRange.getLowerRange(); - if (!keyRange.lowerUnbound()) { - ptr.set(lower);; - type.coerceBytes(ptr, null, type, null, null, SortOrder.DESC, null, null, SortOrder.DESC, false); - lower = ByteUtil.copyKeyBytesIfNecessary(ptr); + if (type != null && !table.rowKeyOrderOptimizable()) { + if (type.isArrayType() && column.getSortOrder() == SortOrder.DESC) { + ImmutableBytesWritable ptr = context.getTempPtr(); + List newKeyRanges = Lists.newArrayListWithExpectedSize(keyRanges.size()); + for (KeyRange keyRange : keyRanges) { + byte[] lower = keyRange.getLowerRange(); + if (!keyRange.lowerUnbound()) { + ptr.set(lower); + type.coerceBytes(ptr, null, type, null, null, SortOrder.DESC, null, null, SortOrder.DESC, false); + lower = ByteUtil.copyKeyBytesIfNecessary(ptr); + } + byte[] upper = keyRange.getUpperRange(); + if (!keyRange.upperUnbound()) { + ptr.set(upper); + type.coerceBytes(ptr, null, type, null, null, SortOrder.DESC, null, null, SortOrder.DESC, false); + upper = ByteUtil.copyKeyBytesIfNecessary(ptr); + } + keyRange = KeyRange.getKeyRange(lower, keyRange.isLowerInclusive(), upper, keyRange.isUpperInclusive()); + newKeyRanges.add(keyRange); } - byte[] upper = keyRange.getUpperRange(); - if (!keyRange.upperUnbound()) { - ptr.set(upper);; - type.coerceBytes(ptr, null, type, null, null, SortOrder.DESC, null, null, SortOrder.DESC, false); - upper = ByteUtil.copyKeyBytesIfNecessary(ptr); + return newKeyRanges; + } else if (type == PBinary.INSTANCE || (column.getSortOrder() == SortOrder.DESC && type == PChar.INSTANCE)) { + // Since the table has not been upgraded, we need to replace the correct trailing byte back to the incorrect + // trailing byte so that we form the correct start/stop key + List newKeyRanges = Lists.newArrayListWithExpectedSize(keyRanges.size()); + byte byteToReplace; + if (type == PBinary.INSTANCE) { + byteToReplace = column.getSortOrder() == SortOrder.ASC ? QueryConstants.SEPARATOR_BYTE : QueryConstants.DESC_SEPARATOR_BYTE; + } else { + byteToReplace = StringUtil.INVERTED_SPACE_UTF8; + } + for (KeyRange keyRange : keyRanges) { + byte[] lower = keyRange.getLowerRange(); + if (!keyRange.lowerUnbound()) { + int len = computeUnpaddedLength(lower, byteToReplace); + if (len != lower.length) { + lower = Arrays.copyOf(lower, len); + lower = StringUtil.padChar(lower, lower.length); + } + } + byte[] upper = keyRange.getUpperRange(); + if (!keyRange.upperUnbound()) { + int len = computeUnpaddedLength(upper, byteToReplace); + if (len != upper.length) { + upper = Arrays.copyOf(upper, len); + upper = StringUtil.padChar(upper, upper.length); + } + } + keyRange = KeyRange.getKeyRange(lower, keyRange.isLowerInclusive(), upper, keyRange.isUpperInclusive()); + newKeyRanges.add(keyRange); } - keyRange = KeyRange.getKeyRange(lower, keyRange.isLowerInclusive(), upper, keyRange.isUpperInclusive()); - newKeyRanges.add(keyRange); + return newKeyRanges; } - return newKeyRanges; } } return keyRanges; @@ -532,7 +573,7 @@ private KeySlots newKeyParts(KeySlot slot, Expression extractNode, List extractNodes = extractNode == null || slot.getKeyPart().getExtractNodes().isEmpty() ? Collections.emptyList() : Collections.singletonList(extractNode); - return new SingleKeySlot(new BaseKeyPart(slot.getKeyPart().getColumn(), extractNodes), slot.getPKPosition(), slot.getPKSpan(), keyRanges, minMaxRange, slot.getOrderPreserving()); + return new SingleKeySlot(new BaseKeyPart(table, slot.getKeyPart().getColumn(), extractNodes), slot.getPKPosition(), slot.getPKSpan(), keyRanges, minMaxRange, slot.getOrderPreserving()); } private KeySlots newKeyParts(KeySlot slot, List extractNodes, List keyRanges, KeyRange minMaxRange) { @@ -540,7 +581,7 @@ private KeySlots newKeyParts(KeySlot slot, List extractNodes, List childSlots) { @@ -648,6 +689,11 @@ public List getExtractNodes() { public PColumn getColumn() { return childPart.getColumn(); } + + @Override + public PTable getTable() { + return childPart.getTable(); + } }, slot.getPKPosition(), slot.getKeyRanges()); } @@ -724,7 +770,7 @@ private KeySlots andKeySlots(AndExpression andExpression, List childSl if (!minMaxExtractNodes.isEmpty()) { if (keySlot[initPosition] == null) { - keySlot[initPosition] = new KeySlot(new BaseKeyPart(table.getPKColumns().get(initPosition), minMaxExtractNodes), initPosition, 1, EVERYTHING_RANGES, null); + keySlot[initPosition] = new KeySlot(new BaseKeyPart(table, table.getPKColumns().get(initPosition), minMaxExtractNodes), initPosition, 1, EVERYTHING_RANGES, null); } else { keySlot[initPosition] = keySlot[initPosition].concatExtractNodes(minMaxExtractNodes); } @@ -841,7 +887,7 @@ private KeySlots orKeySlots(OrExpression orExpression, List childSlots slotRanges = Collections.emptyList(); } if (theSlot == null) { - theSlot = new KeySlot(new BaseKeyPart(table.getPKColumns().get(initialPos), slotExtractNodes), initialPos, 1, EVERYTHING_RANGES, null); + theSlot = new KeySlot(new BaseKeyPart(table, table.getPKColumns().get(initialPos), slotExtractNodes), initialPos, 1, EVERYTHING_RANGES, null); } else if (minMaxRange != KeyRange.EMPTY_RANGE && !slotExtractNodes.isEmpty()) { theSlot = theSlot.concatExtractNodes(slotExtractNodes); } @@ -926,7 +972,7 @@ public KeySlots visitLeave(RowValueConstructorExpression node, List ch @Override public KeySlots visit(RowKeyColumnExpression node) { PColumn column = table.getPKColumns().get(node.getPosition()); - return new SingleKeySlot(new BaseKeyPart(column, Collections.singletonList(node)), node.getPosition(), 1, EVERYTHING_RANGES); + return new SingleKeySlot(new BaseKeyPart(table, column, Collections.singletonList(node)), node.getPosition(), 1, EVERYTHING_RANGES); } @Override @@ -997,7 +1043,8 @@ public KeySlots visitLeave(LikeExpression node, List childParts) { KeySlots childSlots = childParts.get(0); KeySlot childSlot = childSlots.iterator().next(); final String startsWith = node.getLiteralPrefix(); - byte[] key = PVarchar.INSTANCE.toBytes(startsWith, node.getChildren().get(0).getSortOrder()); + SortOrder sortOrder = node.getChildren().get(0).getSortOrder(); + byte[] key = PVarchar.INSTANCE.toBytes(startsWith, sortOrder); // If the expression is an equality expression against a fixed length column // and the key length doesn't match the column length, the expression can // never be true. @@ -1014,8 +1061,15 @@ public KeySlots visitLeave(LikeExpression node, List childParts) { byte[] upperRange = ByteUtil.nextKey(key); Integer columnFixedLength = column.getMaxLength(); if (type.isFixedWidth() && columnFixedLength != null) { - lowerRange = StringUtil.padChar(lowerRange, columnFixedLength); - upperRange = StringUtil.padChar(upperRange, columnFixedLength); + if (table.rowKeyOrderOptimizable()) { + // Always use minimum byte to fill as otherwise our key is bigger + // that it should be when the sort order is descending. + lowerRange = type.pad(lowerRange, columnFixedLength, SortOrder.ASC); + upperRange = type.pad(upperRange, columnFixedLength, SortOrder.ASC); + } else { // TODO: remove broken logic once tables are required to have been upgraded for PHOENIX-2067 and PHOENIX-2120 + lowerRange = StringUtil.padChar(lowerRange, columnFixedLength); + upperRange = StringUtil.padChar(upperRange, columnFixedLength); + } } KeyRange keyRange = type.getKeyRange(lowerRange, true, upperRange, false); // Only extract LIKE expression if pattern ends with a wildcard and everything else was extracted @@ -1117,7 +1171,7 @@ public List getKeyRanges() { public final KeySlot concatExtractNodes(List extractNodes) { return new KeySlot( - new BaseKeyPart(this.getKeyPart().getColumn(), + new BaseKeyPart(this.getKeyPart().getTable(), this.getKeyPart().getColumn(), SchemaUtil.concat(this.getKeyPart().getExtractNodes(),extractNodes)), this.getPKPosition(), this.getPKSpan(), @@ -1135,7 +1189,7 @@ public final KeySlot intersect(KeySlot that) { return null; } return new KeySlot( - new BaseKeyPart(this.getKeyPart().getColumn(), + new BaseKeyPart(this.getKeyPart().getTable(), this.getKeyPart().getColumn(), SchemaUtil.concat(this.getKeyPart().getExtractNodes(), that.getKeyPart().getExtractNodes())), this.getPKPosition(), @@ -1181,7 +1235,7 @@ public final KeySlot intersect(KeySlot that) { return null; } return new KeySlot( - new BaseKeyPart(this.getKeyPart().getColumn(), + new BaseKeyPart(this.getKeyPart().getTable(), this.getKeyPart().getColumn(), SchemaUtil.concat(this.getKeyPart().getExtractNodes(), that.getKeyPart().getExtractNodes())), this.getPKPosition(), @@ -1272,10 +1326,12 @@ public KeyRange getKeyRange(CompareOp op, Expression rhs) { return ByteUtil.getKeyRange(key, op, type); } + private final PTable table; private final PColumn column; private final List nodes; - private BaseKeyPart(PColumn column, List nodes) { + private BaseKeyPart(PTable table, PColumn column, List nodes) { + this.table = table; this.column = column; this.nodes = nodes; } @@ -1289,9 +1345,14 @@ public List getExtractNodes() { public PColumn getColumn() { return column; } + + @Override + public PTable getTable() { + return table; + } } - private class RowValueConstructorKeyPart implements KeyPart { + private class RowValueConstructorKeyPart implements KeyPart { private final RowValueConstructorExpression rvc; private final PColumn column; private final List nodes; @@ -1319,7 +1380,13 @@ public List getExtractNodes() { public PColumn getColumn() { return column; } - @Override + + @Override + public PTable getTable() { + return table; + } + + @Override public KeyRange getKeyRange(CompareOp op, Expression rhs) { // With row value constructors, we need to convert the operator for any transformation we do on individual values // to prevent keys from being increased to the next key as would be done for fixed width values. The next key is @@ -1446,7 +1513,6 @@ public T accept(ExpressionVisitor visitor) { byte[] key = ByteUtil.copyKeyBytesIfNecessary(ptr); return ByteUtil.getKeyRange(key, op, PVarbinary.INSTANCE); } - } } } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java index 788a3422f8d..a7e3e44016b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java @@ -85,7 +85,8 @@ import org.apache.phoenix.schema.ValueSchema.Field; import org.apache.phoenix.schema.stats.StatisticsCollector; import org.apache.phoenix.schema.tuple.MultiKeyValueTuple; -import org.apache.phoenix.schema.types.PArrayDataType; +import org.apache.phoenix.schema.types.PBinary; +import org.apache.phoenix.schema.types.PChar; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.IndexUtil; @@ -95,6 +96,7 @@ import org.apache.phoenix.util.ScanUtil; import org.apache.phoenix.util.SchemaUtil; import org.apache.phoenix.util.ServerUtil; +import org.apache.phoenix.util.StringUtil; import org.apache.phoenix.util.TimeKeeper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -296,11 +298,30 @@ protected RegionScanner doPostScannerOpen(final ObserverContext 0 && ptr.get()[ptr.getOffset() + len - 1] == StringUtil.SPACE_UTF8) { + len--; + } + ptr.set(ptr.get(), ptr.getOffset(), len); + } + } else if (field.getDataType() == PBinary.INSTANCE) { + // Remove trailing space characters so that the setValues call below will replace them + // with the correct zero byte character. Note this is somewhat dangerous as these + // could be legit, but I don't know what the alternative is. + int len = ptr.getLength(); + while (len > 0 && ptr.get()[ptr.getOffset() + len - 1] == StringUtil.SPACE_UTF8) { + len--; + } + ptr.set(ptr.get(), ptr.getOffset(), len); } values[i] = ptr.copyBytes(); } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java index 26c076c9607..c1faf6671b9 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java @@ -37,7 +37,6 @@ import org.apache.phoenix.schema.types.PVarchar; import org.apache.phoenix.schema.types.PhoenixArray; import org.apache.phoenix.util.ByteUtil; -import org.apache.phoenix.util.StringUtil; import com.google.common.base.Preconditions; @@ -174,7 +173,7 @@ else if (value instanceof Boolean) { byte[] b = type.toBytes(value, sortOrder); if (type == PVarchar.INSTANCE || type == PChar.INSTANCE) { if (type == PChar.INSTANCE && maxLength != null && b.length < maxLength) { - b = StringUtil.padChar(b, maxLength); + b = type.pad(b, maxLength, sortOrder); } else if (value != null) { maxLength = ((String)value).length(); } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java index 3dcbf7ce103..6a3e2a1f0b9 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java @@ -28,9 +28,10 @@ import org.apache.phoenix.parse.FunctionParseNode.BuiltInFunction; import org.apache.phoenix.query.KeyRange; import org.apache.phoenix.schema.PColumn; -import org.apache.phoenix.schema.types.PDataType; +import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.tuple.Tuple; +import org.apache.phoenix.schema.types.PDataType; @BuiltInFunction(name = InvertFunction.NAME, args = { @Argument() }) public class InvertFunction extends ScalarFunction { @@ -107,6 +108,11 @@ public List getExtractNodes() { public PColumn getColumn() { return childPart.getColumn(); } + + @Override + public PTable getTable() { + return childPart.getTable(); + } }; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java index 3373df72025..111c8b3302d 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java @@ -26,9 +26,10 @@ import org.apache.phoenix.expression.Expression; import org.apache.phoenix.query.KeyRange; import org.apache.phoenix.schema.PColumn; +import org.apache.phoenix.schema.PTable; +import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.util.ByteUtil; -import org.apache.phoenix.util.StringUtil; abstract public class PrefixFunction extends ScalarFunction { public PrefixFunction() { @@ -84,16 +85,22 @@ public KeyRange getKeyRange(CompareOp op, Expression rhs) { return childPart.getKeyRange(op, rhs); } Integer length = getColumn().getMaxLength(); + SortOrder sortOrder = getColumn().getSortOrder(); if (type.isFixedWidth() && length != null) { if (lowerRange != KeyRange.UNBOUND) { - lowerRange = StringUtil.padChar(lowerRange, length); + lowerRange = type.pad(lowerRange, length, sortOrder); } if (upperRange != KeyRange.UNBOUND) { - upperRange = StringUtil.padChar(upperRange, length); + upperRange = type.pad(upperRange, length, sortOrder); } } return KeyRange.getKeyRange(lowerRange, lowerInclusive, upperRange, false); } + + @Override + public PTable getTable() { + return childPart.getTable(); + } }; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java index 911ed190531..aa632e2ed86 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java @@ -29,10 +29,11 @@ import org.apache.phoenix.parse.FunctionParseNode.BuiltInFunction; import org.apache.phoenix.query.KeyRange; import org.apache.phoenix.schema.PColumn; -import org.apache.phoenix.schema.types.PDataType; -import org.apache.phoenix.schema.types.PVarchar; +import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.tuple.Tuple; +import org.apache.phoenix.schema.types.PDataType; +import org.apache.phoenix.schema.types.PVarchar; import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.StringUtil; @@ -123,12 +124,13 @@ public KeyRange getKeyRange(CompareOp op, Expression rhs) { return childPart.getKeyRange(op, rhs); } Integer length = getColumn().getMaxLength(); + SortOrder sortOrder = getColumn().getSortOrder(); if (type.isFixedWidth() && length != null) { if (lowerRange != KeyRange.UNBOUND) { - lowerRange = StringUtil.padChar(lowerRange, length); + lowerRange = type.pad(lowerRange, length, sortOrder); } if (upperRange != KeyRange.UNBOUND) { - upperRange = StringUtil.padChar(upperRange, length); + upperRange = type.pad(upperRange, length, sortOrder); } } return KeyRange.getKeyRange(lowerRange, lowerInclusive, upperRange, false); @@ -143,6 +145,11 @@ public List getExtractNodes() { public PColumn getColumn() { return childPart.getColumn(); } + + @Override + public PTable getTable() { + return childPart.getTable(); + } }; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java index d8fa1dc2cc2..e410e344d45 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java @@ -33,13 +33,14 @@ import org.apache.phoenix.expression.Expression; import org.apache.phoenix.expression.LiteralExpression; import org.apache.phoenix.query.KeyRange; -import org.apache.phoenix.schema.types.PInteger; import org.apache.phoenix.schema.PColumn; +import org.apache.phoenix.schema.PTable; +import org.apache.phoenix.schema.SortOrder; +import org.apache.phoenix.schema.tuple.Tuple; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PDataType.PDataCodec; +import org.apache.phoenix.schema.types.PInteger; import org.apache.phoenix.schema.types.PVarchar; -import org.apache.phoenix.schema.SortOrder; -import org.apache.phoenix.schema.tuple.Tuple; import org.apache.phoenix.util.ByteUtil; import com.google.common.collect.Lists; @@ -255,6 +256,11 @@ public KeyRange getKeyRange(CompareOp op, Expression rhs) { return childPart.getKeyRange(op, rhs); } } + + @Override + public PTable getTable() { + return childPart.getTable(); + } }; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java index e81650f0355..7f440827ca9 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDecimalExpression.java @@ -23,26 +23,27 @@ import java.math.BigDecimal; import java.math.RoundingMode; import java.sql.SQLException; +import java.util.Collections; import java.util.List; +import org.apache.hadoop.hbase.filter.CompareFilter; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.io.WritableUtils; +import org.apache.phoenix.compile.KeyPart; import org.apache.phoenix.expression.Determinism; import org.apache.phoenix.expression.Expression; import org.apache.phoenix.expression.LiteralExpression; -import org.apache.phoenix.schema.types.PDecimal; +import org.apache.phoenix.query.KeyRange; import org.apache.phoenix.schema.IllegalDataException; -import org.apache.phoenix.schema.types.PInteger; +import org.apache.phoenix.schema.PColumn; +import org.apache.phoenix.schema.PTable; +import org.apache.phoenix.schema.tuple.Tuple; import org.apache.phoenix.schema.types.PDataType; +import org.apache.phoenix.schema.types.PDecimal; +import org.apache.phoenix.schema.types.PInteger; import org.apache.phoenix.schema.types.PLong; -import org.apache.phoenix.schema.tuple.Tuple; import com.google.common.collect.Lists; -import java.util.Collections; -import org.apache.hadoop.hbase.filter.CompareFilter; -import org.apache.phoenix.compile.KeyPart; -import org.apache.phoenix.query.KeyRange; -import org.apache.phoenix.schema.PColumn; /** * @@ -274,6 +275,11 @@ private BigDecimal roundAndPreserveOperator(BigDecimal decimal, CompareFilter.Co // otherwise, rounding has not affected the operator, so return normally return rounded; } + + @Override + public PTable getTable() { + return childPart.getTable(); + } }; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java index a17e28aba4f..dede98d0197 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java @@ -1983,9 +1983,13 @@ public Void call() throws Exception { props.remove(PhoenixRuntime.TENANT_ID_ATTRIB); PhoenixConnection conn = new PhoenixConnection(ConnectionQueryServicesImpl.this, metaConnection.getURL(), props, metaConnection.getMetaDataCache()); try { - Set tablesNeedingUpgrade = UpgradeUtil.getPhysicalTablesWithDescVarLengthRowKey(conn); + List tablesNeedingUpgrade = UpgradeUtil.getPhysicalTablesWithDescRowKey(conn); if (!tablesNeedingUpgrade.isEmpty()) { - logger.warn("The following tables require upgrade due to a bug causing the row key to be incorrect for descending columns (PHOENIX-2067):\n" + Joiner.on(' ').join(tablesNeedingUpgrade)); + logger.warn("The following tables require upgrade due to a bug causing the row key to be incorrect for descending columns and ascending BINARY columns (PHOENIX-2067 and PHOENIX-2120):\n" + Joiner.on(' ').join(tablesNeedingUpgrade) + "\nTo upgrade issue the \"bin/psql.py -u\" command."); + } + List unsupportedTables = UpgradeUtil.getPhysicalTablesWithDescVarbinaryRowKey(conn); + if (!unsupportedTables.isEmpty()) { + logger.warn("The following tables use an unsupported VARBINARY DESC construct and need to be changed:\n" + Joiner.on(' ').join(unsupportedTables)); } } catch (Exception ex) { logger.error("Unable to determine tables requiring upgrade due to PHOENIX-2067", ex); diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java index 1756c2fe365..521fb427842 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java @@ -54,6 +54,8 @@ import org.apache.phoenix.schema.stats.GuidePostsInfo; import org.apache.phoenix.schema.stats.PTableStats; import org.apache.phoenix.schema.stats.PTableStatsImpl; +import org.apache.phoenix.schema.types.PBinary; +import org.apache.phoenix.schema.types.PChar; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PVarchar; import org.apache.phoenix.util.ByteUtil; @@ -125,8 +127,8 @@ public class PTableImpl implements PTable { private IndexType indexType; private PTableStats tableStats = PTableStats.EMPTY_STATS; private int baseColumnCount; - private boolean hasDescVarLengthColumns; - private boolean rowKeyOrderOptimizable; + private boolean rowKeyOrderOptimizable; // TODO: remove when required that tables have been upgrade for PHOENIX-2067 + private boolean hasColumnsRequiringUpgrade; // TODO: remove when required that tables have been upgrade for PHOENIX-2067 public PTableImpl() { this.indexes = Collections.emptyList(); @@ -405,7 +407,8 @@ private void init(PName tenantId, PName schemaName, PName tableName, PTableType for (PColumn column : allColumns) { PName familyName = column.getFamilyName(); if (familyName == null) { - hasDescVarLengthColumns |= (column.getSortOrder() == SortOrder.DESC && !column.getDataType().isFixedWidth()); + hasColumnsRequiringUpgrade |= (column.getSortOrder() == SortOrder.DESC && (!column.getDataType().isFixedWidth() || column.getDataType() == PChar.INSTANCE || column.getDataType() == PBinary.INSTANCE)) + || (column.getSortOrder() == SortOrder.ASC && column.getDataType() == PBinary.INSTANCE && column.getMaxLength() != null && column.getMaxLength() > 1); pkColumns.add(column); } if (familyName == null) { @@ -547,8 +550,16 @@ public int newKey(ImmutableBytesWritable key, byte[][] values) { throw new ConstraintViolationException(name.getString() + "." + column.getName().getString() + " may not be null"); } Integer maxLength = column.getMaxLength(); - if (maxLength != null && type.isFixedWidth() && byteValue.length <= maxLength) { - byteValue = StringUtil.padChar(byteValue, maxLength); + if (maxLength != null && type.isFixedWidth() && byteValue.length < maxLength) { + if (rowKeyOrderOptimizable()) { + key.set(byteValue); + type.pad(key, maxLength, sortOrder); + byteValue = ByteUtil.copyKeyBytesIfNecessary(key); + } else { + // TODO: remove this incorrect code and move StringUtil.padChar() to TestUtil + // once we require tables to have been upgraded + byteValue = StringUtil.padChar(byteValue, maxLength); + } } else if (maxLength != null && byteValue.length > maxLength) { throw new DataExceedsCapacityException(name.getString() + "." + column.getName().getString() + " may not exceed " + maxLength + " bytes (" + SchemaUtil.toString(type, byteValue) + ")"); } @@ -733,7 +744,7 @@ public void setValue(PColumn column, byte[] byteValue) { HConstants.EMPTY_BYTE_ARRAY : byteValue); Integer maxLength = column.getMaxLength(); if (!isNull && type.isFixedWidth() && maxLength != null) { - if (ptr.getLength() <= maxLength) { + if (ptr.getLength() < maxLength) { type.pad(ptr, maxLength, column.getSortOrder()); } else if (ptr.getLength() > maxLength) { throw new DataExceedsCapacityException(name.getString() + "." + column.getName().getString() + " may not exceed " + maxLength + " bytes (" + type.toObject(byteValue) + ")"); @@ -1131,6 +1142,6 @@ public int getBaseColumnCount() { @Override public boolean rowKeyOrderOptimizable() { - return rowKeyOrderOptimizable || !hasDescVarLengthColumns; + return rowKeyOrderOptimizable || !hasColumnsRequiringUpgrade; } } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBinary.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBinary.java index b3975542343..8e0c4b56f6f 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBinary.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBinary.java @@ -19,11 +19,13 @@ import java.sql.Types; import java.text.Format; +import java.util.Arrays; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Base64; import org.apache.hadoop.hbase.util.Bytes; import org.apache.phoenix.exception.DataExceedsCapacityException; +import org.apache.phoenix.query.QueryConstants; import org.apache.phoenix.schema.SortOrder; public class PBinary extends PDataType { @@ -34,6 +36,19 @@ private PBinary() { super("BINARY", Types.BINARY, byte[].class, null, 23); } + @Override + public byte[] pad(byte[] b, Integer maxLength, SortOrder sortOrder) { + if (b == null || b.length >= maxLength) { + return b; + } + byte[] newBytes = new byte[maxLength]; + System.arraycopy(b, 0, newBytes, 0, b.length); + if (sortOrder == SortOrder.DESC) { + Arrays.fill(newBytes, b.length, maxLength, QueryConstants.DESC_SEPARATOR_BYTE); + } + return newBytes; + } + @Override public void pad(ImmutableBytesWritable ptr, Integer maxLength, SortOrder sortOrder) { if (ptr.getLength() >= maxLength) { @@ -41,23 +56,24 @@ public void pad(ImmutableBytesWritable ptr, Integer maxLength, SortOrder sortOrd } byte[] newBytes = new byte[maxLength]; System.arraycopy(ptr.get(), ptr.getOffset(), newBytes, 0, ptr.getLength()); + if (sortOrder == SortOrder.DESC) { + Arrays.fill(newBytes, ptr.getLength(), maxLength, QueryConstants.DESC_SEPARATOR_BYTE); + } ptr.set(newBytes); } @Override public Object pad(Object object, Integer maxLength) { byte[] b = (byte[]) object; - if (b == null) { - return new byte[maxLength]; - } - if (b.length == maxLength) { + int length = (b == null ? 0 : b.length); + if (length == maxLength) { return object; } - if (b.length > maxLength) { + if (length > maxLength) { throw new DataExceedsCapacityException(this, maxLength, null); } byte[] newBytes = new byte[maxLength]; - System.arraycopy(b, 0, newBytes, 0, b.length); + System.arraycopy(b, 0, newBytes, 0, length); return newBytes; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PChar.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PChar.java index c7cc1c18d4d..6b261975938 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PChar.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PChar.java @@ -52,6 +52,17 @@ public void pad(ImmutableBytesWritable ptr, Integer maxLength, SortOrder sortOrd ptr.set(newBytes); } + @Override + public byte[] pad(byte[] b, Integer maxLength, SortOrder sortOrder) { + if (b == null || b.length >= maxLength) { + return b; + } + byte[] newBytes = new byte[maxLength]; + System.arraycopy(b, 0, newBytes, 0, b.length); + Arrays.fill(newBytes, b.length, maxLength, sortOrder == SortOrder.ASC ? StringUtil.SPACE_UTF8 : StringUtil.INVERTED_SPACE_UTF8); + return newBytes; + } + @Override public Object pad(Object object, Integer maxLength) { String s = (String) object; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java index 2c91dc51cc6..43bab0e7a9b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java @@ -1147,6 +1147,7 @@ public Object pad(Object object, Integer maxLength) { } public void pad(ImmutableBytesWritable ptr, Integer maxLength, SortOrder sortOrder) {} + public byte[] pad(byte[] b, Integer maxLength, SortOrder sortOrder) { return b; } public static PDataType arrayBaseType(PDataType arrayType) { Preconditions.checkArgument(arrayType.isArrayType(), "Not a phoenix array type"); diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java index 1a2019d7999..c99d47ea354 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java @@ -193,17 +193,28 @@ public static void main(String [] args) { .unwrap(PhoenixConnection.class); if (execCmd.isUpgrade()) { + if (conn.getClientInfo(PhoenixRuntime.CURRENT_SCN_ATTRIB) != null) { + throw new SQLException("May not specify the CURRENT_SCN property when upgrading"); + } + if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { + throw new SQLException("May not specify the TENANT_ID_ATTRIB property when upgrading"); + } if (execCmd.getInputFiles().isEmpty()) { - Set tablesNeedingUpgrade = UpgradeUtil.getPhysicalTablesWithDescVarLengthRowKey(conn); + List tablesNeedingUpgrade = UpgradeUtil.getPhysicalTablesWithDescRowKey(conn); if (tablesNeedingUpgrade.isEmpty()) { - String msg = "No tables are required to be upgraded due to incorrect row key order for descending, variable length columsn (PHOENIX-2067)"; + String msg = "No tables are required to be upgraded due to incorrect row key order (PHOENIX-2067 and PHOENIX-2120)"; System.out.println(msg); } else { - String msg = "The following tables require upgrade due to a bug causing the row key to be incorrect for descending columns (PHOENIX-2067):\n" + Joiner.on(' ').join(tablesNeedingUpgrade); + String msg = "The following tables require upgrade due to a bug causing the row key to be incorrectly ordered (PHOENIX-2067 and PHOENIX-2120):\n" + Joiner.on(' ').join(tablesNeedingUpgrade); + System.out.println("WARNING: " + msg); + } + List unsupportedTables = UpgradeUtil.getPhysicalTablesWithDescVarbinaryRowKey(conn); + if (!unsupportedTables.isEmpty()) { + String msg = "The following tables use an unsupported VARBINARY DESC construct and need to be changed:\n" + Joiner.on(' ').join(unsupportedTables); System.out.println("WARNING: " + msg); } } else { - UpgradeUtil.upgradeDescVarLengthRowKeys(conn, execCmd.getInputFiles()); + UpgradeUtil.upgradeDescVarLengthRowKeys(conn, execCmd.getInputFiles(), execCmd.isBypassUpgrade()); } } else { for (String inputFile : execCmd.getInputFiles()) { @@ -476,6 +487,7 @@ static class ExecutionCommand { private boolean strict; private List inputFiles; private boolean isUpgrade; + private boolean isBypassUpgrade; /** * Factory method to build up an {@code ExecutionCommand} based on supplied parameters. @@ -502,9 +514,18 @@ public static ExecutionCommand parseArgs(String[] args) { "Define the array element separator, defaults to ':'"); Option upgradeOption = new Option("u", "upgrade", false, "Upgrades tables specified as arguments " + "by rewriting them with the correct row key for descending columns. If no arguments are " + - "specified, then tables that need to be upgraded will be displayed. " + + "specified, then tables that need to be upgraded will be displayed without being upgraded. " + + "Use the -b option to bypass the rewrite if you know that your data does not need to be upgrade. " + + "This would only be the case if you have not relied on auto padding for BINARY and CHAR data, " + + "but instead have always provided data up to the full max length of the column. See PHOENIX-2067 " + + "and PHOENIX-2120 for more information. " + "Note that " + QueryServices.THREAD_TIMEOUT_MS_ATTRIB + " and hbase.regionserver.lease.period " + "parameters must be set very high to prevent timeouts when upgrading."); + Option bypassUpgradeOption = new Option("b", "bypass-upgrade", false, + "Used in conjunction with the -u option to bypass the rewrite during upgrade if you know that your data does not need to be upgrade. " + + "This would only be the case if you have not relied on auto padding for BINARY and CHAR data, " + + "but instead have always provided data up to the full max length of the column. See PHOENIX-2067 " + + "and PHOENIX-2120 for more information. "); Options options = new Options(); options.addOption(tableOption); options.addOption(headerOption); @@ -514,6 +535,7 @@ public static ExecutionCommand parseArgs(String[] args) { options.addOption(escapeCharacterOption); options.addOption(arrayValueSeparatorOption); options.addOption(upgradeOption); + options.addOption(bypassUpgradeOption); CommandLineParser parser = new PosixParser(); CommandLine cmdLine = null; @@ -558,6 +580,13 @@ public static ExecutionCommand parseArgs(String[] args) { execCmd.isUpgrade = true; } + if (cmdLine.hasOption(bypassUpgradeOption.getOpt())) { + if (!execCmd.isUpgrade()) { + usageError("The bypass-upgrade option may only be used in conjunction with the -u option", options); + } + execCmd.isBypassUpgrade = true; + } + List argList = Lists.newArrayList(cmdLine.getArgList()); if (argList.isEmpty()) { @@ -649,6 +678,10 @@ public boolean isStrict() { public boolean isUpgrade() { return isUpgrade; } + + public boolean isBypassUpgrade() { + return isBypassUpgrade; + } } /** diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/StringUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/StringUtil.java index 89ae43b7d6f..4d3a36f9114 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/StringUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/StringUtil.java @@ -268,13 +268,6 @@ public static int getUnpaddedCharLength(byte[] b, int offset, int length, SortOr return getFirstNonBlankCharIdxFromEnd(b, offset, length, sortOrder) - offset + 1; } - public static byte[] padChar(byte[] value, int offset, int length, int paddedLength) { - byte[] key = new byte[paddedLength]; - System.arraycopy(value,offset, key, 0, length); - Arrays.fill(key, length, paddedLength, SPACE_UTF8); - return key; - } - public static byte[] padChar(byte[] value, Integer byteSize) { byte[] newValue = Arrays.copyOf(value, byteSize); if (newValue.length > value.length) { diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java index b4fcef844e3..2ab4f8b5194 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java @@ -79,11 +79,14 @@ import org.apache.phoenix.schema.PTableType; import org.apache.phoenix.schema.SaltingUtil; import org.apache.phoenix.schema.SortOrder; +import org.apache.phoenix.schema.types.PBinary; import org.apache.phoenix.schema.types.PBoolean; +import org.apache.phoenix.schema.types.PChar; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PDecimal; import org.apache.phoenix.schema.types.PInteger; import org.apache.phoenix.schema.types.PLong; +import org.apache.phoenix.schema.types.PVarbinary; import org.apache.phoenix.schema.types.PVarchar; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -878,10 +881,17 @@ private static List addPhysicalTables(PhoenixConnection conn, ResultSet return otherTables; } - // Return all types that are not fixed width that may need upgrading due to PHOENIX-2067 + // Return all types that are descending and either: + // 1) variable length, which includes all array types (PHOENIX-2067) + // 2) fixed length with padding (PHOENIX-2120) // We exclude VARBINARY as we no longer support DESC for it. private static String getAffectedDataTypes() { - StringBuilder buf = new StringBuilder("(" + PVarchar.INSTANCE.getSqlType() + "," + PDecimal.INSTANCE.getSqlType() + ","); + StringBuilder buf = new StringBuilder("(" + + PVarchar.INSTANCE.getSqlType() + "," + + + PChar.INSTANCE.getSqlType() + "," + + + PBinary.INSTANCE.getSqlType() + "," + + + PDecimal.INSTANCE.getSqlType() + "," + ); for (PDataType type : PDataType.values()) { if (type.isArrayType()) { buf.append(type.getSqlType()); @@ -891,20 +901,46 @@ private static String getAffectedDataTypes() { buf.setCharAt(buf.length()-1, ')'); return buf.toString(); } + + /** - * Identify the tables that need to be upgraded due to PHOENIX-2067 + * Identify the tables that are DESC VARBINARY as this is no longer supported */ - public static Set getPhysicalTablesWithDescVarLengthRowKey(PhoenixConnection conn) throws SQLException { - // First query finds column rows of tables that need to be upgraded. - // We cannot tell if the column is from a table, view, or index however. - ResultSet rs = conn.createStatement().executeQuery( - "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" + + public static List getPhysicalTablesWithDescVarbinaryRowKey(PhoenixConnection conn) throws SQLException { + String query = "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" + "FROM SYSTEM.CATALOG cat1\n" + "WHERE COLUMN_NAME IS NOT NULL\n" + "AND COLUMN_FAMILY IS NULL\n" + "AND SORT_ORDER = " + SortOrder.DESC.getSystemValue() + "\n" + - "AND DATA_TYPE IN " + getAffectedDataTypes() + "\n" + - "GROUP BY TENANT_ID,TABLE_SCHEM,TABLE_NAME"); + "AND DATA_TYPE = " + PVarbinary.INSTANCE.getSqlType() + "\n" + + "GROUP BY TENANT_ID,TABLE_SCHEM,TABLE_NAME"; + return getPhysicalTablesWithDescRowKey(query, conn); + } + + /** + * Identify the tables that need to be upgraded due to PHOENIX-2067 and PHOENIX-2120 + */ + public static List getPhysicalTablesWithDescRowKey(PhoenixConnection conn) throws SQLException { + String query = "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" + + "FROM SYSTEM.CATALOG cat1\n" + + "WHERE COLUMN_NAME IS NOT NULL\n" + + "AND COLUMN_FAMILY IS NULL\n" + + "AND ( ( SORT_ORDER = " + SortOrder.DESC.getSystemValue() + "\n" + + " AND DATA_TYPE IN " + getAffectedDataTypes() + ")\n" + + " OR ( SORT_ORDER = " + SortOrder.ASC.getSystemValue() + "\n" + + " AND DATA_TYPE = " + PBinary.INSTANCE.getSqlType() + "\n" + + " AND COLUMN_SIZE > 1 ) )\n" + + "GROUP BY TENANT_ID,TABLE_SCHEM,TABLE_NAME"; + return getPhysicalTablesWithDescRowKey(query, conn); + } + + /** + * Identify the tables that need to be upgraded due to PHOENIX-2067 + */ + private static List getPhysicalTablesWithDescRowKey(String query, PhoenixConnection conn) throws SQLException { + // First query finds column rows of tables that need to be upgraded. + // We cannot tell if the column is from a table, view, or index however. + ResultSet rs = conn.createStatement().executeQuery(query); Set physicalTables = Sets.newHashSetWithExpectedSize(1024); List remainingTableNames = addPhysicalTables(conn, rs, PTableType.INDEX, physicalTables); if (!remainingTableNames.isEmpty()) { @@ -931,71 +967,129 @@ public static Set getPhysicalTablesWithDescVarLengthRowKey(PhoenixConnec addPhysicalTables(conn, rs, PTableType.TABLE, physicalTables); } } - return physicalTables; + List sortedPhysicalTables = new ArrayList(physicalTables); + Collections.sort(sortedPhysicalTables); + return sortedPhysicalTables; } - private static void upgradeDescVarLengthRowKeys(PhoenixConnection upgradeConn, PhoenixConnection globalConn, String schemaName, String tableName, boolean isTable) throws SQLException { - String escapedTableName = SchemaUtil.getEscapedTableName(schemaName, tableName); - String tenantInfo = ""; - PName tenantId = PName.EMPTY_NAME; - if (upgradeConn.getTenantId() != null) { - tenantId = upgradeConn.getTenantId(); - tenantInfo = " for tenant " + tenantId.getString(); + private static void upgradeDescVarLengthRowKeys(PhoenixConnection upgradeConn, PhoenixConnection globalConn, String schemaName, String tableName, boolean isTable, boolean bypassUpgrade) throws SQLException { + String physicalName = SchemaUtil.getTableName(schemaName, tableName); + long currentTime = System.currentTimeMillis(); + String snapshotName = physicalName + "_" + currentTime; + HBaseAdmin admin = null; + if (isTable && !bypassUpgrade) { + admin = globalConn.getQueryServices().getAdmin(); } - String msg = "Starting upgrade of " + escapedTableName + tenantInfo + "..."; - System.out.println(msg); - logger.info(msg); - ResultSet rs = upgradeConn.createStatement().executeQuery("SELECT /*+ NO_INDEX */ count(*) FROM " + escapedTableName); - rs.next(); // Run query - List tableNames = Lists.newArrayListWithExpectedSize(1024); - tableNames.add(tenantId == PName.EMPTY_NAME ? null : tenantId.getString()); - tableNames.add(schemaName); - tableNames.add(tableName); - // Find views to mark as upgraded - if (isTable) { - String physicalName = SchemaUtil.getTableName(schemaName, tableName); - String query = - "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" + - "FROM SYSTEM.CATALOG\n" + - "WHERE COLUMN_NAME IS NULL\n" + - "AND COLUMN_FAMILY = '" + physicalName + "'" + - "AND LINK_TYPE = " + LinkType.PHYSICAL_TABLE.getSerializedValue(); - rs = globalConn.createStatement().executeQuery(query); - while (rs.next()) { - tableNames.add(rs.getString(1)); - tableNames.add(rs.getString(2)); - tableNames.add(rs.getString(3)); + boolean restoreSnapshot = false; + boolean success = false; + try { + if (isTable && !bypassUpgrade) { + String msg = "Taking snapshot of physical table " + physicalName + " prior to upgrade..."; + System.out.println(msg); + logger.info(msg); + admin.disableTable(physicalName); + admin.snapshot(snapshotName, physicalName); + admin.enableTable(physicalName); + restoreSnapshot = true; + } + String escapedTableName = SchemaUtil.getEscapedTableName(schemaName, tableName); + String tenantInfo = ""; + PName tenantId = PName.EMPTY_NAME; + if (upgradeConn.getTenantId() != null) { + tenantId = upgradeConn.getTenantId(); + tenantInfo = " for tenant " + tenantId.getString(); + } + String msg = "Starting upgrade of " + escapedTableName + tenantInfo + "..."; + System.out.println(msg); + logger.info(msg); + ResultSet rs; + if (!bypassUpgrade) { + rs = upgradeConn.createStatement().executeQuery("SELECT /*+ NO_INDEX */ count(*) FROM " + escapedTableName); + rs.next(); // Run query + } + List tableNames = Lists.newArrayListWithExpectedSize(1024); + tableNames.add(tenantId == PName.EMPTY_NAME ? null : tenantId.getString()); + tableNames.add(schemaName); + tableNames.add(tableName); + // Find views to mark as upgraded + if (isTable) { + String query = + "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" + + "FROM SYSTEM.CATALOG\n" + + "WHERE COLUMN_NAME IS NULL\n" + + "AND COLUMN_FAMILY = '" + physicalName + "'" + + "AND LINK_TYPE = " + LinkType.PHYSICAL_TABLE.getSerializedValue(); + rs = globalConn.createStatement().executeQuery(query); + while (rs.next()) { + tableNames.add(rs.getString(1)); + tableNames.add(rs.getString(2)); + tableNames.add(rs.getString(3)); + } + } + // Mark the table and views as upgraded now + for (int i = 0; i < tableNames.size(); i += 3) { + String theTenantId = tableNames.get(i); + String theSchemaName = tableNames.get(i+1); + String theTableName = tableNames.get(i+2); + globalConn.createStatement().execute("UPSERT INTO " + PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME + + " (" + PhoenixDatabaseMetaData.TENANT_ID + "," + + PhoenixDatabaseMetaData.TABLE_SCHEM + "," + + PhoenixDatabaseMetaData.TABLE_NAME + "," + + MetaDataEndpointImpl.ROW_KEY_ORDER_OPTIMIZABLE + " BOOLEAN" + + ") VALUES (" + + "'" + (theTenantId == null ? StringUtil.EMPTY_STRING : theTenantId) + "'," + + "'" + (theSchemaName == null ? StringUtil.EMPTY_STRING : theSchemaName) + "'," + + "'" + theTableName + "'," + + "TRUE)"); + } + globalConn.commit(); + for (int i = 0; i < tableNames.size(); i += 3) { + String theTenantId = tableNames.get(i); + String theSchemaName = tableNames.get(i+1); + String theTableName = tableNames.get(i+2); + globalConn.getQueryServices().clearTableFromCache( + theTenantId == null ? ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(theTenantId), + theSchemaName == null ? ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(schemaName), + Bytes.toBytes(theTableName), HConstants.LATEST_TIMESTAMP); + } + success = true; + msg = "Completed upgrade of " + escapedTableName + tenantInfo; + System.out.println(msg); + logger.info(msg); + } catch (Exception e) { + logger.error("Exception during upgrade of " + physicalName + ":", e); + } finally { + boolean restored = false; + try { + if (!success && restoreSnapshot) { + admin.disableTable(physicalName); + admin.restoreSnapshot(snapshotName, false); + admin.enableTable(physicalName); + String msg = "Restored snapshot of " + physicalName + " due to failure of upgrade"; + System.out.println(msg); + logger.info(msg); + } + restored = true; + } catch (Exception e) { + logger.warn("Unable to restoring snapshot " + snapshotName + " after failed upgrade", e); + } finally { + try { + if (restoreSnapshot && restored) { + admin.deleteSnapshot(snapshotName); + } + } catch (Exception e) { + logger.warn("Unable to delete snapshot " + snapshotName + " after upgrade:", e); + } finally { + try { + if (admin != null) { + admin.close(); + } + } catch (IOException e) { + logger.warn("Unable to close admin after upgrade:", e); + } + } } } - // Mark the table and views as upgraded now - for (int i = 0; i < tableNames.size(); i += 3) { - String theTenantId = tableNames.get(i); - String theSchemaName = tableNames.get(i+1); - String theTableName = tableNames.get(i+2); - globalConn.createStatement().execute("UPSERT INTO " + PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME + - " (" + PhoenixDatabaseMetaData.TENANT_ID + "," + - PhoenixDatabaseMetaData.TABLE_SCHEM + "," + - PhoenixDatabaseMetaData.TABLE_NAME + "," + - MetaDataEndpointImpl.ROW_KEY_ORDER_OPTIMIZABLE + " BOOLEAN" - + ") VALUES (" + - "'" + (theTenantId == null ? StringUtil.EMPTY_STRING : theTenantId) + "'," + - "'" + (theSchemaName == null ? StringUtil.EMPTY_STRING : theSchemaName) + "'," + - "'" + theTableName + "'," + - "TRUE)"); - } - globalConn.commit(); - for (int i = 0; i < tableNames.size(); i += 3) { - String theTenantId = tableNames.get(i); - String theSchemaName = tableNames.get(i+1); - String theTableName = tableNames.get(i+2); - globalConn.getQueryServices().clearTableFromCache( - theTenantId == null ? ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(theTenantId), - theSchemaName == null ? ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(schemaName), - Bytes.toBytes(theTableName), HConstants.LATEST_TIMESTAMP); - } - msg = "Completed upgrade of " + escapedTableName + tenantInfo; - System.out.println(msg); - logger.info(msg); } private static boolean isInvalidTableToUpgrade(PTable table) throws SQLException { @@ -1007,16 +1101,10 @@ private static boolean isInvalidTableToUpgrade(PTable table) throws SQLException * Upgrade tables and their indexes due to a bug causing descending row keys to have a row key that * prevents them from being sorted correctly (PHOENIX-2067). */ - public static void upgradeDescVarLengthRowKeys(PhoenixConnection conn, List tablesToUpgrade) throws SQLException { + public static void upgradeDescVarLengthRowKeys(PhoenixConnection conn, List tablesToUpgrade, boolean bypassUpgrade) throws SQLException { if (tablesToUpgrade.isEmpty()) { return; } - if (conn.getClientInfo(PhoenixRuntime.CURRENT_SCN_ATTRIB) != null) { - throw new SQLException("May not specify the CURRENT_SCN property when upgrading"); - } - if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { - throw new SQLException("May not specify the TENANT_ID_ATTRIB property when upgrading"); - } List tablesNeedingUpgrading = Lists.newArrayListWithExpectedSize(tablesToUpgrade.size()); List invalidTables = Lists.newArrayListWithExpectedSize(tablesToUpgrade.size()); for (String fullTableName : tablesToUpgrade) { @@ -1042,23 +1130,23 @@ public static void upgradeDescVarLengthRowKeys(PhoenixConnection conn, List