This is an automated email from the ASF dual-hosted git repository.
yanxinyi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/master by this push:
new ce0de92 PHOENIX-6022 RVC Offset does not handle trailing nulls
properly
ce0de92 is described below
commit ce0de92029efecbfe3be2bd47dfc57e1da7cf6a0
Author: Daniel Wong <daniel.w...@salesforce.com>
AuthorDate: Wed Jul 29 13:02:34 2020 -0700
PHOENIX-6022 RVC Offset does not handle trailing nulls properly
Signed-off-by: Xinyi Yan <yanxi...@apache.org>
---
.../end2end/RowValueConstructorOffsetIT.java | 83 ++++++++++++++++++--
.../apache/phoenix/compile/RVCOffsetCompiler.java | 88 ++++++++++++++++------
.../phoenix/compile/RVCOffsetCompilerTest.java | 33 ++++----
3 files changed, 159 insertions(+), 45 deletions(-)
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorOffsetIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorOffsetIT.java
index 841e4aa..0981757 100644
---
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorOffsetIT.java
+++
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorOffsetIT.java
@@ -21,6 +21,7 @@ import static
org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -1116,19 +1117,22 @@ public class RowValueConstructorOffsetIT extends
ParallelStatsDisabledIT {
@Test
public void rvcOffsetTrailingVariableLengthKeyTest() throws Exception {
- String TEST_DDL = "CREATE TABLE IF NOT EXISTS TEST_SCHEMA (\n"
+
+ String tableName = generateUniqueName();
+ String ddl = String.format("CREATE TABLE IF NOT EXISTS %s (\n"
+ " ORGANIZATION_ID VARCHAR(15), \n" + " TEST_ID VARCHAR(15),
\n"
+ " CREATED_DATE DATE, \n" + " LAST_UPDATE DATE\n"
- + " CONSTRAINT TEST_SCHEMA_PK PRIMARY KEY (ORGANIZATION_ID,
TEST_ID) \n" + ")";
+ + " CONSTRAINT TEST_SCHEMA_PK PRIMARY KEY (ORGANIZATION_ID,
TEST_ID) \n" + ")",tableName);
try (Statement statement = conn.createStatement()) {
- statement.execute(TEST_DDL);
+ statement.execute(ddl);
}
//setup
+ String upsert = "UPSERT INTO %s(ORGANIZATION_ID,TEST_ID) VALUES
(%s,%s)";
List<String> upserts = new ArrayList<>();
- upserts.add("UPSERT INTO TEST_SCHEMA(ORGANIZATION_ID,TEST_ID) VALUES
('1','1')");
- upserts.add("UPSERT INTO TEST_SCHEMA(ORGANIZATION_ID,TEST_ID) VALUES
('1','10')");
- upserts.add("UPSERT INTO TEST_SCHEMA(ORGANIZATION_ID,TEST_ID) VALUES
('2','2')");
+ upserts.add(String.format(upsert,tableName,"'1'","'1'"));
+ upserts.add(String.format(upsert,tableName,"'1'","'10'"));
+ upserts.add(String.format(upsert,tableName,"'2'","'2'"));
for(String sql : upserts) {
try (Statement statement = conn.createStatement()) {
@@ -1137,7 +1141,7 @@ public class RowValueConstructorOffsetIT extends
ParallelStatsDisabledIT {
}
conn.commit();
- String query = "SELECT * FROM TEST_SCHEMA OFFSET
(ORGANIZATION_ID,TEST_ID) = ('1','1')";
+ String query = String.format("SELECT * FROM %s OFFSET
(ORGANIZATION_ID,TEST_ID) = ('1','1')",tableName);
try (Statement statement = conn.createStatement() ; ResultSet rs2 =
statement.executeQuery(query) ) {
assertTrue(rs2.next());
@@ -1150,4 +1154,69 @@ public class RowValueConstructorOffsetIT extends
ParallelStatsDisabledIT {
}
}
+ //Note that phoenix does not suport a rowkey with a null inserted so there
is no single key version of this.
+ @Test
+ public void rvcOffsetTrailingNullKeyTest() throws Exception {
+ String tableName = generateUniqueName();
+ String ddl = String.format("CREATE TABLE IF NOT EXISTS %s (\n"
+ + " ORGANIZATION_ID VARCHAR(15), \n" + " TEST_ID VARCHAR(15),
\n"
+ + " CREATED_DATE DATE, \n" + " LAST_UPDATE DATE\n"
+ + " CONSTRAINT TEST_SCHEMA_PK PRIMARY KEY (ORGANIZATION_ID,
TEST_ID) \n" + ")",tableName);
+
+ try (Statement statement = conn.createStatement()) {
+ statement.execute(ddl);
+ }
+ //setup
+ String upsert = "UPSERT INTO %s(ORGANIZATION_ID,TEST_ID) VALUES
(%s,%s)";
+ List<String> upserts = new ArrayList<>();
+ upserts.add(String.format(upsert,tableName,"'0'","null"));
+ upserts.add(String.format(upsert,tableName,"'1'","null"));
+ upserts.add(String.format(upsert,tableName,"'1'","'1'"));
+ upserts.add(String.format(upsert,tableName,"'1'","'10'"));
+ upserts.add(String.format(upsert,tableName,"'2'","null"));
+
+ for(String sql : upserts) {
+ try (Statement statement = conn.createStatement()) {
+ statement.execute(sql);
+ }
+ }
+ conn.commit();
+
+ String query = String.format("SELECT * FROM %s OFFSET
(ORGANIZATION_ID,TEST_ID) = ('1',null)",tableName);
+
+ try (Statement statement = conn.createStatement() ; ResultSet rs2 =
statement.executeQuery(query) ) {
+ assertTrue(rs2.next());
+ assertEquals("1",rs2.getString(1));
+ assertEquals("1",rs2.getString(2));
+ assertTrue(rs2.next());
+ assertEquals("1",rs2.getString(1));
+ assertEquals("10",rs2.getString(2));
+ assertTrue(rs2.next());
+ assertEquals("2",rs2.getString(1));
+ assertNull(rs2.getString(2));
+ assertFalse(rs2.next());
+ }
+
+ String preparedQuery = String.format("SELECT * FROM %s OFFSET
(ORGANIZATION_ID,TEST_ID) = (?,?)",tableName);
+
+ try (PreparedStatement statement =
conn.prepareStatement(preparedQuery) ) {
+
+ statement.setString(1,"1");
+ statement.setString(2,null);
+
+ try(ResultSet rs2 = statement.executeQuery()) {
+ assertTrue(rs2.next());
+ assertEquals("1", rs2.getString(1));
+ assertEquals("1", rs2.getString(2));
+ assertTrue(rs2.next());
+ assertEquals("1", rs2.getString(1));
+ assertEquals("10", rs2.getString(2));
+ assertTrue(rs2.next());
+ assertEquals("2", rs2.getString(1));
+ assertNull(rs2.getString(2));
+ assertFalse(rs2.next());
+ }
+ }
+ }
+
}
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java
b/phoenix-core/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java
index bf6514d..afcd00e 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java
@@ -195,9 +195,12 @@ public class RVCOffsetCompiler {
}
// Now that columns etc have been resolved lets check to make sure
they match the pk order
- List<RowKeyColumnExpression>
- rowKeyColumnExpressionList =
+ RowKeyColumnExpressionOutput rowKeyColumnExpressionOutput =
buildListOfRowKeyColumnExpressions(whereExpression, isIndex);
+
+ List<RowKeyColumnExpression>
+ rowKeyColumnExpressionList =
rowKeyColumnExpressionOutput.getRowKeyColumnExpressions();
+
if (rowKeyColumnExpressionList.size() != numUserColumns) {
LOGGER.warn("Unexpected error while compiling RVC Offset, expected
" + numUserColumns
+ " found " + rowKeyColumnExpressionList.size());
@@ -240,34 +243,41 @@ public class RVCOffsetCompiler {
}
}
+ byte[] key;
+
// check to see if this was a single key expression
ScanRanges scanRanges = context.getScanRanges();
+ //We do not generate a point lookup today in phoenix if the rowkey has
a trailing null, we generate a range scan.
if (!scanRanges.isPointLookup()) {
- throw new RowValueConstructorOffsetNotCoercibleException(
- "RVC Offset must be a point lookup.");
- }
-
-
- RowKeySchema.RowKeySchemaBuilder builder = new
RowKeySchema.RowKeySchemaBuilder(columns.size());
+ //Since we use a range scan to guarantee we get only the null
value and the upper bound is unset this suffices
+ //sanity check
+ if (!rowKeyColumnExpressionOutput.isTrailingNull()) {
+ throw new RowValueConstructorOffsetNotCoercibleException(
+ "RVC Offset must be a point lookup.");
+ }
+ key = scanRanges.getScanRange().getUpperRange();
+ } else {
+ RowKeySchema.RowKeySchemaBuilder builder = new
RowKeySchema.RowKeySchemaBuilder(columns.size());
- for(PColumn column : columns) {
- builder.addField(column, column.isNullable(),
column.getSortOrder());
- }
+ for (PColumn column : columns) {
+ builder.addField(column, column.isNullable(),
column.getSortOrder());
+ }
- RowKeySchema rowKeySchema = builder.build();
+ RowKeySchema rowKeySchema = builder.build();
- //we make a ScanRange with 1 keyslots that cover the entire PK to
reuse the code
- KeyRange pointKeyRange = scanRanges.getScanRange();
- KeyRange keyRange =
KeyRange.getKeyRange(pointKeyRange.getLowerRange(), false, KeyRange.UNBOUND,
true);
- List<KeyRange> myRangeList = Lists.newArrayList(keyRange);
- List<List<KeyRange>> slots = new ArrayList<>();
- slots.add(myRangeList);
- int[] slotSpan = new int[1];
+ //we make a ScanRange with 1 keyslots that cover the entire PK to
reuse the code
+ KeyRange pointKeyRange = scanRanges.getScanRange();
+ KeyRange keyRange =
KeyRange.getKeyRange(pointKeyRange.getLowerRange(), false, KeyRange.UNBOUND,
true);
+ List<KeyRange> myRangeList = Lists.newArrayList(keyRange);
+ List<List<KeyRange>> slots = new ArrayList<>();
+ slots.add(myRangeList);
+ int[] slotSpan = new int[1];
- //subtract 1 see ScanUtil.SINGLE_COLUMN_SLOT_SPAN is 0
- slotSpan[0] = columns.size() - 1;
- byte[] key = ScanUtil.getMinKey(rowKeySchema, slots, slotSpan);
+ //subtract 1 see ScanUtil.SINGLE_COLUMN_SLOT_SPAN is 0
+ slotSpan[0] = columns.size() - 1;
+ key = ScanUtil.getMinKey(rowKeySchema, slots, slotSpan);
+ }
// Note the use of ByteUtil.nextKey() to generate exclusive offset
CompiledOffset
@@ -279,10 +289,30 @@ public class RVCOffsetCompiler {
}
@VisibleForTesting
- List<RowKeyColumnExpression> buildListOfRowKeyColumnExpressions (
+ static class RowKeyColumnExpressionOutput {
+ public RowKeyColumnExpressionOutput(List<RowKeyColumnExpression>
rowKeyColumnExpressions, boolean trailingNull) {
+ this.rowKeyColumnExpressions = rowKeyColumnExpressions;
+ this.trailingNull = trailingNull;
+ }
+
+ private final List<RowKeyColumnExpression> rowKeyColumnExpressions;
+ private final boolean trailingNull;
+
+ public List<RowKeyColumnExpression> getRowKeyColumnExpressions() {
+ return rowKeyColumnExpressions;
+ }
+
+ public boolean isTrailingNull() {
+ return trailingNull;
+ }
+ }
+
+ @VisibleForTesting
+ RowKeyColumnExpressionOutput buildListOfRowKeyColumnExpressions (
Expression whereExpression, boolean isIndex)
throws RowValueConstructorOffsetNotCoercibleException,
RowValueConstructorOffsetInternalErrorException {
+ boolean trailingNull = false;
List<Expression> expressions;
if((whereExpression instanceof AndExpression)) {
expressions = whereExpression.getChildren();
@@ -298,13 +328,21 @@ public class RVCOffsetCompiler {
List<RowKeyColumnExpression>
rowKeyColumnExpressionList =
new ArrayList<RowKeyColumnExpression>();
- for (Expression child : expressions) {
+ for (int i = 0; i < expressions.size(); i++) {
+ Expression child = expressions.get(i);
if (!(child instanceof ComparisonExpression || child instanceof
IsNullExpression)) {
LOGGER.warn("Unexpected error while compiling RVC Offset");
throw new RowValueConstructorOffsetNotCoercibleException(
"RVC Offset must specify the tables PKs.");
}
+ //if this is the last position
+ if(i == expressions.size() - 1) {
+ if(child instanceof IsNullExpression) {
+ trailingNull = true;
+ }
+ }
+
//For either case comparison/isNull the first child should be the
rowkey
Expression possibleRowKeyColumnExpression =
child.getChildren().get(0);
@@ -325,7 +363,7 @@ public class RVCOffsetCompiler {
}
rowKeyColumnExpressionList.add((RowKeyColumnExpression)
possibleRowKeyColumnExpression);
}
- return rowKeyColumnExpressionList;
+ return new
RowKeyColumnExpressionOutput(rowKeyColumnExpressionList,trailingNull);
}
@VisibleForTesting List<ColumnParseNode> buildListOfColumnParseNodes(
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/compile/RVCOffsetCompilerTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/compile/RVCOffsetCompilerTest.java
index a1c62b4..93c2c48 100644
---
a/phoenix-core/src/test/java/org/apache/phoenix/compile/RVCOffsetCompilerTest.java
+++
b/phoenix-core/src/test/java/org/apache/phoenix/compile/RVCOffsetCompilerTest.java
@@ -18,6 +18,7 @@
package org.apache.phoenix.compile;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import java.util.ArrayList;
@@ -119,9 +120,10 @@ public class RVCOffsetCompilerTest {
AndExpression expression = mock(AndExpression.class);
Mockito.when(expression.getChildren()).thenReturn(expressions);
+ RVCOffsetCompiler.RowKeyColumnExpressionOutput
+ output =
offsetCompiler.buildListOfRowKeyColumnExpressions(expression, false);
List<RowKeyColumnExpression>
- result =
- offsetCompiler.buildListOfRowKeyColumnExpressions(expression,
false);
+ result = output.getRowKeyColumnExpressions();
assertEquals(2,result.size());
assertEquals(rvc1,result.get(0));
@@ -156,9 +158,10 @@ public class RVCOffsetCompilerTest {
AndExpression expression = mock(AndExpression.class);
Mockito.when(expression.getChildren()).thenReturn(expressions);
+ RVCOffsetCompiler.RowKeyColumnExpressionOutput
+ output =
offsetCompiler.buildListOfRowKeyColumnExpressions(expression, true);
List<RowKeyColumnExpression>
- result =
- offsetCompiler.buildListOfRowKeyColumnExpressions(expression,
true);
+ result = output.getRowKeyColumnExpressions();
assertEquals(2,result.size());
assertEquals(rvc1,result.get(0));
@@ -175,9 +178,10 @@ public class RVCOffsetCompilerTest {
Mockito.when(expression.getChildren()).thenReturn(Lists.<Expression>newArrayList(rvc));
+ RVCOffsetCompiler.RowKeyColumnExpressionOutput
+ output =
offsetCompiler.buildListOfRowKeyColumnExpressions(expression, false);
List<RowKeyColumnExpression>
- result =
- offsetCompiler.buildListOfRowKeyColumnExpressions(expression,
false);
+ result = output.getRowKeyColumnExpressions();
assertEquals(1,result.size());
assertEquals(rvc,result.get(0));
@@ -193,12 +197,14 @@ public class RVCOffsetCompilerTest {
Mockito.when(expression.getChildren()).thenReturn(Lists.<Expression>newArrayList(rvc));
- List<RowKeyColumnExpression>
- result =
- offsetCompiler.buildListOfRowKeyColumnExpressions(expression,
false);
+ RVCOffsetCompiler.RowKeyColumnExpressionOutput output =
offsetCompiler.buildListOfRowKeyColumnExpressions(expression, false);
+
+ List<RowKeyColumnExpression> result =
output.getRowKeyColumnExpressions();
assertEquals(1,result.size());
assertEquals(rvc,result.get(0));
+
+ assertTrue(output.isTrailingNull());
}
@Test
@@ -220,13 +226,14 @@ public class RVCOffsetCompilerTest {
AndExpression expression = mock(AndExpression.class);
Mockito.when(expression.getChildren()).thenReturn(expressions);
- List<RowKeyColumnExpression>
- result =
- offsetCompiler.buildListOfRowKeyColumnExpressions(expression,
false);
+ RVCOffsetCompiler.RowKeyColumnExpressionOutput output =
offsetCompiler.buildListOfRowKeyColumnExpressions(expression, false);
+
+ List<RowKeyColumnExpression> result =
output.getRowKeyColumnExpressions();
assertEquals(2,result.size());
assertEquals(rvc1,result.get(0));
-
assertEquals(rvc2,result.get(1));
+
+ assertTrue(output.isTrailingNull());
}
}
