Repository: phoenix Updated Branches: refs/heads/3.0 ca776fe89 -> 48a4bd670
PHOENIX-1288 Fix selecting multiple array indexes Don't attempt to remove the same KeyValue multiple times when transforming multiple array index references into a single filtered KeyValue. Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/48a4bd67 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/48a4bd67 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/48a4bd67 Branch: refs/heads/3.0 Commit: 48a4bd6709e49d8e629e766b2469bb96b0699c37 Parents: ca776fe Author: Gabriel Reid <gabri...@ngdata.com> Authored: Wed Sep 24 21:04:46 2014 +0200 Committer: Gabriel Reid <gabri...@ngdata.com> Committed: Wed Sep 24 21:04:46 2014 +0200 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/ArrayIT.java | 99 ++++++++++---------- .../phoenix/coprocessor/ScanRegionObserver.java | 58 ++++++------ 2 files changed, 79 insertions(+), 78 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a4bd67/phoenix-core/src/it/java/org/apache/phoenix/end2end/ArrayIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ArrayIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ArrayIT.java index a7fe827..21cfcdd 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ArrayIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ArrayIT.java @@ -292,7 +292,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testUpsertValuesWithArray() throws Exception { long ts = nextTimestamp(); @@ -373,7 +373,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { } } } - + @Test public void testArraySelectWithORCondition() throws Exception { long ts = nextTimestamp(); @@ -552,7 +552,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { } } } - + @Test public void testSelectWithArrayWithColumnRef() throws Exception { long ts = nextTimestamp(); @@ -583,7 +583,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testSelectWithArrayWithColumnRefWithVarLengthArray() throws Exception { long ts = nextTimestamp(); @@ -648,7 +648,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testUpsertSelectWithColumnRef() throws Exception { long ts = nextTimestamp(); @@ -790,7 +790,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn = DriverManager.getConnection(getUrl(), props); conn.createStatement().execute("CREATE TABLE t ( k VARCHAR PRIMARY KEY, a bigint ARRAY[])"); conn.close(); - + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 30)); conn = DriverManager.getConnection(getUrl(), props); stmt = conn.prepareStatement("UPSERT INTO t VALUES(?,?)"); @@ -883,21 +883,21 @@ public class ArrayIT extends BaseClientManagedTimeIT { Connection conn; PreparedStatement stmt; ResultSet rs; - + long ts = nextTimestamp(); Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 10)); conn = DriverManager.getConnection(getUrl(), props); conn.createStatement().execute("CREATE TABLE t ( k VARCHAR PRIMARY KEY, a CHAR(5) ARRAY)"); conn.close(); - + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 20)); conn = DriverManager.getConnection(getUrl(), props); rs = conn.getMetaData().getColumns(null, null, "T", "A"); assertTrue(rs.next()); assertEquals(5, rs.getInt("COLUMN_SIZE")); conn.close(); - + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 30)); conn = DriverManager.getConnection(getUrl(), props); stmt = conn.prepareStatement("UPSERT INTO t VALUES(?,?)"); @@ -908,7 +908,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { stmt.execute(); conn.commit(); conn.close(); - + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 40)); conn = DriverManager.getConnection(getUrl(), props); rs = conn.createStatement().executeQuery("SELECT k, a[2] FROM t"); @@ -917,7 +917,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { assertEquals("2",rs.getString(2)); conn.close(); } - + @Test public void testSelectArrayUsingUpsertLikeSyntax() throws Exception { long ts = nextTimestamp(); @@ -947,7 +947,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testArrayIndexUsedInWhereClause() throws Exception { long ts = nextTimestamp(); @@ -1008,7 +1008,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testVariableLengthArrayWithNullValue() throws Exception { long ts = nextTimestamp(); @@ -1033,7 +1033,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testSelectSpecificIndexOfAVariableArrayAlongWithAnotherColumn1() throws Exception { long ts = nextTimestamp(); @@ -1061,7 +1061,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testSelectSpecificIndexOfAVariableArrayAlongWithAnotherColumn2() throws Exception { long ts = nextTimestamp(); @@ -1089,7 +1089,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testSelectMultipleArrayColumns() throws Exception { long ts = nextTimestamp(); @@ -1117,9 +1117,9 @@ public class ArrayIT extends BaseClientManagedTimeIT { assertFalse(rs.next()); } finally { conn.close(); - } + } } - + @Test public void testSelectSameArrayColumnMultipleTimesWithDifferentIndices() throws Exception { long ts = nextTimestamp(); @@ -1127,7 +1127,9 @@ public class ArrayIT extends BaseClientManagedTimeIT { createTableWithArray(getUrl(), getDefaultSplits(tenantId), null, ts - 2); initTablesWithArrays(tenantId, null, ts, false, getUrl()); - String query = "SELECT a_string_array[1], a_string_array[3] FROM table_with_array"; + String query = "SELECT a_string_array[1], a_string_array[2], " + + "a_string_array[3], a_double_array[1], a_double_array[2], a_double_array[3] " + + "FROM table_with_array"; Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 2)); // Execute at timestamp 2 @@ -1136,19 +1138,18 @@ public class ArrayIT extends BaseClientManagedTimeIT { PreparedStatement statement = conn.prepareStatement(query); ResultSet rs = statement.executeQuery(); assertTrue(rs.next()); - String[] strArr = new String[2]; - strArr[0] = "ABC"; - strArr[1] = "XYZWER"; - String result = rs.getString(1); - assertEquals(strArr[0], result); - result = rs.getString(2); - assertEquals(strArr[1], result); + assertEquals("ABC", rs.getString(1)); + assertEquals("CEDF", rs.getString(2)); + assertEquals("XYZWER", rs.getString(3)); + assertEquals(25.343, rs.getDouble(4), 0.0); + assertEquals(36.763, rs.getDouble(5), 0.0); + assertEquals(37.56, rs.getDouble(6), 0.0); assertFalse(rs.next()); } finally { conn.close(); - } + } } - + @Test public void testSelectSameArrayColumnMultipleTimesWithSameIndices() throws Exception { long ts = nextTimestamp(); @@ -1174,7 +1175,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { assertFalse(rs.next()); } finally { conn.close(); - } + } } @Test @@ -1246,7 +1247,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { try { PreparedStatement statement = conn.prepareStatement(query); ResultSet rs = statement.executeQuery(); - assertTrue(rs.next()); + assertTrue(rs.next()); int result = rs.getInt(1); assertEquals(result, 4); assertFalse(rs.next()); @@ -1254,7 +1255,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testArrayLengthFunctionForFixedLength() throws Exception { @@ -1271,7 +1272,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { try { PreparedStatement statement = conn.prepareStatement(query); ResultSet rs = statement.executeQuery(); - assertTrue(rs.next()); + assertTrue(rs.next()); int result = rs.getInt(1); assertEquals(result, 4); assertFalse(rs.next()); @@ -1279,7 +1280,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testArraySizeRoundtrip() throws Exception { long ts = nextTimestamp(); @@ -1292,17 +1293,17 @@ public class ArrayIT extends BaseClientManagedTimeIT { Connection conn = DriverManager.getConnection(getUrl(), props); try { ResultSet rs = conn.getMetaData().getColumns(null, null, StringUtil.escapeLike(TABLE_WITH_ARRAY), StringUtil.escapeLike(SchemaUtil.normalizeIdentifier("x_long_array"))); - assertTrue(rs.next()); + assertTrue(rs.next()); assertEquals(5, rs.getInt("ARRAY_SIZE")); assertFalse(rs.next()); rs = conn.getMetaData().getColumns(null, null, StringUtil.escapeLike(TABLE_WITH_ARRAY), StringUtil.escapeLike(SchemaUtil.normalizeIdentifier("a_string_array"))); - assertTrue(rs.next()); + assertTrue(rs.next()); assertEquals(3, rs.getInt("ARRAY_SIZE")); assertFalse(rs.next()); rs = conn.getMetaData().getColumns(null, null, StringUtil.escapeLike(TABLE_WITH_ARRAY), StringUtil.escapeLike(SchemaUtil.normalizeIdentifier("a_double_array"))); - assertTrue(rs.next()); + assertTrue(rs.next()); assertEquals(0, rs.getInt("ARRAY_SIZE")); assertTrue(rs.wasNull()); assertFalse(rs.next()); @@ -1310,13 +1311,13 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testVarLengthArrComparisonInWhereClauseWithSameArrays() throws Exception { Connection conn; PreparedStatement stmt; ResultSet rs; - + long ts = nextTimestamp(); Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 10)); @@ -1325,7 +1326,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { .execute( "CREATE TABLE t_same_size ( k VARCHAR PRIMARY KEY, a_string_array VARCHAR(100) ARRAY[4], b_string_array VARCHAR(100) ARRAY[4])"); conn.close(); - + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 30)); conn = DriverManager.getConnection(getUrl(), props); stmt = conn.prepareStatement("UPSERT INTO t_same_size VALUES(?,?,?)"); @@ -1339,7 +1340,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { stmt.execute(); conn.commit(); conn.close(); - + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 40)); conn = DriverManager.getConnection(getUrl(), props); rs = conn.createStatement().executeQuery("SELECT k, a_string_array[2] FROM t_same_size where a_string_array=b_string_array"); @@ -1348,7 +1349,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { assertEquals("def",rs.getString(2)); conn.close(); } - + @Test public void testVarLengthArrComparisonInWhereClauseWithDiffSizeArrays() throws Exception { Connection conn; @@ -1386,7 +1387,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { assertEquals("def", rs.getString(2)); conn.close(); } - + @Test public void testVarLengthArrComparisonWithNulls() throws Exception { Connection conn; @@ -1462,7 +1463,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { conn.close(); } } - + @Test public void testUpsertValuesWithNullUsingPreparedStmt() throws Exception { long ts = nextTimestamp(); @@ -1561,7 +1562,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { } } - + @Test public void testArrayRefToLiteral() throws Exception { Connection conn; @@ -1585,7 +1586,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { } } - + static void createTableWithArray(String url, byte[][] bs, Object object, long ts) throws SQLException { String ddlStmt = "create table " @@ -1611,7 +1612,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { + ")"; BaseTest.createTestTable(url, ddlStmt, bs, ts); } - + static void createSimpleTableWithArray(String url, byte[][] bs, Object object, long ts) throws SQLException { String ddlStmt = "create table " @@ -1625,7 +1626,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { + ")"; BaseTest.createTestTable(url, ddlStmt, bs, ts); } - + protected static void initSimpleArrayTable(String tenantId, Date date, Long ts, boolean useNull) throws Exception { Properties props = new Properties(); if (ts != null) { @@ -1652,7 +1653,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { //doubleArr[2] = 9.9; Array array = conn.createArrayOf("DOUBLE", doubleArr); stmt.setArray(4, array); - + // create character array String[] charArr = new String[2]; charArr[0] = "a"; @@ -1660,7 +1661,7 @@ public class ArrayIT extends BaseClientManagedTimeIT { array = conn.createArrayOf("CHAR", charArr); stmt.setArray(5, array); stmt.execute(); - + conn.commit(); } finally { conn.close(); http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a4bd67/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ScanRegionObserver.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ScanRegionObserver.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ScanRegionObserver.java index 6d8a834..b5682af 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ScanRegionObserver.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ScanRegionObserver.java @@ -22,9 +22,10 @@ import java.io.ByteArrayOutputStream; import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; -import java.util.ArrayList; import java.util.List; +import java.util.Set; +import com.google.common.collect.Sets; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.KeyValue; @@ -62,13 +63,13 @@ import com.google.common.collect.Lists; /** - * + * * Wraps the scan performing a non aggregate query to prevent needless retries * if a Phoenix bug is encountered from our custom filter expression evaluation. * Unfortunately, until HBASE-7481 gets fixed, there's no way to do this from our * custom filters. * - * + * * @since 0.1 */ public class ScanRegionObserver extends BaseScannerRegionObserver { @@ -97,7 +98,7 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { } } } - + public static OrderedResultIterator deserializeFromScan(Scan scan, RegionScanner s) { byte[] topN = scan.getAttribute(BaseScannerRegionObserver.TOPN); if (topN == null) { @@ -110,7 +111,7 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { int limit = WritableUtils.readVInt(input); int estimatedRowSize = WritableUtils.readVInt(input); int size = WritableUtils.readVInt(input); - List<OrderByExpression> orderByExpressions = Lists.newArrayListWithExpectedSize(size); + List<OrderByExpression> orderByExpressions = Lists.newArrayListWithExpectedSize(size); for (int i = 0; i < size; i++) { OrderByExpression orderByExpression = new OrderByExpression(); orderByExpression.readFields(input); @@ -128,9 +129,9 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { } } } - + private Expression[] deserializeArrayPostionalExpressionInfoFromScan(Scan scan, RegionScanner s, - List<KeyValueColumnExpression> arrayKVRefs) { + Set<KeyValueColumnExpression> arrayKVRefs) { byte[] specificArrayIdx = scan.getAttribute(BaseScannerRegionObserver.SPECIFIC_ARRAY_INDEX); if (specificArrayIdx == null) { return null; @@ -172,14 +173,14 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { final TupleProjector p = TupleProjector.deserializeProjectorFromScan(scan); final HashJoinInfo j = HashJoinInfo.deserializeHashJoinFromScan(scan); final ImmutableBytesWritable tenantId = ScanUtil.getTenantId(scan); - + RegionScanner innerScanner = s; if (p != null || j != null) { innerScanner = new HashJoinRegionScanner(s, p, j, tenantId, c.getEnvironment()); } - + final OrderedResultIterator iterator = deserializeFromScan(scan,innerScanner); - List<KeyValueColumnExpression> arrayKVRefs = new ArrayList<KeyValueColumnExpression>(); + Set<KeyValueColumnExpression> arrayKVRefs = Sets.newHashSet(); Expression[] arrayFuncRefs = deserializeArrayPostionalExpressionInfoFromScan( scan, innerScanner, arrayKVRefs); innerScanner = getWrappedScanner(c, innerScanner, arrayKVRefs, arrayFuncRefs); @@ -189,7 +190,7 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { // TODO:the above wrapped scanner should be used here also return getTopNScanner(c, innerScanner, iterator, tenantId); } - + /** * Return region scanner that does TopN. * We only need to call startRegionOperation and closeRegionOperation when @@ -218,10 +219,10 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { } return new BaseRegionScanner() { private Tuple tuple = firstTuple; - + @Override public boolean isFilterDone() { - return tuple == null; + return tuple == null; } @Override @@ -235,11 +236,11 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { if (isFilterDone()) { return false; } - + for (int i = 0; i < tuple.size(); i++) { results.add(tuple.getValue(i)); } - + tuple = iterator.next(); return !isFilterDone(); } catch (Throwable t) { @@ -257,17 +258,17 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { } }; } - + /** * Return wrapped scanner that catches unexpected exceptions (i.e. Phoenix bugs) and * re-throws as DoNotRetryIOException to prevent needless retrying hanging the query * for 30 seconds. Unfortunately, until HBASE-7481 gets fixed, there's no way to do * the same from a custom filter. - * @param arrayFuncRefs - * @param arrayKVRefs + * @param arrayFuncRefs + * @param arrayKVRefs */ - private RegionScanner getWrappedScanner(final ObserverContext<RegionCoprocessorEnvironment> c, final RegionScanner s, - final List<KeyValueColumnExpression> arrayKVRefs, final Expression[] arrayFuncRefs) { + private RegionScanner getWrappedScanner(final ObserverContext<RegionCoprocessorEnvironment> c, final RegionScanner s, + final Set<KeyValueColumnExpression> arrayKVRefs, final Expression[] arrayFuncRefs) { return new RegionScanner() { @Override @@ -329,7 +330,7 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { public boolean reseek(byte[] row) throws IOException { return s.reseek(row); } - + @Override public long getMvccReadPoint() { return s.getMvccReadPoint(); @@ -352,7 +353,7 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { } } - + @Override public boolean nextRaw(List<KeyValue> result, int limit, String metric) throws IOException { @@ -360,8 +361,8 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { boolean next = s.nextRaw(result, limit, metric); if (result.size() == 0) { return next; - } else if ((arrayFuncRefs != null && arrayFuncRefs.length == 0) || arrayKVRefs.size() == 0) { - return next; + } else if ((arrayFuncRefs != null && arrayFuncRefs.length == 0) || arrayKVRefs.size() == 0) { + return next; } // There is a scanattribute set to retrieve the specific array element replaceArrayIndexElement(arrayKVRefs, arrayFuncRefs, result); @@ -372,15 +373,14 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { } } - private void replaceArrayIndexElement(final List<KeyValueColumnExpression> arrayKVRefs, + private void replaceArrayIndexElement(final Set<KeyValueColumnExpression> arrayKVRefs, final Expression[] arrayFuncRefs, List<KeyValue> result) { MultiKeyValueTuple tuple = new MultiKeyValueTuple(result); // The size of both the arrays would be same? // Using KeyValueSchema to set and retrieve the value // collect the first kv to get the row KeyValue rowKv = result.get(0); - for (int i = 0; i < arrayKVRefs.size(); i++) { - KeyValueColumnExpression kvExp = arrayKVRefs.get(i); + for (KeyValueColumnExpression kvExp : arrayKVRefs) { if (kvExp.evaluate(tuple, ptr)) { for (int idx = tuple.size() - 1; idx >= 0; idx--) { KeyValue kv = tuple.getValue(idx); @@ -409,6 +409,6 @@ public class ScanRegionObserver extends BaseScannerRegionObserver { protected boolean isRegionObserverFor(Scan scan) { return scan.getAttribute(BaseScannerRegionObserver.NON_AGGREGATE_QUERY) != null; } - - + + }