From 0ac2b0ee50ae32f69dd89eeedc34f5db2c364d3e Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Mon, 16 Feb 2026 19:05:31 +0530 Subject: [PATCH 01/15] Generate point lookups for IS NULL --- .../apache/phoenix/compile/ScanRanges.java | 21 +- .../end2end/WhereOptimizerForArrayAnyIT.java | 272 ++++++++++++++++++ .../phoenix/compile/WhereOptimizerTest.java | 36 ++- 3 files changed, 314 insertions(+), 15 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index fc2176d6b24..8c0ca737f2d 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -101,6 +101,9 @@ public static ScanRanges create(RowKeySchema schema, List> ranges TimeRange rowTimestampRange = getRowTimestampColumnRange(ranges, schema, rowTimestampColIndex); boolean isPointLookup = isPointLookup(schema, ranges, slotSpan, useSkipScan); if (isPointLookup) { + if (ranges.get(ranges.size() - 1).get(0) == KeyRange.IS_NULL_RANGE) { + System.out.println("IS_NULL_RANGE: "); + } // TODO: consider keeping original to use for serialization as it would be smaller? List keys = ScanRanges.getPointKeys(ranges, slotSpan, schema, nBuckets); List keyRanges = Lists.newArrayListWithExpectedSize(keys.size()); @@ -618,9 +621,12 @@ private static boolean isPointLookup(RowKeySchema schema, List> r return false; } for (KeyRange keyRange : orRanges) { - // Special case for single trailing IS NULL. We cannot consider this as a point key because - // we strip trailing nulls when we form the key. - if (!keyRange.isSingleKey() || (i == lastIndex && keyRange == KeyRange.IS_NULL_RANGE)) { + // COL IS NULL on trailing nullable PK column can be treated as a point lookup + // because: + // 1. Trailing nulls are stripped when storing the key (no trailing separator bytes) + // 2. getPointKeys() generates the correct key without trailing null bytes + // 3. The generated key matches exactly what's stored for rows with trailing NULL + if (!keyRange.isSingleKey()) { return false; } } @@ -644,9 +650,18 @@ private static List getPointKeys(List> ranges, int[] slot boolean isSalted = bucketNum != null; int count = 1; int offset = isSalted ? 1 : 0; + int lastNonNullIndex = -1; // Skip salt byte range in the first position if salted for (int i = offset; i < ranges.size(); i++) { count *= ranges.get(i).size(); + if (ranges.get(i).size() == 1 && ranges.get(i).get(0) == KeyRange.IS_NULL_RANGE) { + continue; + } + lastNonNullIndex = i; + } + if (lastNonNullIndex != -1) { + // Create ranges without trailing null + ranges = ranges.subList(0, lastNonNullIndex + 1); } List keys = Lists.newArrayListWithExpectedSize(count); int[] position = new int[ranges.size()]; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java index dd47aefc5b5..3c0a2f1ed14 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java @@ -19,6 +19,7 @@ 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 java.math.BigDecimal; @@ -43,6 +44,7 @@ import org.apache.phoenix.query.BaseTest; import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.DateUtil; +import org.apache.phoenix.util.QueryUtil; import org.apache.phoenix.util.ReadOnlyProps; import org.apache.phoenix.util.TestUtil; import org.bson.RawBsonDocument; @@ -888,4 +890,274 @@ private static RawBsonDocument getBsonDocument2() { return RawBsonDocument.parse(json); } + private static final BigDecimal PK3_VAL = new BigDecimal("100.5"); + + /** + * Creates a table with 5 PK columns (last one nullable) and inserts test data. + * Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 DECIMAL, PK4 VARCHAR, PK5 DECIMAL (nullable) + * COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR + * Inserts 5 rows: + * Row 1: (A, B, 100.5, X, NULL, val1, val2, val3) + * Row 2: (A, B, 100.5, Y, NULL, val4, val5, val6) + * Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) + * Row 4: (A, B, 100.5, Z, NULL, val10, val11, val12) + * Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) + * @param pk5Desc if true, PK5 will have DESC sort order + * @return the generated table name + */ + private String createTableAndInsertTestDataForNullablePKTests(boolean pk4Desc, boolean pk5Desc) throws Exception { + String tableName = generateUniqueName(); + String pk4SortOrder = pk4Desc ? " DESC" : ""; + String pk5SortOrder = pk5Desc ? " DESC" : ""; + String ddl = "CREATE TABLE " + tableName + " (" + + "PK1 VARCHAR NOT NULL, " + + "PK2 VARCHAR NOT NULL, " + + "PK3 DECIMAL NOT NULL, " + + "PK4 VARCHAR NOT NULL, " + + "PK5 DECIMAL, " + + "COL1 VARCHAR, " + + "COL2 VARCHAR, " + + "COL3 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + pk4SortOrder + ", PK5" + pk5SortOrder + ")" + + ")"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (Statement stmt = conn.createStatement()) { + stmt.execute(ddl); + conn.commit(); + } + } + + String upsertStmt = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, PK4, PK5, COL1, COL2, COL3) " + + "VALUES (?, ?, ?, ?, ?, ?, ?, ?)"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (PreparedStatement stmt = conn.prepareStatement(upsertStmt)) { + // Row 1: PK5 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val1"); + stmt.setString(7, "val2"); + stmt.setString(8, "val3"); + stmt.executeUpdate(); + + // Row 2: PK5 is NULL, different PK4 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Y"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val4"); + stmt.setString(7, "val5"); + stmt.setString(8, "val6"); + stmt.executeUpdate(); + + // Row 3: PK5 is NOT NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setBigDecimal(5, new BigDecimal("1.0")); + stmt.setString(6, "val7"); + stmt.setString(7, "val8"); + stmt.setString(8, "val9"); + stmt.executeUpdate(); + + // Row 4: PK5 is NULL, different PK4 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Z"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val10"); + stmt.setString(7, "val11"); + stmt.setString(8, "val12"); + stmt.executeUpdate(); + + // Row 5: Different PK1 + stmt.setString(1, "C"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val13"); + stmt.setString(7, "val14"); + stmt.setString(8, "val15"); + stmt.executeUpdate(); + + conn.commit(); + } + } + return tableName; + } + + @Test + public void testSinglePointLookupWithNullablePK() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); + + // Query with = for PK4 and IS NULL for PK5 + // IS NULL on trailing nullable PK column generates POINT LOOKUP because: + // - Trailing nulls are stripped when storing, so key for NULL matches stored key + // - The generated lookup key is exactly what's stored for rows with trailing NULL + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " + + "AND PK4 = ? AND PK5 IS NULL"; + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='X' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("X", rs.getString("PK4")); + assertEquals("val1", rs.getString("COL1")); + assertEquals("val2", rs.getString("COL2")); + assertEquals("val3", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + + stmt.setString(4, "Y"); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("Y", rs.getString("PK4")); + assertEquals("val4", rs.getString("COL1")); + assertEquals("val5", rs.getString("COL2")); + assertEquals("val6", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + } + } + } + + @Test + public void testMultiPointLookupsWithNullablePK() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); + + // Query with =ANY(?) for PK4 and IS NULL for PK5 + // IS NULL on trailing nullable PK column generates POINT LOOKUPS because: + // - Trailing nulls are stripped when storing, so key for NULL matches stored key + // - The generated lookup key is exactly what's stored for rows with trailing NULL + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; + Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setArray(4, pk4Arr); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + String pk4Val = rs.getString("PK4"); + assertTrue("X".equals(pk4Val)); + assertNull(rs.getBytes("PK5")); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Y".equals(pk4Val)); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) + assertPointLookupsAreGenerated(stmt, 2); + } + } + } + + @Test + public void testSinglePointLookupWithNullablePKDesc() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); + + // Query with = for PK4 and IS NULL for PK5 (DESC sort order) + // IS NULL on trailing nullable PK column with DESC sort order generates POINT LOOKUP + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " + + "AND PK4 = ? AND PK5 IS NULL"; + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='X' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("X", rs.getString("PK4")); + assertEquals("val1", rs.getString("COL1")); + assertEquals("val2", rs.getString("COL2")); + assertEquals("val3", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + + stmt.setString(4, "Y"); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("Y", rs.getString("PK4")); + assertEquals("val4", rs.getString("COL1")); + assertEquals("val5", rs.getString("COL2")); + assertEquals("val6", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + } + } + } + + @Test + public void testMultiPointLookupsWithNullablePKDesc() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); + + // Query with =ANY(?) for PK4 and IS NULL for PK5 (DESC sort order) + // IS NULL on trailing nullable PK column with DESC generates POINT LOOKUPS + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; + Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setArray(4, pk4Arr); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + String pk4Val = rs.getString("PK4"); + assertTrue("X".equals(pk4Val)); + assertNull(rs.getBytes("PK5")); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Y".equals(pk4Val)); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) + assertPointLookupsAreGenerated(stmt, 2); + } + } + } } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java index 33f616f189e..67d9d55f6fc 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java @@ -2696,10 +2696,16 @@ public void testTrailingIsNull() throws Exception { StatementContext context = compileStatement(query, Collections. emptyList()); Scan scan = context.getScan(); Filter filter = scan.getFilter(); + // With trailing IS NULL as point lookup, no filter is needed assertNull(filter); - assertArrayEquals(Bytes.toBytes("a"), scan.getStartRow()); - assertArrayEquals(ByteUtil.concat(Bytes.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, - QueryConstants.SEPARATOR_BYTE_ARRAY), scan.getStopRow()); + // Point lookup for trailing IS NULL: startRow = "a", stopRow = "a\0" + // The separator is added to create an exclusive upper bound + byte[] expectedStartKey = Bytes.toBytes("a"); + byte[] expectedStopKey = ByteUtil.concat(expectedStartKey, QueryConstants.SEPARATOR_BYTE_ARRAY); + assertArrayEquals(expectedStartKey, scan.getStartRow()); + assertArrayEquals(expectedStopKey, scan.getStopRow()); + // Verify it's a point lookup + assertTrue(context.getScanRanges().isPointLookup()); } @Test @@ -2714,18 +2720,24 @@ public void testTrailingIsNullWithOr() throws Exception { StatementContext context = compileStatement(query, Collections. emptyList()); Scan scan = context.getScan(); Filter filter = scan.getFilter(); + // With trailing IS NULL as point lookup, when combined with OR, it becomes + // a point lookup with 2 keys (one for NULL, one for 'b') assertTrue(filter instanceof SkipScanFilter); SkipScanFilter skipScan = (SkipScanFilter) filter; List> slots = skipScan.getSlots(); - assertEquals(2, slots.size()); - assertEquals(1, slots.get(0).size()); - assertEquals(2, slots.get(1).size()); - assertEquals(KeyRange.getKeyRange(Bytes.toBytes("a")), slots.get(0).get(0)); - assertTrue(KeyRange.IS_NULL_RANGE == slots.get(1).get(0)); - assertEquals(KeyRange.getKeyRange(Bytes.toBytes("b")), slots.get(1).get(1)); - assertArrayEquals(Bytes.toBytes("a"), scan.getStartRow()); - assertArrayEquals(ByteUtil.concat(Bytes.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, - Bytes.toBytes("b"), QueryConstants.SEPARATOR_BYTE_ARRAY), scan.getStopRow()); + // Point lookup collapses to single slot with 2 keys + assertEquals(1, slots.size()); + assertEquals(2, slots.get(0).size()); + // Verify it's a point lookup + assertTrue(context.getScanRanges().isPointLookup()); + // Key 1: "a" (for b IS NULL - trailing separator stripped) + byte[] key1 = Bytes.toBytes("a"); + // Key 2: "a\0b" (for b = 'b' - trailing separator stripped for last column) + byte[] key2 = ByteUtil.concat(Bytes.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, + Bytes.toBytes("b")); + // Keys are sorted, so key1 comes before key2 + assertEquals(KeyRange.getKeyRange(key1), slots.get(0).get(0)); + assertEquals(KeyRange.getKeyRange(key2), slots.get(0).get(1)); } @Test From 9afd4e9034eee4b608e1745f6a15500f64f22f3e Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Tue, 17 Feb 2026 01:33:00 +0530 Subject: [PATCH 02/15] Add test coverage --- .../apache/phoenix/compile/ScanRanges.java | 10 +- .../end2end/WhereOptimizerForArrayAnyIT.java | 147 ++++++++++++++++++ 2 files changed, 154 insertions(+), 3 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index 8c0ca737f2d..9852597e652 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -100,9 +100,10 @@ public static ScanRanges create(RowKeySchema schema, List> ranges } TimeRange rowTimestampRange = getRowTimestampColumnRange(ranges, schema, rowTimestampColIndex); boolean isPointLookup = isPointLookup(schema, ranges, slotSpan, useSkipScan); + boolean isPointLookupWithNull = false; if (isPointLookup) { if (ranges.get(ranges.size() - 1).get(0) == KeyRange.IS_NULL_RANGE) { - System.out.println("IS_NULL_RANGE: "); + isPointLookupWithNull = true; } // TODO: consider keeping original to use for serialization as it would be smaller? List keys = ScanRanges.getPointKeys(ranges, slotSpan, schema, nBuckets); @@ -148,6 +149,9 @@ public static ScanRanges create(RowKeySchema schema, List> ranges if (nBuckets == null || !isPointLookup || !useSkipScan) { byte[] minKey = ScanUtil.getMinKey(schema, sortedRanges, slotSpan); byte[] maxKey = ScanUtil.getMaxKey(schema, sortedRanges, slotSpan); + if (isPointLookupWithNull) { + System.out.println("IS_NULL_RANGE: "); + } // If the maxKey has crossed the salt byte boundary, then we do not // have anything to filter at the upper end of the range if (ScanUtil.crossesPrefixBoundary(maxKey, ScanUtil.getPrefix(minKey, offset), offset)) { @@ -650,7 +654,7 @@ private static List getPointKeys(List> ranges, int[] slot boolean isSalted = bucketNum != null; int count = 1; int offset = isSalted ? 1 : 0; - int lastNonNullIndex = -1; + int lastNonNullIndex = offset - 1; // Skip salt byte range in the first position if salted for (int i = offset; i < ranges.size(); i++) { count *= ranges.get(i).size(); @@ -659,7 +663,7 @@ private static List getPointKeys(List> ranges, int[] slot } lastNonNullIndex = i; } - if (lastNonNullIndex != -1) { + if (lastNonNullIndex < ranges.size() - 1) { // Create ranges without trailing null ranges = ranges.subList(0, lastNonNullIndex + 1); } diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java index 3c0a2f1ed14..5d1dcf98249 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java @@ -817,6 +817,16 @@ private void assertPointLookupsAreGenerated(QueryPlan queryPlan, int noOfPointLo assertEquals(expectedScanType, planAttributes.getExplainScanType()); } + private void assertQueryUsesIndex(PreparedStatement stmt, String indexName) throws SQLException { + QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); + ExplainPlan explain = queryPlan.getExplainPlan(); + ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); + String tableName = planAttributes.getTableName(); + System.out.println("Explain plan: " + explain.toString()); + assertTrue("Expected query to use index " + indexName + " but used table " + tableName, + tableName != null && tableName.contains(indexName)); + } + private void createTableASCPkColumns(String tableName) throws SQLException { String ddl = "CREATE TABLE " + tableName + " (" + "pk1 INTEGER NOT NULL, " + "pk2 VARCHAR(3), " + "col1 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (pk1, pk2)" + ")"; @@ -1160,4 +1170,141 @@ public void testMultiPointLookupsWithNullablePKDesc() throws Exception { } } } + + @Test + public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { + String tableName = generateUniqueName(); + String indexName = "IDX_" + generateUniqueName(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + // Step 1: Create table with one nullable PK column (PK3) at the end + String createTableDdl = "CREATE TABLE " + tableName + " (" + + "PK1 VARCHAR NOT NULL, " + + "PK2 VARCHAR NOT NULL, " + + "PK3 VARCHAR, " // Nullable PK column at end + + "COL1 VARCHAR, " + + "COL2 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3)" + + ")"; + conn.createStatement().execute(createTableDdl); + conn.commit(); + + // Step 2: Create a covered global index on the data table + String createIndexDdl = "CREATE INDEX " + indexName + " ON " + tableName + + " (COL1) INCLUDE (COL2)"; + conn.createStatement().execute(createIndexDdl); + conn.commit(); + + // Step 3: Insert initial data (before ALTER TABLE) + // Row 1: PK3 is NULL + String upsertSql = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; + try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "indexed_val1"); + stmt.setString(5, "col2_val1"); + stmt.executeUpdate(); + + // Row 2: PK3 has a value + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setString(3, "pk3_val1"); + stmt.setString(4, "indexed_val2"); + stmt.setString(5, "col2_val2"); + stmt.executeUpdate(); + + // Row 3: Different PK prefix + stmt.setString(1, "C"); + stmt.setString(2, "D"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "indexed_val3"); + stmt.setString(5, "col2_val3"); + stmt.executeUpdate(); + } + conn.commit(); + + // Step 4: Add a new nullable PK column (PK4) via ALTER TABLE + String alterTableDdl = "ALTER TABLE " + tableName + " ADD PK4 VARCHAR PRIMARY KEY"; + conn.createStatement().execute(alterTableDdl); + conn.commit(); + + // Step 5: Insert more data with same PK prefix but different PK4 values + upsertSql = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, PK4, COL1, COL2) VALUES (?, ?, ?, ?, ?, ?)"; + try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { + // Row 4: Same prefix as Row 1 (A, B, NULL) but PK4 = 'X' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "X"); + stmt.setString(5, "indexed_val4"); + stmt.setString(6, "col2_val4"); + stmt.executeUpdate(); + + // Row 5: Same prefix as Row 1 (A, B, NULL) but PK4 = 'Y' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "Y"); + stmt.setString(5, "indexed_val5"); + stmt.setString(6, "col2_val5"); + stmt.executeUpdate(); + + // Row 6: Same prefix as Row 2 (A, B, pk3_val1) but PK4 = 'Y' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setString(3, "pk3_val1"); + stmt.setString(4, "Y"); + stmt.setString(5, "indexed_val6"); + stmt.setString(6, "col2_val6"); + stmt.executeUpdate(); + + // Row 7: Same prefix as Row 2 (A, B, NULL) but PK4 = 'Z' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "Z"); + stmt.setString(5, "indexed_val7"); + stmt.setString(6, "col2_val7"); + stmt.executeUpdate(); + } + conn.commit(); + + String selectSql = "SELECT /*+ INDEX(" + tableName + " " + indexName + ") */ " + + "PK1, PK2, PK3, PK4, COL1, COL2 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 IS NULL AND (PK4 IS NULL OR PK4 = ANY(?)) AND COL1 = ANY(?)"; + Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "Z", "Y" }); + Array col1Arr = conn.createArrayOf("VARCHAR", new String[] { "indexed_val5", "indexed_val1", "indexed_val7", "indexed_val4" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setArray(3, pk4Arr); + stmt.setArray(4, col1Arr); + try (ResultSet rs = stmt.executeQuery()) { + assertTrue(rs.next()); + String pk4Val = rs.getString("PK4"); + assertNull(pk4Val); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Y".equals(pk4Val)); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Z".equals(pk4Val)); + + // No more rows + assertFalse(rs.next()); + } + // Should generate point lookups for the three PK4 values + assertPointLookupsAreGenerated(stmt, 12); + // Assert that the query uses the index table + assertQueryUsesIndex(stmt, indexName); + } + } + } + + } From 0a534e400cbcbfd23b64483955d3533200696a70 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Tue, 17 Feb 2026 02:01:18 +0530 Subject: [PATCH 03/15] Add test coverage --- .../end2end/WhereOptimizerForArrayAnyIT.java | 137 +++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java index 5d1dcf98249..b89fcda2306 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java @@ -1306,5 +1306,140 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { } } - + @Test + public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception { + String tableName = generateUniqueName(); + String viewName = "VW_" + generateUniqueName(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + // Step 1: Create parent table with fixed-width NOT NULL last PK + // Using CHAR (fixed-width) for PK2 to allow view to add PK columns + String createTableDdl = "CREATE TABLE " + tableName + " (" + + "PK1 VARCHAR NOT NULL, " + + "PK2 CHAR(10) NOT NULL, " // Fixed-width NOT NULL - allows view to add PKs + + "COL1 VARCHAR, " + + "COL2 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2)" + + ")"; + conn.createStatement().execute(createTableDdl); + conn.commit(); + + // Step 2: Create view that adds two nullable PK columns + String createViewDdl = "CREATE VIEW " + viewName + " (" + + "VIEW_PK1 VARCHAR, " // Nullable PK column added by view + + "VIEW_PK2 VARCHAR, " // Second nullable PK column added by view + + "VIEW_COL1 VARCHAR, " + + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1, VIEW_PK2)" + + ") AS SELECT * FROM " + tableName; + conn.createStatement().execute(createViewDdl); + conn.commit(); + + // Step 3: Insert data through the view with various combinations + String upsertSql = "UPSERT INTO " + viewName + + " (PK1, PK2, VIEW_PK1, VIEW_PK2, COL1, COL2, VIEW_COL1) VALUES (?, ?, ?, ?, ?, ?, ?)"; + try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { + // Row 1: Both VIEW_PK1 and VIEW_PK2 are NULL + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setNull(3, Types.VARCHAR); + stmt.setNull(4, Types.VARCHAR); + stmt.setString(5, "col1_val1"); + stmt.setString(6, "col2_val1"); + stmt.setString(7, "view_col1_val1"); + stmt.executeUpdate(); + + // Row 2: VIEW_PK1 = 'X', VIEW_PK2 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "X"); + stmt.setNull(4, Types.VARCHAR); + stmt.setString(5, "col1_val2"); + stmt.setString(6, "col2_val2"); + stmt.setString(7, "view_col1_val2"); + stmt.executeUpdate(); + + // Row 3: VIEW_PK1 = 'X', VIEW_PK2 = 'P' + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "X"); + stmt.setString(4, "P"); + stmt.setString(5, "col1_val3"); + stmt.setString(6, "col2_val3"); + stmt.setString(7, "view_col1_val3"); + stmt.executeUpdate(); + + // Row 4: VIEW_PK1 = 'X', VIEW_PK2 = 'Q' + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "X"); + stmt.setString(4, "Q"); + stmt.setString(5, "col1_val4"); + stmt.setString(6, "col2_val4"); + stmt.setString(7, "view_col1_val4"); + stmt.executeUpdate(); + + // Row 5: VIEW_PK1 = 'Y', VIEW_PK2 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "Y"); + stmt.setNull(4, Types.VARCHAR); + stmt.setString(5, "col1_val5"); + stmt.setString(6, "col2_val5"); + stmt.setString(7, "view_col1_val5"); + stmt.executeUpdate(); + + // Row 6: VIEW_PK1 = 'Y', VIEW_PK2 = 'Q' + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "Y"); + stmt.setString(4, "Q"); + stmt.setString(5, "col1_val6"); + stmt.setString(6, "col2_val6"); + stmt.setString(7, "view_col1_val6"); + stmt.executeUpdate(); + + // Row 7: Different base PK prefix + stmt.setString(1, "B"); + stmt.setString(2, "BASE2"); + stmt.setString(3, "X"); + stmt.setString(4, "P"); + stmt.setString(5, "col1_val7"); + stmt.setString(6, "col2_val7"); + stmt.setString(7, "view_col1_val7"); + stmt.executeUpdate(); + } + conn.commit(); + + String selectSql = "SELECT PK1, PK2, VIEW_PK1, VIEW_PK2, COL1, VIEW_COL1 FROM " + viewName + + " WHERE PK1 = ? AND PK2 = ? AND VIEW_PK1 = ANY(?) AND (VIEW_PK2 IS NULL OR VIEW_PK2 = ANY(?))"; + Array viewPk1Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); + Array viewPk2Arr = conn.createArrayOf("VARCHAR", new String[] { "P", "Q" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setArray(3, viewPk1Arr); + stmt.setArray(4, viewPk2Arr); + try (ResultSet rs = stmt.executeQuery()) { + int rowCount = 0; + while (rs.next()) { + rowCount++; + String viewPk1 = rs.getString("VIEW_PK1"); + String viewPk2 = rs.getString("VIEW_PK2"); + // Verify VIEW_PK1 is either X or Y + assertTrue("X".equals(viewPk1) || "Y".equals(viewPk1)); + // Verify VIEW_PK2 is NULL, P, or Q + assertTrue(viewPk2 == null || "P".equals(viewPk2) || "Q".equals(viewPk2)); + } + // Expected rows: + // (A, BASE1, X, NULL), (A, BASE1, X, P), (A, BASE1, X, Q), + // (A, BASE1, Y, NULL), (A, BASE1, Y, Q) + assertEquals(5, rowCount); + } + // Assert point lookups are generated + // VIEW_PK1 has 2 values (X, Y), VIEW_PK2 has 3 values (NULL, P, Q) + // Total combinations: 2 * 3 = 6 point lookups + assertPointLookupsAreGenerated(stmt, 6); + } + } + } } From d3329116d0b9da474daed64e709eac5f63b672dc Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Tue, 17 Feb 2026 10:18:08 +0530 Subject: [PATCH 04/15] Separate IT class for nullable PK --- .../end2end/WhereOptimizerForArrayAnyIT.java | 580 +---------------- .../WhereOptimizerForArrayAnyITBase.java | 58 ++ ...WhereOptimizerForArrayAnyNullablePKIT.java | 595 ++++++++++++++++++ 3 files changed, 654 insertions(+), 579 deletions(-) create mode 100644 phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java create mode 100644 phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java index b89fcda2306..437861c2910 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java @@ -19,7 +19,6 @@ 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 java.math.BigDecimal; @@ -34,30 +33,21 @@ import java.sql.Time; import java.sql.Timestamp; import java.sql.Types; -import java.util.HashMap; import org.apache.hadoop.hbase.TableName; import org.apache.phoenix.compile.ExplainPlan; import org.apache.phoenix.compile.ExplainPlanAttributes; import org.apache.phoenix.compile.QueryPlan; import org.apache.phoenix.jdbc.PhoenixPreparedStatement; import org.apache.phoenix.jdbc.PhoenixStatement; -import org.apache.phoenix.query.BaseTest; import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.DateUtil; -import org.apache.phoenix.util.QueryUtil; -import org.apache.phoenix.util.ReadOnlyProps; import org.apache.phoenix.util.TestUtil; import org.bson.RawBsonDocument; -import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; @Category(NeedsOwnMiniClusterTest.class) -public class WhereOptimizerForArrayAnyIT extends BaseTest { - @BeforeClass - public static void setup() throws Exception { - setUpTestDriver(new ReadOnlyProps(new HashMap())); - } +public class WhereOptimizerForArrayAnyIT extends WhereOptimizerForArrayAnyITBase { @Test public void testArrayAnyComparisonForNonPkColumn() throws Exception { @@ -786,12 +776,6 @@ private void assertPointLookupsAreNotGenerated(PreparedStatement stmt) throws SQ assertEquals("FULL SCAN ", planAttributes.getExplainScanType()); } - private void assertPointLookupsAreGenerated(PreparedStatement stmt, int noOfPointLookups) - throws SQLException { - QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); - assertPointLookupsAreGenerated(queryPlan, noOfPointLookups); - } - private void assertPointLookupsAreGenerated(Statement stmt, String selectSql, int noOfPointLookups) throws SQLException { QueryPlan queryPlan = stmt.unwrap(PhoenixStatement.class).optimizeQuery(selectSql); @@ -808,25 +792,6 @@ private void assertSkipScanIsGenerated(PreparedStatement stmt, int skipListSize) assertEquals(expectedScanType, planAttributes.getExplainScanType()); } - private void assertPointLookupsAreGenerated(QueryPlan queryPlan, int noOfPointLookups) - throws SQLException { - ExplainPlan explain = queryPlan.getExplainPlan(); - ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); - String expectedScanType = - "POINT LOOKUP ON " + noOfPointLookups + " KEY" + (noOfPointLookups > 1 ? "S " : " "); - assertEquals(expectedScanType, planAttributes.getExplainScanType()); - } - - private void assertQueryUsesIndex(PreparedStatement stmt, String indexName) throws SQLException { - QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); - ExplainPlan explain = queryPlan.getExplainPlan(); - ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); - String tableName = planAttributes.getTableName(); - System.out.println("Explain plan: " + explain.toString()); - assertTrue("Expected query to use index " + indexName + " but used table " + tableName, - tableName != null && tableName.contains(indexName)); - } - private void createTableASCPkColumns(String tableName) throws SQLException { String ddl = "CREATE TABLE " + tableName + " (" + "pk1 INTEGER NOT NULL, " + "pk2 VARCHAR(3), " + "col1 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (pk1, pk2)" + ")"; @@ -899,547 +864,4 @@ private static RawBsonDocument getBsonDocument2() { + " \"attr_0\" : \"str_val_0\"\n" + "}"; return RawBsonDocument.parse(json); } - - private static final BigDecimal PK3_VAL = new BigDecimal("100.5"); - - /** - * Creates a table with 5 PK columns (last one nullable) and inserts test data. - * Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 DECIMAL, PK4 VARCHAR, PK5 DECIMAL (nullable) - * COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR - * Inserts 5 rows: - * Row 1: (A, B, 100.5, X, NULL, val1, val2, val3) - * Row 2: (A, B, 100.5, Y, NULL, val4, val5, val6) - * Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) - * Row 4: (A, B, 100.5, Z, NULL, val10, val11, val12) - * Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) - * @param pk5Desc if true, PK5 will have DESC sort order - * @return the generated table name - */ - private String createTableAndInsertTestDataForNullablePKTests(boolean pk4Desc, boolean pk5Desc) throws Exception { - String tableName = generateUniqueName(); - String pk4SortOrder = pk4Desc ? " DESC" : ""; - String pk5SortOrder = pk5Desc ? " DESC" : ""; - String ddl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR NOT NULL, " - + "PK2 VARCHAR NOT NULL, " - + "PK3 DECIMAL NOT NULL, " - + "PK4 VARCHAR NOT NULL, " - + "PK5 DECIMAL, " - + "COL1 VARCHAR, " - + "COL2 VARCHAR, " - + "COL3 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + pk4SortOrder + ", PK5" + pk5SortOrder + ")" - + ")"; - try (Connection conn = DriverManager.getConnection(getUrl())) { - try (Statement stmt = conn.createStatement()) { - stmt.execute(ddl); - conn.commit(); - } - } - - String upsertStmt = "UPSERT INTO " + tableName - + " (PK1, PK2, PK3, PK4, PK5, COL1, COL2, COL3) " - + "VALUES (?, ?, ?, ?, ?, ?, ?, ?)"; - try (Connection conn = DriverManager.getConnection(getUrl())) { - try (PreparedStatement stmt = conn.prepareStatement(upsertStmt)) { - // Row 1: PK5 is NULL - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "X"); - stmt.setNull(5, Types.DECIMAL); - stmt.setString(6, "val1"); - stmt.setString(7, "val2"); - stmt.setString(8, "val3"); - stmt.executeUpdate(); - - // Row 2: PK5 is NULL, different PK4 - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "Y"); - stmt.setNull(5, Types.DECIMAL); - stmt.setString(6, "val4"); - stmt.setString(7, "val5"); - stmt.setString(8, "val6"); - stmt.executeUpdate(); - - // Row 3: PK5 is NOT NULL - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "X"); - stmt.setBigDecimal(5, new BigDecimal("1.0")); - stmt.setString(6, "val7"); - stmt.setString(7, "val8"); - stmt.setString(8, "val9"); - stmt.executeUpdate(); - - // Row 4: PK5 is NULL, different PK4 - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "Z"); - stmt.setNull(5, Types.DECIMAL); - stmt.setString(6, "val10"); - stmt.setString(7, "val11"); - stmt.setString(8, "val12"); - stmt.executeUpdate(); - - // Row 5: Different PK1 - stmt.setString(1, "C"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "X"); - stmt.setNull(5, Types.DECIMAL); - stmt.setString(6, "val13"); - stmt.setString(7, "val14"); - stmt.setString(8, "val15"); - stmt.executeUpdate(); - - conn.commit(); - } - } - return tableName; - } - - @Test - public void testSinglePointLookupWithNullablePK() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); - - // Query with = for PK4 and IS NULL for PK5 - // IS NULL on trailing nullable PK column generates POINT LOOKUP because: - // - Trailing nulls are stripped when storing, so key for NULL matches stored key - // - The generated lookup key is exactly what's stored for rows with trailing NULL - try (Connection conn = DriverManager.getConnection(getUrl())) { - String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " - + "AND PK4 = ? AND PK5 IS NULL"; - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "X"); - try (ResultSet rs = stmt.executeQuery()) { - // Should return 1 row: PK4='X' with PK5 IS NULL - assertTrue(rs.next()); - assertEquals("X", rs.getString("PK4")); - assertEquals("val1", rs.getString("COL1")); - assertEquals("val2", rs.getString("COL2")); - assertEquals("val3", rs.getString("COL3")); - assertNull(rs.getBigDecimal("PK5")); - - // No more rows - assertFalse(rs.next()); - } - // IS NULL on trailing nullable PK column generates single POINT LOOKUP - assertPointLookupsAreGenerated(stmt, 1); - - stmt.setString(4, "Y"); - try (ResultSet rs = stmt.executeQuery()) { - // Should return 1 row: PK4='Y' with PK5 IS NULL - assertTrue(rs.next()); - assertEquals("Y", rs.getString("PK4")); - assertEquals("val4", rs.getString("COL1")); - assertEquals("val5", rs.getString("COL2")); - assertEquals("val6", rs.getString("COL3")); - assertNull(rs.getBigDecimal("PK5")); - } - // IS NULL on trailing nullable PK column generates single POINT LOOKUP - assertPointLookupsAreGenerated(stmt, 1); - } - } - } - - @Test - public void testMultiPointLookupsWithNullablePK() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); - - // Query with =ANY(?) for PK4 and IS NULL for PK5 - // IS NULL on trailing nullable PK column generates POINT LOOKUPS because: - // - Trailing nulls are stripped when storing, so key for NULL matches stored key - // - The generated lookup key is exactly what's stored for rows with trailing NULL - try (Connection conn = DriverManager.getConnection(getUrl())) { - String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; - Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setArray(4, pk4Arr); - try (ResultSet rs = stmt.executeQuery()) { - // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL - assertTrue(rs.next()); - String pk4Val = rs.getString("PK4"); - assertTrue("X".equals(pk4Val)); - assertNull(rs.getBytes("PK5")); - - assertTrue(rs.next()); - pk4Val = rs.getString("PK4"); - assertTrue("Y".equals(pk4Val)); - assertNull(rs.getBigDecimal("PK5")); - - // No more rows - assertFalse(rs.next()); - } - // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) - assertPointLookupsAreGenerated(stmt, 2); - } - } - } - - @Test - public void testSinglePointLookupWithNullablePKDesc() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); - - // Query with = for PK4 and IS NULL for PK5 (DESC sort order) - // IS NULL on trailing nullable PK column with DESC sort order generates POINT LOOKUP - try (Connection conn = DriverManager.getConnection(getUrl())) { - String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " - + "AND PK4 = ? AND PK5 IS NULL"; - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "X"); - try (ResultSet rs = stmt.executeQuery()) { - // Should return 1 row: PK4='X' with PK5 IS NULL - assertTrue(rs.next()); - assertEquals("X", rs.getString("PK4")); - assertEquals("val1", rs.getString("COL1")); - assertEquals("val2", rs.getString("COL2")); - assertEquals("val3", rs.getString("COL3")); - assertNull(rs.getBigDecimal("PK5")); - - // No more rows - assertFalse(rs.next()); - } - // IS NULL on trailing nullable PK column generates single POINT LOOKUP - assertPointLookupsAreGenerated(stmt, 1); - - stmt.setString(4, "Y"); - try (ResultSet rs = stmt.executeQuery()) { - // Should return 1 row: PK4='Y' with PK5 IS NULL - assertTrue(rs.next()); - assertEquals("Y", rs.getString("PK4")); - assertEquals("val4", rs.getString("COL1")); - assertEquals("val5", rs.getString("COL2")); - assertEquals("val6", rs.getString("COL3")); - assertNull(rs.getBigDecimal("PK5")); - } - // IS NULL on trailing nullable PK column generates single POINT LOOKUP - assertPointLookupsAreGenerated(stmt, 1); - } - } - } - - @Test - public void testMultiPointLookupsWithNullablePKDesc() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); - - // Query with =ANY(?) for PK4 and IS NULL for PK5 (DESC sort order) - // IS NULL on trailing nullable PK column with DESC generates POINT LOOKUPS - try (Connection conn = DriverManager.getConnection(getUrl())) { - String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; - Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setArray(4, pk4Arr); - try (ResultSet rs = stmt.executeQuery()) { - // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL - assertTrue(rs.next()); - String pk4Val = rs.getString("PK4"); - assertTrue("X".equals(pk4Val)); - assertNull(rs.getBytes("PK5")); - - assertTrue(rs.next()); - pk4Val = rs.getString("PK4"); - assertTrue("Y".equals(pk4Val)); - assertNull(rs.getBigDecimal("PK5")); - - // No more rows - assertFalse(rs.next()); - } - // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) - assertPointLookupsAreGenerated(stmt, 2); - } - } - } - - @Test - public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { - String tableName = generateUniqueName(); - String indexName = "IDX_" + generateUniqueName(); - - try (Connection conn = DriverManager.getConnection(getUrl())) { - // Step 1: Create table with one nullable PK column (PK3) at the end - String createTableDdl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR NOT NULL, " - + "PK2 VARCHAR NOT NULL, " - + "PK3 VARCHAR, " // Nullable PK column at end - + "COL1 VARCHAR, " - + "COL2 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3)" - + ")"; - conn.createStatement().execute(createTableDdl); - conn.commit(); - - // Step 2: Create a covered global index on the data table - String createIndexDdl = "CREATE INDEX " + indexName + " ON " + tableName - + " (COL1) INCLUDE (COL2)"; - conn.createStatement().execute(createIndexDdl); - conn.commit(); - - // Step 3: Insert initial data (before ALTER TABLE) - // Row 1: PK3 is NULL - String upsertSql = "UPSERT INTO " + tableName - + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; - try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setNull(3, Types.VARCHAR); - stmt.setString(4, "indexed_val1"); - stmt.setString(5, "col2_val1"); - stmt.executeUpdate(); - - // Row 2: PK3 has a value - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setString(3, "pk3_val1"); - stmt.setString(4, "indexed_val2"); - stmt.setString(5, "col2_val2"); - stmt.executeUpdate(); - - // Row 3: Different PK prefix - stmt.setString(1, "C"); - stmt.setString(2, "D"); - stmt.setNull(3, Types.VARCHAR); - stmt.setString(4, "indexed_val3"); - stmt.setString(5, "col2_val3"); - stmt.executeUpdate(); - } - conn.commit(); - - // Step 4: Add a new nullable PK column (PK4) via ALTER TABLE - String alterTableDdl = "ALTER TABLE " + tableName + " ADD PK4 VARCHAR PRIMARY KEY"; - conn.createStatement().execute(alterTableDdl); - conn.commit(); - - // Step 5: Insert more data with same PK prefix but different PK4 values - upsertSql = "UPSERT INTO " + tableName - + " (PK1, PK2, PK3, PK4, COL1, COL2) VALUES (?, ?, ?, ?, ?, ?)"; - try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { - // Row 4: Same prefix as Row 1 (A, B, NULL) but PK4 = 'X' - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setNull(3, Types.VARCHAR); - stmt.setString(4, "X"); - stmt.setString(5, "indexed_val4"); - stmt.setString(6, "col2_val4"); - stmt.executeUpdate(); - - // Row 5: Same prefix as Row 1 (A, B, NULL) but PK4 = 'Y' - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setNull(3, Types.VARCHAR); - stmt.setString(4, "Y"); - stmt.setString(5, "indexed_val5"); - stmt.setString(6, "col2_val5"); - stmt.executeUpdate(); - - // Row 6: Same prefix as Row 2 (A, B, pk3_val1) but PK4 = 'Y' - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setString(3, "pk3_val1"); - stmt.setString(4, "Y"); - stmt.setString(5, "indexed_val6"); - stmt.setString(6, "col2_val6"); - stmt.executeUpdate(); - - // Row 7: Same prefix as Row 2 (A, B, NULL) but PK4 = 'Z' - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setNull(3, Types.VARCHAR); - stmt.setString(4, "Z"); - stmt.setString(5, "indexed_val7"); - stmt.setString(6, "col2_val7"); - stmt.executeUpdate(); - } - conn.commit(); - - String selectSql = "SELECT /*+ INDEX(" + tableName + " " + indexName + ") */ " - + "PK1, PK2, PK3, PK4, COL1, COL2 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 IS NULL AND (PK4 IS NULL OR PK4 = ANY(?)) AND COL1 = ANY(?)"; - Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "Z", "Y" }); - Array col1Arr = conn.createArrayOf("VARCHAR", new String[] { "indexed_val5", "indexed_val1", "indexed_val7", "indexed_val4" }); - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setArray(3, pk4Arr); - stmt.setArray(4, col1Arr); - try (ResultSet rs = stmt.executeQuery()) { - assertTrue(rs.next()); - String pk4Val = rs.getString("PK4"); - assertNull(pk4Val); - - assertTrue(rs.next()); - pk4Val = rs.getString("PK4"); - assertTrue("Y".equals(pk4Val)); - - assertTrue(rs.next()); - pk4Val = rs.getString("PK4"); - assertTrue("Z".equals(pk4Val)); - - // No more rows - assertFalse(rs.next()); - } - // Should generate point lookups for the three PK4 values - assertPointLookupsAreGenerated(stmt, 12); - // Assert that the query uses the index table - assertQueryUsesIndex(stmt, indexName); - } - } - } - - @Test - public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception { - String tableName = generateUniqueName(); - String viewName = "VW_" + generateUniqueName(); - - try (Connection conn = DriverManager.getConnection(getUrl())) { - // Step 1: Create parent table with fixed-width NOT NULL last PK - // Using CHAR (fixed-width) for PK2 to allow view to add PK columns - String createTableDdl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR NOT NULL, " - + "PK2 CHAR(10) NOT NULL, " // Fixed-width NOT NULL - allows view to add PKs - + "COL1 VARCHAR, " - + "COL2 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2)" - + ")"; - conn.createStatement().execute(createTableDdl); - conn.commit(); - - // Step 2: Create view that adds two nullable PK columns - String createViewDdl = "CREATE VIEW " + viewName + " (" - + "VIEW_PK1 VARCHAR, " // Nullable PK column added by view - + "VIEW_PK2 VARCHAR, " // Second nullable PK column added by view - + "VIEW_COL1 VARCHAR, " - + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1, VIEW_PK2)" - + ") AS SELECT * FROM " + tableName; - conn.createStatement().execute(createViewDdl); - conn.commit(); - - // Step 3: Insert data through the view with various combinations - String upsertSql = "UPSERT INTO " + viewName - + " (PK1, PK2, VIEW_PK1, VIEW_PK2, COL1, COL2, VIEW_COL1) VALUES (?, ?, ?, ?, ?, ?, ?)"; - try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { - // Row 1: Both VIEW_PK1 and VIEW_PK2 are NULL - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setNull(3, Types.VARCHAR); - stmt.setNull(4, Types.VARCHAR); - stmt.setString(5, "col1_val1"); - stmt.setString(6, "col2_val1"); - stmt.setString(7, "view_col1_val1"); - stmt.executeUpdate(); - - // Row 2: VIEW_PK1 = 'X', VIEW_PK2 is NULL - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setString(3, "X"); - stmt.setNull(4, Types.VARCHAR); - stmt.setString(5, "col1_val2"); - stmt.setString(6, "col2_val2"); - stmt.setString(7, "view_col1_val2"); - stmt.executeUpdate(); - - // Row 3: VIEW_PK1 = 'X', VIEW_PK2 = 'P' - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setString(3, "X"); - stmt.setString(4, "P"); - stmt.setString(5, "col1_val3"); - stmt.setString(6, "col2_val3"); - stmt.setString(7, "view_col1_val3"); - stmt.executeUpdate(); - - // Row 4: VIEW_PK1 = 'X', VIEW_PK2 = 'Q' - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setString(3, "X"); - stmt.setString(4, "Q"); - stmt.setString(5, "col1_val4"); - stmt.setString(6, "col2_val4"); - stmt.setString(7, "view_col1_val4"); - stmt.executeUpdate(); - - // Row 5: VIEW_PK1 = 'Y', VIEW_PK2 is NULL - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setString(3, "Y"); - stmt.setNull(4, Types.VARCHAR); - stmt.setString(5, "col1_val5"); - stmt.setString(6, "col2_val5"); - stmt.setString(7, "view_col1_val5"); - stmt.executeUpdate(); - - // Row 6: VIEW_PK1 = 'Y', VIEW_PK2 = 'Q' - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setString(3, "Y"); - stmt.setString(4, "Q"); - stmt.setString(5, "col1_val6"); - stmt.setString(6, "col2_val6"); - stmt.setString(7, "view_col1_val6"); - stmt.executeUpdate(); - - // Row 7: Different base PK prefix - stmt.setString(1, "B"); - stmt.setString(2, "BASE2"); - stmt.setString(3, "X"); - stmt.setString(4, "P"); - stmt.setString(5, "col1_val7"); - stmt.setString(6, "col2_val7"); - stmt.setString(7, "view_col1_val7"); - stmt.executeUpdate(); - } - conn.commit(); - - String selectSql = "SELECT PK1, PK2, VIEW_PK1, VIEW_PK2, COL1, VIEW_COL1 FROM " + viewName - + " WHERE PK1 = ? AND PK2 = ? AND VIEW_PK1 = ANY(?) AND (VIEW_PK2 IS NULL OR VIEW_PK2 = ANY(?))"; - Array viewPk1Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); - Array viewPk2Arr = conn.createArrayOf("VARCHAR", new String[] { "P", "Q" }); - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setArray(3, viewPk1Arr); - stmt.setArray(4, viewPk2Arr); - try (ResultSet rs = stmt.executeQuery()) { - int rowCount = 0; - while (rs.next()) { - rowCount++; - String viewPk1 = rs.getString("VIEW_PK1"); - String viewPk2 = rs.getString("VIEW_PK2"); - // Verify VIEW_PK1 is either X or Y - assertTrue("X".equals(viewPk1) || "Y".equals(viewPk1)); - // Verify VIEW_PK2 is NULL, P, or Q - assertTrue(viewPk2 == null || "P".equals(viewPk2) || "Q".equals(viewPk2)); - } - // Expected rows: - // (A, BASE1, X, NULL), (A, BASE1, X, P), (A, BASE1, X, Q), - // (A, BASE1, Y, NULL), (A, BASE1, Y, Q) - assertEquals(5, rowCount); - } - // Assert point lookups are generated - // VIEW_PK1 has 2 values (X, Y), VIEW_PK2 has 3 values (NULL, P, Q) - // Total combinations: 2 * 3 = 6 point lookups - assertPointLookupsAreGenerated(stmt, 6); - } - } - } } diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java new file mode 100644 index 00000000000..f090a2399e3 --- /dev/null +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.phoenix.end2end; + +import static org.junit.Assert.assertEquals; + +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.HashMap; + +import org.apache.phoenix.compile.ExplainPlan; +import org.apache.phoenix.compile.ExplainPlanAttributes; +import org.apache.phoenix.compile.QueryPlan; +import org.apache.phoenix.jdbc.PhoenixPreparedStatement; +import org.apache.phoenix.query.BaseTest; +import org.apache.phoenix.util.ReadOnlyProps; +import org.junit.BeforeClass; + +/** + * Base class for WhereOptimizerForArrayAnyIT tests containing shared helper methods. + */ +public abstract class WhereOptimizerForArrayAnyITBase extends BaseTest { + + @BeforeClass + public static void setup() throws Exception { + setUpTestDriver(new ReadOnlyProps(new HashMap())); + } + + protected void assertPointLookupsAreGenerated(PreparedStatement stmt, int noOfPointLookups) + throws SQLException { + QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); + assertPointLookupsAreGenerated(queryPlan, noOfPointLookups); + } + + protected void assertPointLookupsAreGenerated(QueryPlan queryPlan, int noOfPointLookups) + throws SQLException { + ExplainPlan explain = queryPlan.getExplainPlan(); + ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); + String expectedScanType = + "POINT LOOKUP ON " + noOfPointLookups + " KEY" + (noOfPointLookups > 1 ? "S " : " "); + assertEquals(expectedScanType, planAttributes.getExplainScanType()); + } +} diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java new file mode 100644 index 00000000000..5e4b9199820 --- /dev/null +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -0,0 +1,595 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.phoenix.end2end; + +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 java.math.BigDecimal; +import java.sql.Array; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.sql.Types; + +import org.apache.phoenix.compile.ExplainPlan; +import org.apache.phoenix.compile.ExplainPlanAttributes; +import org.apache.phoenix.compile.QueryPlan; +import org.apache.phoenix.jdbc.PhoenixPreparedStatement; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category(NeedsOwnMiniClusterTest.class) +public class WhereOptimizerForArrayAnyNullablePKIT extends WhereOptimizerForArrayAnyITBase { + + private static final BigDecimal PK3_VAL = new BigDecimal("100.5"); + + /** + * Creates a table with 5 PK columns (last one nullable) and inserts test data. + * Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 DECIMAL, PK4 VARCHAR, PK5 DECIMAL (nullable) + * COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR + * Inserts 5 rows: + * Row 1: (A, B, 100.5, X, NULL, val1, val2, val3) + * Row 2: (A, B, 100.5, Y, NULL, val4, val5, val6) + * Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) + * Row 4: (A, B, 100.5, Z, NULL, val10, val11, val12) + * Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) + * @param pk5Desc if true, PK5 will have DESC sort order + * @return the generated table name + */ + private String createTableAndInsertTestDataForNullablePKTests(boolean pk4Desc, boolean pk5Desc) throws Exception { + String tableName = generateUniqueName(); + String pk4SortOrder = pk4Desc ? " DESC" : ""; + String pk5SortOrder = pk5Desc ? " DESC" : ""; + String ddl = "CREATE TABLE " + tableName + " (" + + "PK1 VARCHAR NOT NULL, " + + "PK2 VARCHAR NOT NULL, " + + "PK3 DECIMAL NOT NULL, " + + "PK4 VARCHAR NOT NULL, " + + "PK5 DECIMAL, " + + "COL1 VARCHAR, " + + "COL2 VARCHAR, " + + "COL3 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + pk4SortOrder + ", PK5" + pk5SortOrder + ")" + + ")"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (java.sql.Statement stmt = conn.createStatement()) { + stmt.execute(ddl); + conn.commit(); + } + } + + String upsertStmt = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, PK4, PK5, COL1, COL2, COL3) " + + "VALUES (?, ?, ?, ?, ?, ?, ?, ?)"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (PreparedStatement stmt = conn.prepareStatement(upsertStmt)) { + // Row 1: PK5 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val1"); + stmt.setString(7, "val2"); + stmt.setString(8, "val3"); + stmt.executeUpdate(); + + // Row 2: PK5 is NULL, different PK4 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Y"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val4"); + stmt.setString(7, "val5"); + stmt.setString(8, "val6"); + stmt.executeUpdate(); + + // Row 3: PK5 is NOT NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setBigDecimal(5, new BigDecimal("1.0")); + stmt.setString(6, "val7"); + stmt.setString(7, "val8"); + stmt.setString(8, "val9"); + stmt.executeUpdate(); + + // Row 4: PK5 is NULL, different PK4 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Z"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val10"); + stmt.setString(7, "val11"); + stmt.setString(8, "val12"); + stmt.executeUpdate(); + + // Row 5: Different PK1 + stmt.setString(1, "C"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val13"); + stmt.setString(7, "val14"); + stmt.setString(8, "val15"); + stmt.executeUpdate(); + + conn.commit(); + } + } + return tableName; + } + + private void assertQueryUsesIndex(PreparedStatement stmt, String indexName) throws SQLException { + QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); + ExplainPlan explain = queryPlan.getExplainPlan(); + ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); + String tableName = planAttributes.getTableName(); + System.out.println("Explain plan: " + explain.toString()); + assertTrue("Expected query to use index " + indexName + " but used table " + tableName, + tableName != null && tableName.contains(indexName)); + } + + @Test + public void testSinglePointLookupWithNullablePK() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); + + // Query with = for PK4 and IS NULL for PK5 + // IS NULL on trailing nullable PK column generates POINT LOOKUP because: + // - Trailing nulls are stripped when storing, so key for NULL matches stored key + // - The generated lookup key is exactly what's stored for rows with trailing NULL + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " + + "AND PK4 = ? AND PK5 IS NULL"; + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='X' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("X", rs.getString("PK4")); + assertEquals("val1", rs.getString("COL1")); + assertEquals("val2", rs.getString("COL2")); + assertEquals("val3", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + + stmt.setString(4, "Y"); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("Y", rs.getString("PK4")); + assertEquals("val4", rs.getString("COL1")); + assertEquals("val5", rs.getString("COL2")); + assertEquals("val6", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + } + } + } + + @Test + public void testMultiPointLookupsWithNullablePK() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); + + // Query with =ANY(?) for PK4 and IS NULL for PK5 + // IS NULL on trailing nullable PK column generates POINT LOOKUPS because: + // - Trailing nulls are stripped when storing, so key for NULL matches stored key + // - The generated lookup key is exactly what's stored for rows with trailing NULL + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; + Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setArray(4, pk4Arr); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + String pk4Val = rs.getString("PK4"); + assertTrue("X".equals(pk4Val)); + assertNull(rs.getBytes("PK5")); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Y".equals(pk4Val)); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) + assertPointLookupsAreGenerated(stmt, 2); + } + } + } + + @Test + public void testSinglePointLookupWithNullablePKDesc() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); + + // Query with = for PK4 and IS NULL for PK5 (DESC sort order) + // IS NULL on trailing nullable PK column with DESC sort order generates POINT LOOKUP + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " + + "AND PK4 = ? AND PK5 IS NULL"; + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='X' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("X", rs.getString("PK4")); + assertEquals("val1", rs.getString("COL1")); + assertEquals("val2", rs.getString("COL2")); + assertEquals("val3", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + + stmt.setString(4, "Y"); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + assertEquals("Y", rs.getString("PK4")); + assertEquals("val4", rs.getString("COL1")); + assertEquals("val5", rs.getString("COL2")); + assertEquals("val6", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + } + // IS NULL on trailing nullable PK column generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + } + } + } + + @Test + public void testMultiPointLookupsWithNullablePKDesc() throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); + + // Query with =ANY(?) for PK4 and IS NULL for PK5 (DESC sort order) + // IS NULL on trailing nullable PK column with DESC generates POINT LOOKUPS + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; + Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setArray(4, pk4Arr); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL + assertTrue(rs.next()); + String pk4Val = rs.getString("PK4"); + assertTrue("X".equals(pk4Val)); + assertNull(rs.getBytes("PK5")); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Y".equals(pk4Val)); + assertNull(rs.getBigDecimal("PK5")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) + assertPointLookupsAreGenerated(stmt, 2); + } + } + } + + @Test + public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { + String tableName = generateUniqueName(); + String indexName = "IDX_" + generateUniqueName(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + // Step 1: Create table with one nullable PK column (PK3) at the end + String createTableDdl = "CREATE TABLE " + tableName + " (" + + "PK1 VARCHAR NOT NULL, " + + "PK2 VARCHAR NOT NULL, " + + "PK3 VARCHAR, " // Nullable PK column at end + + "COL1 VARCHAR, " + + "COL2 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3)" + + ")"; + conn.createStatement().execute(createTableDdl); + conn.commit(); + + // Step 2: Create a covered global index on the data table + String createIndexDdl = "CREATE INDEX " + indexName + " ON " + tableName + + " (COL1) INCLUDE (COL2)"; + conn.createStatement().execute(createIndexDdl); + conn.commit(); + + // Step 3: Insert initial data (before ALTER TABLE) + // Row 1: PK3 is NULL + String upsertSql = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; + try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "indexed_val1"); + stmt.setString(5, "col2_val1"); + stmt.executeUpdate(); + + // Row 2: PK3 has a value + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setString(3, "pk3_val1"); + stmt.setString(4, "indexed_val2"); + stmt.setString(5, "col2_val2"); + stmt.executeUpdate(); + + // Row 3: Different PK prefix + stmt.setString(1, "C"); + stmt.setString(2, "D"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "indexed_val3"); + stmt.setString(5, "col2_val3"); + stmt.executeUpdate(); + } + conn.commit(); + + // Step 4: Add a new nullable PK column (PK4) via ALTER TABLE + String alterTableDdl = "ALTER TABLE " + tableName + " ADD PK4 VARCHAR PRIMARY KEY"; + conn.createStatement().execute(alterTableDdl); + conn.commit(); + + // Step 5: Insert more data with same PK prefix but different PK4 values + upsertSql = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, PK4, COL1, COL2) VALUES (?, ?, ?, ?, ?, ?)"; + try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { + // Row 4: Same prefix as Row 1 (A, B, NULL) but PK4 = 'X' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "X"); + stmt.setString(5, "indexed_val4"); + stmt.setString(6, "col2_val4"); + stmt.executeUpdate(); + + // Row 5: Same prefix as Row 1 (A, B, NULL) but PK4 = 'Y' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "Y"); + stmt.setString(5, "indexed_val5"); + stmt.setString(6, "col2_val5"); + stmt.executeUpdate(); + + // Row 6: Same prefix as Row 2 (A, B, pk3_val1) but PK4 = 'Y' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setString(3, "pk3_val1"); + stmt.setString(4, "Y"); + stmt.setString(5, "indexed_val6"); + stmt.setString(6, "col2_val6"); + stmt.executeUpdate(); + + // Row 7: Same prefix as Row 2 (A, B, NULL) but PK4 = 'Z' + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.VARCHAR); + stmt.setString(4, "Z"); + stmt.setString(5, "indexed_val7"); + stmt.setString(6, "col2_val7"); + stmt.executeUpdate(); + } + conn.commit(); + + String selectSql = "SELECT /*+ INDEX(" + tableName + " " + indexName + ") */ " + + "PK1, PK2, PK3, PK4, COL1, COL2 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 IS NULL AND (PK4 IS NULL OR PK4 = ANY(?)) AND COL1 = ANY(?)"; + Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "Z", "Y" }); + Array col1Arr = conn.createArrayOf("VARCHAR", new String[] { "indexed_val5", "indexed_val1", "indexed_val7", "indexed_val4" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setArray(3, pk4Arr); + stmt.setArray(4, col1Arr); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + assertTrue(rs.next()); + String pk4Val = rs.getString("PK4"); + assertNull(pk4Val); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Y".equals(pk4Val)); + + assertTrue(rs.next()); + pk4Val = rs.getString("PK4"); + assertTrue("Z".equals(pk4Val)); + + // No more rows + assertFalse(rs.next()); + } + // Should generate point lookups for the three PK4 values + assertPointLookupsAreGenerated(stmt, 12); + // Assert that the query uses the index table + assertQueryUsesIndex(stmt, indexName); + } + } + } + + @Test + public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception { + String tableName = generateUniqueName(); + String viewName = "VW_" + generateUniqueName(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + // Step 1: Create parent table with fixed-width NOT NULL last PK + // Using CHAR (fixed-width) for PK2 to allow view to add PK columns + String createTableDdl = "CREATE TABLE " + tableName + " (" + + "PK1 VARCHAR NOT NULL, " + + "PK2 CHAR(10) NOT NULL, " // Fixed-width NOT NULL - allows view to add PKs + + "COL1 VARCHAR, " + + "COL2 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2)" + + ")"; + conn.createStatement().execute(createTableDdl); + conn.commit(); + + // Step 2: Create view that adds two nullable PK columns + String createViewDdl = "CREATE VIEW " + viewName + " (" + + "VIEW_PK1 VARCHAR, " // Nullable PK column added by view + + "VIEW_PK2 VARCHAR, " // Second nullable PK column added by view + + "VIEW_COL1 VARCHAR, " + + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1, VIEW_PK2)" + + ") AS SELECT * FROM " + tableName; + conn.createStatement().execute(createViewDdl); + conn.commit(); + + // Step 3: Insert data through the view with various combinations + String upsertSql = "UPSERT INTO " + viewName + + " (PK1, PK2, VIEW_PK1, VIEW_PK2, COL1, COL2, VIEW_COL1) VALUES (?, ?, ?, ?, ?, ?, ?)"; + try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { + // Row 1: Both VIEW_PK1 and VIEW_PK2 are NULL + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setNull(3, Types.VARCHAR); + stmt.setNull(4, Types.VARCHAR); + stmt.setString(5, "col1_val1"); + stmt.setString(6, "col2_val1"); + stmt.setString(7, "view_col1_val1"); + stmt.executeUpdate(); + + // Row 2: VIEW_PK1 = 'X', VIEW_PK2 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "X"); + stmt.setNull(4, Types.VARCHAR); + stmt.setString(5, "col1_val2"); + stmt.setString(6, "col2_val2"); + stmt.setString(7, "view_col1_val2"); + stmt.executeUpdate(); + + // Row 3: VIEW_PK1 = 'X', VIEW_PK2 = 'P' + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "X"); + stmt.setString(4, "P"); + stmt.setString(5, "col1_val3"); + stmt.setString(6, "col2_val3"); + stmt.setString(7, "view_col1_val3"); + stmt.executeUpdate(); + + // Row 4: VIEW_PK1 = 'X', VIEW_PK2 = 'Q' + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "X"); + stmt.setString(4, "Q"); + stmt.setString(5, "col1_val4"); + stmt.setString(6, "col2_val4"); + stmt.setString(7, "view_col1_val4"); + stmt.executeUpdate(); + + // Row 5: VIEW_PK1 = 'Y', VIEW_PK2 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "Y"); + stmt.setNull(4, Types.VARCHAR); + stmt.setString(5, "col1_val5"); + stmt.setString(6, "col2_val5"); + stmt.setString(7, "view_col1_val5"); + stmt.executeUpdate(); + + // Row 6: VIEW_PK1 = 'Y', VIEW_PK2 = 'Q' + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setString(3, "Y"); + stmt.setString(4, "Q"); + stmt.setString(5, "col1_val6"); + stmt.setString(6, "col2_val6"); + stmt.setString(7, "view_col1_val6"); + stmt.executeUpdate(); + + // Row 7: Different base PK prefix + stmt.setString(1, "B"); + stmt.setString(2, "BASE2"); + stmt.setString(3, "X"); + stmt.setString(4, "P"); + stmt.setString(5, "col1_val7"); + stmt.setString(6, "col2_val7"); + stmt.setString(7, "view_col1_val7"); + stmt.executeUpdate(); + } + conn.commit(); + + String selectSql = "SELECT PK1, PK2, VIEW_PK1, VIEW_PK2, COL1, VIEW_COL1 FROM " + viewName + + " WHERE PK1 = ? AND PK2 = ? AND VIEW_PK1 = ANY(?) AND (VIEW_PK2 IS NULL OR VIEW_PK2 = ANY(?))"; + Array viewPk1Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); + Array viewPk2Arr = conn.createArrayOf("VARCHAR", new String[] { "P", "Q" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "BASE1"); + stmt.setArray(3, viewPk1Arr); + stmt.setArray(4, viewPk2Arr); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + int rowCount = 0; + while (rs.next()) { + rowCount++; + String viewPk1 = rs.getString("VIEW_PK1"); + String viewPk2 = rs.getString("VIEW_PK2"); + // Verify VIEW_PK1 is either X or Y + assertTrue("X".equals(viewPk1) || "Y".equals(viewPk1)); + // Verify VIEW_PK2 is NULL, P, or Q + assertTrue(viewPk2 == null || "P".equals(viewPk2) || "Q".equals(viewPk2)); + } + // Expected rows: + // (A, BASE1, X, NULL), (A, BASE1, X, P), (A, BASE1, X, Q), + // (A, BASE1, Y, NULL), (A, BASE1, Y, Q) + assertEquals(5, rowCount); + } + // Assert point lookups are generated + // VIEW_PK1 has 2 values (X, Y), VIEW_PK2 has 3 values (NULL, P, Q) + // Total combinations: 2 * 3 = 6 point lookups + assertPointLookupsAreGenerated(stmt, 6); + } + } + } +} From 08299a816cad97f70798baa3d2db387c63e9fc99 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Tue, 17 Feb 2026 14:07:12 +0530 Subject: [PATCH 05/15] Add test coverage --- .../apache/phoenix/compile/ScanRanges.java | 10 +- .../org/apache/phoenix/util/ScanUtil.java | 2 +- ...WhereOptimizerForArrayAnyNullablePKIT.java | 391 +++++++++++++----- 3 files changed, 281 insertions(+), 122 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index 9852597e652..eae29c0c003 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -654,18 +654,9 @@ private static List getPointKeys(List> ranges, int[] slot boolean isSalted = bucketNum != null; int count = 1; int offset = isSalted ? 1 : 0; - int lastNonNullIndex = offset - 1; // Skip salt byte range in the first position if salted for (int i = offset; i < ranges.size(); i++) { count *= ranges.get(i).size(); - if (ranges.get(i).size() == 1 && ranges.get(i).get(0) == KeyRange.IS_NULL_RANGE) { - continue; - } - lastNonNullIndex = i; - } - if (lastNonNullIndex < ranges.size() - 1) { - // Create ranges without trailing null - ranges = ranges.subList(0, lastNonNullIndex + 1); } List keys = Lists.newArrayListWithExpectedSize(count); int[] position = new int[ranges.size()]; @@ -678,6 +669,7 @@ private static List getPointKeys(List> ranges, int[] slot if (isSalted) { key[0] = SaltingUtil.getSaltingByte(key, offset, length, bucketNum); } + keys.add(Arrays.copyOf(key, length + offset)); } while (incrementKey(ranges, position)); return keys; diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index d668af16b42..0711f3fb2ef 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -609,7 +609,7 @@ public static int setKey(RowKeySchema schema, List> slots, int[] while ( --i >= schemaStartIndex && offset > byteOffset && !(field = schema.getField(--fieldIndex)).getDataType().isFixedWidth() - && field.getSortOrder() == SortOrder.ASC && hasSeparatorBytes(key, field, offset) + && hasSeparatorBytes(key, field, offset) ) { if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { offset--; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index 5e4b9199820..91893f46c9a 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -42,9 +43,42 @@ public class WhereOptimizerForArrayAnyNullablePKIT extends WhereOptimizerForArra private static final BigDecimal PK3_VAL = new BigDecimal("100.5"); + /** + * Configuration for PK4 column type and sort order. + */ + private enum Pk4Config { + /** Fixed-width CHAR(1) NOT NULL */ + FIXED_WIDTH_CHAR("CHAR(1) NOT NULL", ""), + /** Nullable VARCHAR with ASC sort order (default) */ + NULLABLE_ASC("VARCHAR NOT NULL", ""), + /** Nullable VARCHAR with DESC sort order */ + NULLABLE_DESC("VARCHAR NOT NULL", " DESC"); + + private final String dataType; + private final String sortOrder; + + Pk4Config(String dataType, String sortOrder) { + this.dataType = dataType; + this.sortOrder = sortOrder; + } + + public String getDataType() { + return dataType; + } + + public String getSortOrder() { + return sortOrder; + } + + @Override + public String toString() { + return name() + "[" + dataType + (sortOrder.isEmpty() ? "" : sortOrder) + "]"; + } + } + /** * Creates a table with 5 PK columns (last one nullable) and inserts test data. - * Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 DECIMAL, PK4 VARCHAR, PK5 DECIMAL (nullable) + * Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 DECIMAL, PK4 (configurable), PK5 DECIMAL (nullable) * COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR * Inserts 5 rows: * Row 1: (A, B, 100.5, X, NULL, val1, val2, val3) @@ -52,23 +86,23 @@ public class WhereOptimizerForArrayAnyNullablePKIT extends WhereOptimizerForArra * Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) * Row 4: (A, B, 100.5, Z, NULL, val10, val11, val12) * Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) + * @param pk4Config configuration for PK4 column type and sort order * @param pk5Desc if true, PK5 will have DESC sort order * @return the generated table name */ - private String createTableAndInsertTestDataForNullablePKTests(boolean pk4Desc, boolean pk5Desc) throws Exception { + private String createTableAndInsertTestDataForNullablePKTests(Pk4Config pk4Config, boolean pk5Desc) throws Exception { String tableName = generateUniqueName(); - String pk4SortOrder = pk4Desc ? " DESC" : ""; String pk5SortOrder = pk5Desc ? " DESC" : ""; String ddl = "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + "PK2 VARCHAR NOT NULL, " + "PK3 DECIMAL NOT NULL, " - + "PK4 VARCHAR NOT NULL, " + + "PK4 " + pk4Config.getDataType() + ", " + "PK5 DECIMAL, " + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "COL3 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + pk4SortOrder + ", PK5" + pk5SortOrder + ")" + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + pk4Config.getSortOrder() + ", PK5" + pk5SortOrder + ")" + ")"; try (Connection conn = DriverManager.getConnection(getUrl())) { try (java.sql.Statement stmt = conn.createStatement()) { @@ -153,9 +187,30 @@ private void assertQueryUsesIndex(PreparedStatement stmt, String indexName) thro tableName != null && tableName.contains(indexName)); } + /** + * Parameterized test for single point lookup with nullable PK. + * Tests all combinations of: + * - PK4 config: FIXED_WIDTH_CHAR, NULLABLE_ASC, NULLABLE_DESC + * - PK5 sort order: ASC, DESC + */ @Test public void testSinglePointLookupWithNullablePK() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); + for (Pk4Config pk4Config : Pk4Config.values()) { + for (boolean pk5Desc : new boolean[] { false, true }) { + String configDesc = "PK4=" + pk4Config + ", PK5 DESC=" + pk5Desc; + try { + doTestSinglePointLookupWithNullablePK(pk4Config, pk5Desc); + } catch (AssertionError e) { + throw new AssertionError("Failed for configuration: " + configDesc, e); + } catch (Exception e) { + throw new Exception("Failed for configuration: " + configDesc, e); + } + } + } + } + + private void doTestSinglePointLookupWithNullablePK(Pk4Config pk4Config, boolean pk5Desc) throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(pk4Config, pk5Desc); // Query with = for PK4 and IS NULL for PK5 // IS NULL on trailing nullable PK column generates POINT LOOKUP because: @@ -201,9 +256,30 @@ public void testSinglePointLookupWithNullablePK() throws Exception { } } + /** + * Parameterized test for multi-point lookups with nullable PK. + * Tests all combinations of: + * - PK4 config: FIXED_WIDTH_CHAR, NULLABLE_ASC, NULLABLE_DESC + * - PK5 sort order: ASC, DESC + */ @Test public void testMultiPointLookupsWithNullablePK() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, false); + for (Pk4Config pk4Config : Pk4Config.values()) { + for (boolean pk5Desc : new boolean[] { false, true }) { + String configDesc = "PK4=" + pk4Config + ", PK5 DESC=" + pk5Desc; + try { + doTestMultiPointLookupsWithNullablePK(pk4Config, pk5Desc); + } catch (AssertionError e) { + throw new AssertionError("Failed for configuration: " + configDesc, e); + } catch (Exception e) { + throw new Exception("Failed for configuration: " + configDesc, e); + } + } + } + } + + private void doTestMultiPointLookupsWithNullablePK(Pk4Config pk4Config, boolean pk5Desc) throws Exception { + String tableName = createTableAndInsertTestDataForNullablePKTests(pk4Config, pk5Desc); // Query with =ANY(?) for PK4 and IS NULL for PK5 // IS NULL on trailing nullable PK column generates POINT LOOKUPS because: @@ -221,17 +297,18 @@ public void testMultiPointLookupsWithNullablePK() throws Exception { try (java.sql.ResultSet rs = stmt.executeQuery()) { // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL assertTrue(rs.next()); - String pk4Val = rs.getString("PK4"); - assertTrue("X".equals(pk4Val)); + String pk4Val1 = rs.getString("PK4"); + assertTrue("Y".equals(pk4Val1) || "X".equals(pk4Val1)); assertNull(rs.getBytes("PK5")); assertTrue(rs.next()); - pk4Val = rs.getString("PK4"); - assertTrue("Y".equals(pk4Val)); + String pk4Val2 = rs.getString("PK4"); + assertTrue("X".equals(pk4Val2) || "Y".equals(pk4Val2)); assertNull(rs.getBigDecimal("PK5")); // No more rows assertFalse(rs.next()); + assertNotEquals(pk4Val1, pk4Val2); } // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) assertPointLookupsAreGenerated(stmt, 2); @@ -239,93 +316,35 @@ public void testMultiPointLookupsWithNullablePK() throws Exception { } } + /** + * Parameterized test for query with index after adding nullable PK column. + * Tests all combinations of: + * - PK3 sort order: ASC, DESC + * - PK4 sort order: ASC, DESC + */ @Test - public void testSinglePointLookupWithNullablePKDesc() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); - - // Query with = for PK4 and IS NULL for PK5 (DESC sort order) - // IS NULL on trailing nullable PK column with DESC sort order generates POINT LOOKUP - try (Connection conn = DriverManager.getConnection(getUrl())) { - String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " - + "AND PK4 = ? AND PK5 IS NULL"; - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setString(4, "X"); - try (java.sql.ResultSet rs = stmt.executeQuery()) { - // Should return 1 row: PK4='X' with PK5 IS NULL - assertTrue(rs.next()); - assertEquals("X", rs.getString("PK4")); - assertEquals("val1", rs.getString("COL1")); - assertEquals("val2", rs.getString("COL2")); - assertEquals("val3", rs.getString("COL3")); - assertNull(rs.getBigDecimal("PK5")); - - // No more rows - assertFalse(rs.next()); - } - // IS NULL on trailing nullable PK column generates single POINT LOOKUP - assertPointLookupsAreGenerated(stmt, 1); - - stmt.setString(4, "Y"); - try (java.sql.ResultSet rs = stmt.executeQuery()) { - // Should return 1 row: PK4='Y' with PK5 IS NULL - assertTrue(rs.next()); - assertEquals("Y", rs.getString("PK4")); - assertEquals("val4", rs.getString("COL1")); - assertEquals("val5", rs.getString("COL2")); - assertEquals("val6", rs.getString("COL3")); - assertNull(rs.getBigDecimal("PK5")); - } - // IS NULL on trailing nullable PK column generates single POINT LOOKUP - assertPointLookupsAreGenerated(stmt, 1); - } - } - } - - @Test - public void testMultiPointLookupsWithNullablePKDesc() throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(false, true); - - // Query with =ANY(?) for PK4 and IS NULL for PK5 (DESC sort order) - // IS NULL on trailing nullable PK column with DESC generates POINT LOOKUPS - try (Connection conn = DriverManager.getConnection(getUrl())) { - String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; - Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); - try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - stmt.setString(1, "A"); - stmt.setString(2, "B"); - stmt.setBigDecimal(3, PK3_VAL); - stmt.setArray(4, pk4Arr); - try (java.sql.ResultSet rs = stmt.executeQuery()) { - // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL - assertTrue(rs.next()); - String pk4Val = rs.getString("PK4"); - assertTrue("X".equals(pk4Val)); - assertNull(rs.getBytes("PK5")); - - assertTrue(rs.next()); - pk4Val = rs.getString("PK4"); - assertTrue("Y".equals(pk4Val)); - assertNull(rs.getBigDecimal("PK5")); - - // No more rows - assertFalse(rs.next()); + public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { + for (boolean pk3Desc : new boolean[] { false, true }) { + for (boolean pk4Desc : new boolean[] { false, true }) { + String configDesc = "PK3 DESC=" + pk3Desc + ", PK4 DESC=" + pk4Desc; + try { + doTestQueryWithIndexAfterAddingNullablePKColumn(pk3Desc, pk4Desc); + } catch (AssertionError e) { + throw new AssertionError("Failed for configuration: " + configDesc, e); + } catch (Exception e) { + throw new Exception("Failed for configuration: " + configDesc, e); } - // IS NULL on trailing nullable PK column generates POINT LOOKUPS (2 keys in array) - assertPointLookupsAreGenerated(stmt, 2); } } } - @Test - public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { + private void doTestQueryWithIndexAfterAddingNullablePKColumn(boolean pk3Desc, boolean pk4Desc) throws Exception { String tableName = generateUniqueName(); String indexName = "IDX_" + generateUniqueName(); + String pk3SortOrder = pk3Desc ? " DESC" : ""; + String pk4SortOrder = pk4Desc ? " DESC" : ""; + try (Connection conn = DriverManager.getConnection(getUrl())) { // Step 1: Create table with one nullable PK column (PK3) at the end String createTableDdl = "CREATE TABLE " + tableName + " (" @@ -334,7 +353,7 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { + "PK3 VARCHAR, " // Nullable PK column at end + "COL1 VARCHAR, " + "COL2 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3)" + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3" + pk3SortOrder + ")" + ")"; conn.createStatement().execute(createTableDdl); conn.commit(); @@ -375,8 +394,8 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { } conn.commit(); - // Step 4: Add a new nullable PK column (PK4) via ALTER TABLE - String alterTableDdl = "ALTER TABLE " + tableName + " ADD PK4 VARCHAR PRIMARY KEY"; + // Step 4: Add a new nullable PK column (PK4) via ALTER TABLE with configured sort order + String alterTableDdl = "ALTER TABLE " + tableName + " ADD PK4 VARCHAR PRIMARY KEY " + pk4SortOrder; conn.createStatement().execute(alterTableDdl); conn.commit(); @@ -456,8 +475,88 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { } } + /** + * Configuration for VIEW_PK1 column type and sort order. + */ + private enum ViewPk1Config { + /** Fixed-width CHAR(1) */ + FIXED_WIDTH_CHAR("CHAR(1) NOT NULL", ""), + /** Nullable VARCHAR with ASC sort order (default) */ + NULLABLE_ASC("VARCHAR", ""), + /** Nullable VARCHAR with DESC sort order */ + NULLABLE_DESC("VARCHAR", " DESC"); + + private final String dataType; + private final String sortOrder; + + ViewPk1Config(String dataType, String sortOrder) { + this.dataType = dataType; + this.sortOrder = sortOrder; + } + + public String getDataType() { + return dataType; + } + + public String getSortOrder() { + return sortOrder; + } + + @Override + public String toString() { + return name() + "[" + dataType + (sortOrder.isEmpty() ? "" : sortOrder) + "]"; + } + } + + /** + * Configuration for VIEW_PK2 column sort order (always nullable VARCHAR). + */ + private enum ViewPk2Config { + /** Nullable VARCHAR with ASC sort order (default) */ + NULLABLE_ASC(""), + /** Nullable VARCHAR with DESC sort order */ + NULLABLE_DESC(" DESC"); + + private final String sortOrder; + + ViewPk2Config(String sortOrder) { + this.sortOrder = sortOrder; + } + + public String getSortOrder() { + return sortOrder; + } + + @Override + public String toString() { + return name() + "[VARCHAR" + (sortOrder.isEmpty() ? "" : sortOrder) + "]"; + } + } + + /** + * Parameterized test for multi-point lookups on view with nullable PK columns. + * Tests all combinations of: + * - VIEW_PK1 config: FIXED_WIDTH_CHAR, NULLABLE_ASC, NULLABLE_DESC + * - VIEW_PK2 config: NULLABLE_ASC, NULLABLE_DESC + */ @Test public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception { + for (ViewPk1Config viewPk1Config : ViewPk1Config.values()) { + for (ViewPk2Config viewPk2Config : ViewPk2Config.values()) { + String configDesc = "VIEW_PK1=" + viewPk1Config + ", VIEW_PK2=" + viewPk2Config; + try { + doTestMultiPointLookupsOnViewWithNullablePKColumns(viewPk1Config, viewPk2Config); + } catch (AssertionError e) { + throw new AssertionError("Failed for configuration: " + configDesc, e); + } catch (Exception e) { + throw new Exception("Failed for configuration: " + configDesc, e); + } + } + } + } + + private void doTestMultiPointLookupsOnViewWithNullablePKColumns( + ViewPk1Config viewPk1Config, ViewPk2Config viewPk2Config) throws Exception { String tableName = generateUniqueName(); String viewName = "VW_" + generateUniqueName(); @@ -474,12 +573,13 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception conn.createStatement().execute(createTableDdl); conn.commit(); - // Step 2: Create view that adds two nullable PK columns + // Step 2: Create view that adds two nullable PK columns with configured types/sort orders String createViewDdl = "CREATE VIEW " + viewName + " (" - + "VIEW_PK1 VARCHAR, " // Nullable PK column added by view - + "VIEW_PK2 VARCHAR, " // Second nullable PK column added by view + + "VIEW_PK1 " + viewPk1Config.getDataType() + ", " + + "VIEW_PK2 VARCHAR, " + "VIEW_COL1 VARCHAR, " - + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1, VIEW_PK2)" + + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1" + viewPk1Config.getSortOrder() + + ", VIEW_PK2" + viewPk2Config.getSortOrder() + ")" + ") AS SELECT * FROM " + tableName; conn.createStatement().execute(createViewDdl); conn.commit(); @@ -488,17 +588,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception String upsertSql = "UPSERT INTO " + viewName + " (PK1, PK2, VIEW_PK1, VIEW_PK2, COL1, COL2, VIEW_COL1) VALUES (?, ?, ?, ?, ?, ?, ?)"; try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { - // Row 1: Both VIEW_PK1 and VIEW_PK2 are NULL - stmt.setString(1, "A"); - stmt.setString(2, "BASE1"); - stmt.setNull(3, Types.VARCHAR); - stmt.setNull(4, Types.VARCHAR); - stmt.setString(5, "col1_val1"); - stmt.setString(6, "col2_val1"); - stmt.setString(7, "view_col1_val1"); - stmt.executeUpdate(); - - // Row 2: VIEW_PK1 = 'X', VIEW_PK2 is NULL + // Row 1: VIEW_PK1 = 'X', VIEW_PK2 is NULL stmt.setString(1, "A"); stmt.setString(2, "BASE1"); stmt.setString(3, "X"); @@ -508,7 +598,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception stmt.setString(7, "view_col1_val2"); stmt.executeUpdate(); - // Row 3: VIEW_PK1 = 'X', VIEW_PK2 = 'P' + // Row 2: VIEW_PK1 = 'X', VIEW_PK2 = 'P' stmt.setString(1, "A"); stmt.setString(2, "BASE1"); stmt.setString(3, "X"); @@ -518,7 +608,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception stmt.setString(7, "view_col1_val3"); stmt.executeUpdate(); - // Row 4: VIEW_PK1 = 'X', VIEW_PK2 = 'Q' + // Row 3: VIEW_PK1 = 'X', VIEW_PK2 = 'Q' stmt.setString(1, "A"); stmt.setString(2, "BASE1"); stmt.setString(3, "X"); @@ -528,7 +618,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception stmt.setString(7, "view_col1_val4"); stmt.executeUpdate(); - // Row 5: VIEW_PK1 = 'Y', VIEW_PK2 is NULL + // Row 4: VIEW_PK1 = 'Y', VIEW_PK2 is NULL stmt.setString(1, "A"); stmt.setString(2, "BASE1"); stmt.setString(3, "Y"); @@ -538,7 +628,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception stmt.setString(7, "view_col1_val5"); stmt.executeUpdate(); - // Row 6: VIEW_PK1 = 'Y', VIEW_PK2 = 'Q' + // Row 5: VIEW_PK1 = 'Y', VIEW_PK2 = 'Q' stmt.setString(1, "A"); stmt.setString(2, "BASE1"); stmt.setString(3, "Y"); @@ -548,7 +638,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception stmt.setString(7, "view_col1_val6"); stmt.executeUpdate(); - // Row 7: Different base PK prefix + // Row 6: Different base PK prefix stmt.setString(1, "B"); stmt.setString(2, "BASE2"); stmt.setString(3, "X"); @@ -573,8 +663,10 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception int rowCount = 0; while (rs.next()) { rowCount++; + String pk2 = rs.getString("PK2"); String viewPk1 = rs.getString("VIEW_PK1"); String viewPk2 = rs.getString("VIEW_PK2"); + assertEquals(pk2, "BASE1"); // Verify VIEW_PK1 is either X or Y assertTrue("X".equals(viewPk1) || "Y".equals(viewPk1)); // Verify VIEW_PK2 is NULL, P, or Q @@ -592,4 +684,79 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception } } } + + @Test + public void testPointLookupWithAllNullablePKColumns() throws Exception { + String tableName = generateUniqueName(); + + // Create table with all nullable PK columns + String ddl = "CREATE TABLE " + tableName + " (" + + "PK1 VARCHAR, " // Nullable, fixed-width + + "PK2 VARCHAR, " // Nullable + + "PK3 DECIMAL, " // Nullable + + "COL1 VARCHAR, " + + "COL2 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3)" + + ")"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (java.sql.Statement stmt = conn.createStatement()) { + stmt.execute(ddl); + conn.commit(); + } + } + + // Insert test data with various null combinations + String upsertSql = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { + // Row 2: PK1 has value, PK2 and PK3 are NULL + stmt.setString(1, "A"); + stmt.setNull(2, Types.VARCHAR); + stmt.setNull(3, Types.DECIMAL); + stmt.setString(4, "val3"); + stmt.setString(5, "val4"); + stmt.executeUpdate(); + + // Row 3: PK1 and PK2 have values, PK3 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setNull(3, Types.DECIMAL); + stmt.setString(4, "val5"); + stmt.setString(5, "val6"); + stmt.executeUpdate(); + + // Row 4: All PKs have values (no nulls) + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, new BigDecimal("100.5")); + stmt.setString(4, "val7"); + stmt.setString(5, "val8"); + stmt.executeUpdate(); + + // Row 5: PK1 is NULL, PK2 has value, PK3 is NULL + stmt.setNull(1, Types.CHAR); + stmt.setString(2, "C"); + stmt.setNull(3, Types.DECIMAL); + stmt.setString(4, "val9"); + stmt.setString(5, "val10"); + stmt.executeUpdate(); + + conn.commit(); + } + } + + try (Connection conn = DriverManager.getConnection(getUrl())) { + // Query 1: All PKs are NULL - should match no rows + String selectSql = "SELECT COL1, COL2 FROM " + tableName + + " WHERE PK1 IS NULL AND PK2 IS NULL AND PK3 IS NULL"; + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + try (java.sql.ResultSet rs = stmt.executeQuery()) { + assertFalse(rs.next()); + } + // IS NULL on all trailing nullable PK columns generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + } + } + } } From 299a707d41a4c89337a10b9a702d0e0374e2a51d Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Tue, 17 Feb 2026 17:33:07 +0530 Subject: [PATCH 06/15] Add test coverage --- .../apache/phoenix/compile/ScanRanges.java | 17 ++++++- .../org/apache/phoenix/util/ScanUtil.java | 1 + ...WhereOptimizerForArrayAnyNullablePKIT.java | 46 +++++++++++++------ 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index eae29c0c003..71a1a222946 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -630,9 +630,19 @@ private static boolean isPointLookup(RowKeySchema schema, List> r // 1. Trailing nulls are stripped when storing the key (no trailing separator bytes) // 2. getPointKeys() generates the correct key without trailing null bytes // 3. The generated key matches exactly what's stored for rows with trailing NULL - if (!keyRange.isSingleKey()) { + // 4. Not handling legcay tables with DESC sort order and impacted via bug PHOENIX-2067 + if ( + !keyRange.isSingleKey() + ) { return false; } + else if (i == lastIndex && keyRange == KeyRange.IS_NULL_RANGE) { + Field lastField = schema.getField(schema.getFieldCount() - 1); + SortOrder lastFieldSortOrder = lastField.getSortOrder(); + if (lastFieldSortOrder == SortOrder.DESC && !schema.rowKeyOrderOptimizable()) { + return false; + } + } } } return true; @@ -666,10 +676,13 @@ private static List getPointKeys(List> ranges, int[] slot do { length = ScanUtil.setKey(schema, ranges, slotSpan, position, Bound.LOWER, key, offset, offset, ranges.size(), offset); + // Handle case when generated single point key is empty byte array + if (length == 0) { + continue; + } if (isSalted) { key[0] = SaltingUtil.getSaltingByte(key, offset, length, bucketNum); } - keys.add(Arrays.copyOf(key, length + offset)); } while (incrementKey(ranges, position)); return keys; diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index 0711f3fb2ef..e6be7abe19e 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -610,6 +610,7 @@ public static int setKey(RowKeySchema schema, List> slots, int[] --i >= schemaStartIndex && offset > byteOffset && !(field = schema.getField(--fieldIndex)).getDataType().isFixedWidth() && hasSeparatorBytes(key, field, offset) + && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable()) || field.getSortOrder() == SortOrder.ASC) ) { if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { offset--; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index 91893f46c9a..61f3fce8462 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -686,18 +686,35 @@ private void doTestMultiPointLookupsOnViewWithNullablePKColumns( } @Test - public void testPointLookupWithAllNullablePKColumns() throws Exception { + public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { + for (boolean descSortOrder : new boolean[] { false, true }) { + for (boolean salted : new boolean[] { false, true }) { + String configDesc = "DESC=" + descSortOrder + ", SALTED=" + salted; + try { + doTestPointLookupWithIsNullCheckOnAllPKColumns(descSortOrder, salted); + } catch (AssertionError e) { + throw new AssertionError("Failed for configuration: " + configDesc, e); + } catch (Exception e) { + throw new Exception("Failed for configuration: " + configDesc, e); + } + } + } + } + + private void doTestPointLookupWithIsNullCheckOnAllPKColumns(boolean descSortOrder, boolean salted) + throws Exception { String tableName = generateUniqueName(); + String sortOrder = descSortOrder ? " DESC" : ""; // Create table with all nullable PK columns String ddl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR, " // Nullable, fixed-width - + "PK2 VARCHAR, " // Nullable - + "PK3 DECIMAL, " // Nullable + + "PK1 VARCHAR, " + + "PK2 VARCHAR, " + + "PK3 DECIMAL, " + "COL1 VARCHAR, " + "COL2 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3)" - + ")"; + + "CONSTRAINT pk PRIMARY KEY (PK1 " + sortOrder + ", PK2 " + sortOrder + ", PK3 " + sortOrder + ")" + + ")" + (salted ? " SALT_BUCKETS=3" : ""); try (Connection conn = DriverManager.getConnection(getUrl())) { try (java.sql.Statement stmt = conn.createStatement()) { stmt.execute(ddl); @@ -706,11 +723,11 @@ public void testPointLookupWithAllNullablePKColumns() throws Exception { } // Insert test data with various null combinations - String upsertSql = "UPSERT INTO " + tableName - + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; + String upsertSql = + "UPSERT INTO " + tableName + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; try (Connection conn = DriverManager.getConnection(getUrl())) { try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { - // Row 2: PK1 has value, PK2 and PK3 are NULL + // Row 1: PK1 has value, PK2 and PK3 are NULL stmt.setString(1, "A"); stmt.setNull(2, Types.VARCHAR); stmt.setNull(3, Types.DECIMAL); @@ -718,7 +735,7 @@ public void testPointLookupWithAllNullablePKColumns() throws Exception { stmt.setString(5, "val4"); stmt.executeUpdate(); - // Row 3: PK1 and PK2 have values, PK3 is NULL + // Row 2: PK1 and PK2 have values, PK3 is NULL stmt.setString(1, "A"); stmt.setString(2, "B"); stmt.setNull(3, Types.DECIMAL); @@ -726,7 +743,7 @@ public void testPointLookupWithAllNullablePKColumns() throws Exception { stmt.setString(5, "val6"); stmt.executeUpdate(); - // Row 4: All PKs have values (no nulls) + // Row 3: All PKs have values (no nulls) stmt.setString(1, "A"); stmt.setString(2, "B"); stmt.setBigDecimal(3, new BigDecimal("100.5")); @@ -734,7 +751,7 @@ public void testPointLookupWithAllNullablePKColumns() throws Exception { stmt.setString(5, "val8"); stmt.executeUpdate(); - // Row 5: PK1 is NULL, PK2 has value, PK3 is NULL + // Row 4: PK1 is NULL, PK2 has value, PK3 is NULL stmt.setNull(1, Types.CHAR); stmt.setString(2, "C"); stmt.setNull(3, Types.DECIMAL); @@ -754,8 +771,9 @@ public void testPointLookupWithAllNullablePKColumns() throws Exception { try (java.sql.ResultSet rs = stmt.executeQuery()) { assertFalse(rs.next()); } - // IS NULL on all trailing nullable PK columns generates single POINT LOOKUP - assertPointLookupsAreGenerated(stmt, 1); + String scanType = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery() + .getExplainPlan().getPlanStepsAsAttributes().getExplainScanType(); + assertNull(scanType); } } } From 949959c2a64539335bfe3450fcbc738cd7dc9bd3 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Tue, 17 Feb 2026 18:42:55 +0530 Subject: [PATCH 07/15] Refactor tests --- ...WhereOptimizerForArrayAnyNullablePKIT.java | 326 ++++++++---------- 1 file changed, 142 insertions(+), 184 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index 61f3fce8462..ba9cbc81de2 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -30,54 +30,138 @@ import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Types; +import java.util.Arrays; +import java.util.Collection; import org.apache.phoenix.compile.ExplainPlan; import org.apache.phoenix.compile.ExplainPlanAttributes; import org.apache.phoenix.compile.QueryPlan; import org.apache.phoenix.jdbc.PhoenixPreparedStatement; +import org.junit.Assume; import org.junit.Test; import org.junit.experimental.categories.Category; - +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +/** + * Integration tests for WHERE clause optimization with nullable PK columns. + *

+ * Parameterized at class level with the following parameters: + *

    + *
  • {@code salted} - whether the table uses salt buckets
    • + *
  • {@code columnConfig} - primary column configuration (fixed-width, variable-width ASC/DESC)
    • + *
  • {@code secondarySortDesc} - sort order for secondary columns (true=DESC, false=ASC)
    • + *
+ * This covers all combinations of column types and sort orders across all tests. + */ @Category(NeedsOwnMiniClusterTest.class) +@RunWith(Parameterized.class) public class WhereOptimizerForArrayAnyNullablePKIT extends WhereOptimizerForArrayAnyITBase { private static final BigDecimal PK3_VAL = new BigDecimal("100.5"); + private static final int SALT_BUCKETS = 3; /** - * Configuration for PK4 column type and sort order. + * Configuration for column type and sort order. + * Used for primary configurable columns in tests. */ - private enum Pk4Config { + public enum ColumnConfig { /** Fixed-width CHAR(1) NOT NULL */ - FIXED_WIDTH_CHAR("CHAR(1) NOT NULL", ""), - /** Nullable VARCHAR with ASC sort order (default) */ - NULLABLE_ASC("VARCHAR NOT NULL", ""), - /** Nullable VARCHAR with DESC sort order */ - NULLABLE_DESC("VARCHAR NOT NULL", " DESC"); + FIXED_WIDTH("CHAR(1) NOT NULL", "", false), + /** Variable-width VARCHAR with ASC sort order (default) */ + VARWIDTH_ASC("VARCHAR", "", false), + /** Variable-width VARCHAR with DESC sort order */ + VARWIDTH_DESC("VARCHAR", " DESC", true); private final String dataType; - private final String sortOrder; + private final String sortOrderClause; + private final boolean isDesc; - Pk4Config(String dataType, String sortOrder) { + ColumnConfig(String dataType, String sortOrderClause, boolean isDesc) { this.dataType = dataType; - this.sortOrder = sortOrder; + this.sortOrderClause = sortOrderClause; + this.isDesc = isDesc; } public String getDataType() { return dataType; } - public String getSortOrder() { - return sortOrder; + public String getSortOrderClause() { + return sortOrderClause; + } + + public boolean isDesc() { + return isDesc; + } + + public boolean isFixedWidth() { + return this == FIXED_WIDTH; + } + + /** + * Returns the data type with NOT NULL constraint for PK columns that must be NOT NULL. + */ + public String getNotNullDataType() { + if (this == FIXED_WIDTH) { + return dataType; // Already includes NOT NULL + } + return dataType + " NOT NULL"; } @Override public String toString() { - return name() + "[" + dataType + (sortOrder.isEmpty() ? "" : sortOrder) + "]"; + return name(); } } + @Parameters(name = "salted={0}, columnConfig={1}, secondarySortDesc={2}") + public static Collection data() { + return Arrays.asList(new Object[][] { + { false, ColumnConfig.FIXED_WIDTH, false }, + { false, ColumnConfig.FIXED_WIDTH, true }, + { false, ColumnConfig.VARWIDTH_ASC, false }, + { false, ColumnConfig.VARWIDTH_ASC, true }, + { false, ColumnConfig.VARWIDTH_DESC, false }, + { false, ColumnConfig.VARWIDTH_DESC, true }, + { true, ColumnConfig.FIXED_WIDTH, false }, + { true, ColumnConfig.FIXED_WIDTH, true }, + { true, ColumnConfig.VARWIDTH_ASC, false }, + { true, ColumnConfig.VARWIDTH_ASC, true }, + { true, ColumnConfig.VARWIDTH_DESC, false }, + { true, ColumnConfig.VARWIDTH_DESC, true } + }); + } + + private final boolean salted; + private final ColumnConfig columnConfig; + private final boolean secondarySortDesc; + + public WhereOptimizerForArrayAnyNullablePKIT(boolean salted, ColumnConfig columnConfig, + boolean secondarySortDesc) { + this.salted = salted; + this.columnConfig = columnConfig; + this.secondarySortDesc = secondarySortDesc; + } + + /** + * Returns the SALT_BUCKETS clause if salted, otherwise empty string. + */ + private String getSaltClause() { + return salted ? " SALT_BUCKETS=" + SALT_BUCKETS : ""; + } + + /** + * Returns the sort order clause for secondary columns based on secondarySortDesc parameter. + */ + private String getSecondarySortOrder() { + return secondarySortDesc ? " DESC" : ""; + } + /** * Creates a table with 5 PK columns (last one nullable) and inserts test data. + * Uses class-level columnConfig for PK4 and secondarySortDesc for PK5. * Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 DECIMAL, PK4 (configurable), PK5 DECIMAL (nullable) * COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR * Inserts 5 rows: @@ -86,24 +170,22 @@ public String toString() { * Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) * Row 4: (A, B, 100.5, Z, NULL, val10, val11, val12) * Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) - * @param pk4Config configuration for PK4 column type and sort order - * @param pk5Desc if true, PK5 will have DESC sort order * @return the generated table name */ - private String createTableAndInsertTestDataForNullablePKTests(Pk4Config pk4Config, boolean pk5Desc) throws Exception { + private String createTableAndInsertTestDataForNullablePKTests() throws Exception { String tableName = generateUniqueName(); - String pk5SortOrder = pk5Desc ? " DESC" : ""; String ddl = "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + "PK2 VARCHAR NOT NULL, " + "PK3 DECIMAL NOT NULL, " - + "PK4 " + pk4Config.getDataType() + ", " + + "PK4 " + columnConfig.getNotNullDataType() + ", " + "PK5 DECIMAL, " + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "COL3 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + pk4Config.getSortOrder() + ", PK5" + pk5SortOrder + ")" - + ")"; + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + columnConfig.getSortOrderClause() + + ", PK5" + getSecondarySortOrder() + ")" + + ")" + getSaltClause(); try (Connection conn = DriverManager.getConnection(getUrl())) { try (java.sql.Statement stmt = conn.createStatement()) { stmt.execute(ddl); @@ -188,29 +270,14 @@ private void assertQueryUsesIndex(PreparedStatement stmt, String indexName) thro } /** - * Parameterized test for single point lookup with nullable PK. - * Tests all combinations of: - * - PK4 config: FIXED_WIDTH_CHAR, NULLABLE_ASC, NULLABLE_DESC - * - PK5 sort order: ASC, DESC + * Test for single point lookup with nullable PK. + * Uses class-level parameters: + * - columnConfig for PK4 type and sort order + * - secondarySortDesc for PK5 sort order */ @Test public void testSinglePointLookupWithNullablePK() throws Exception { - for (Pk4Config pk4Config : Pk4Config.values()) { - for (boolean pk5Desc : new boolean[] { false, true }) { - String configDesc = "PK4=" + pk4Config + ", PK5 DESC=" + pk5Desc; - try { - doTestSinglePointLookupWithNullablePK(pk4Config, pk5Desc); - } catch (AssertionError e) { - throw new AssertionError("Failed for configuration: " + configDesc, e); - } catch (Exception e) { - throw new Exception("Failed for configuration: " + configDesc, e); - } - } - } - } - - private void doTestSinglePointLookupWithNullablePK(Pk4Config pk4Config, boolean pk5Desc) throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(pk4Config, pk5Desc); + String tableName = createTableAndInsertTestDataForNullablePKTests(); // Query with = for PK4 and IS NULL for PK5 // IS NULL on trailing nullable PK column generates POINT LOOKUP because: @@ -257,29 +324,14 @@ private void doTestSinglePointLookupWithNullablePK(Pk4Config pk4Config, boolean } /** - * Parameterized test for multi-point lookups with nullable PK. - * Tests all combinations of: - * - PK4 config: FIXED_WIDTH_CHAR, NULLABLE_ASC, NULLABLE_DESC - * - PK5 sort order: ASC, DESC + * Test for multi-point lookups with nullable PK. + * Uses class-level parameters: + * - columnConfig for PK4 type and sort order + * - secondarySortDesc for PK5 sort order */ @Test public void testMultiPointLookupsWithNullablePK() throws Exception { - for (Pk4Config pk4Config : Pk4Config.values()) { - for (boolean pk5Desc : new boolean[] { false, true }) { - String configDesc = "PK4=" + pk4Config + ", PK5 DESC=" + pk5Desc; - try { - doTestMultiPointLookupsWithNullablePK(pk4Config, pk5Desc); - } catch (AssertionError e) { - throw new AssertionError("Failed for configuration: " + configDesc, e); - } catch (Exception e) { - throw new Exception("Failed for configuration: " + configDesc, e); - } - } - } - } - - private void doTestMultiPointLookupsWithNullablePK(Pk4Config pk4Config, boolean pk5Desc) throws Exception { - String tableName = createTableAndInsertTestDataForNullablePKTests(pk4Config, pk5Desc); + String tableName = createTableAndInsertTestDataForNullablePKTests(); // Query with =ANY(?) for PK4 and IS NULL for PK5 // IS NULL on trailing nullable PK column generates POINT LOOKUPS because: @@ -317,33 +369,21 @@ private void doTestMultiPointLookupsWithNullablePK(Pk4Config pk4Config, boolean } /** - * Parameterized test for query with index after adding nullable PK column. - * Tests all combinations of: - * - PK3 sort order: ASC, DESC - * - PK4 sort order: ASC, DESC + * Test for query with index after adding nullable PK column. + * Uses class-level parameters: + * - columnConfig.isDesc() for PK3 sort order + * - secondarySortDesc for PK4 sort order (added via ALTER TABLE) + * Note: columnConfig's fixed-width vs variable-width aspect is not applicable here, + * so FIXED_WIDTH and VARWIDTH_ASC behave the same (both use ASC sort order). */ @Test public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { - for (boolean pk3Desc : new boolean[] { false, true }) { - for (boolean pk4Desc : new boolean[] { false, true }) { - String configDesc = "PK3 DESC=" + pk3Desc + ", PK4 DESC=" + pk4Desc; - try { - doTestQueryWithIndexAfterAddingNullablePKColumn(pk3Desc, pk4Desc); - } catch (AssertionError e) { - throw new AssertionError("Failed for configuration: " + configDesc, e); - } catch (Exception e) { - throw new Exception("Failed for configuration: " + configDesc, e); - } - } - } - } - - private void doTestQueryWithIndexAfterAddingNullablePKColumn(boolean pk3Desc, boolean pk4Desc) throws Exception { + Assume.assumeFalse(columnConfig.isFixedWidth()); String tableName = generateUniqueName(); String indexName = "IDX_" + generateUniqueName(); - String pk3SortOrder = pk3Desc ? " DESC" : ""; - String pk4SortOrder = pk4Desc ? " DESC" : ""; + String pk3SortOrder = columnConfig.isDesc() ? " DESC" : ""; + String pk4SortOrder = secondarySortDesc ? " DESC" : ""; try (Connection conn = DriverManager.getConnection(getUrl())) { // Step 1: Create table with one nullable PK column (PK3) at the end @@ -354,13 +394,13 @@ private void doTestQueryWithIndexAfterAddingNullablePKColumn(boolean pk3Desc, bo + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3" + pk3SortOrder + ")" - + ")"; + + ")" + getSaltClause(); conn.createStatement().execute(createTableDdl); conn.commit(); // Step 2: Create a covered global index on the data table String createIndexDdl = "CREATE INDEX " + indexName + " ON " + tableName - + " (COL1) INCLUDE (COL2)"; + + " (COL1) INCLUDE (COL2) " + getSaltClause(); conn.createStatement().execute(createIndexDdl); conn.commit(); @@ -476,87 +516,13 @@ private void doTestQueryWithIndexAfterAddingNullablePKColumn(boolean pk3Desc, bo } /** - * Configuration for VIEW_PK1 column type and sort order. - */ - private enum ViewPk1Config { - /** Fixed-width CHAR(1) */ - FIXED_WIDTH_CHAR("CHAR(1) NOT NULL", ""), - /** Nullable VARCHAR with ASC sort order (default) */ - NULLABLE_ASC("VARCHAR", ""), - /** Nullable VARCHAR with DESC sort order */ - NULLABLE_DESC("VARCHAR", " DESC"); - - private final String dataType; - private final String sortOrder; - - ViewPk1Config(String dataType, String sortOrder) { - this.dataType = dataType; - this.sortOrder = sortOrder; - } - - public String getDataType() { - return dataType; - } - - public String getSortOrder() { - return sortOrder; - } - - @Override - public String toString() { - return name() + "[" + dataType + (sortOrder.isEmpty() ? "" : sortOrder) + "]"; - } - } - - /** - * Configuration for VIEW_PK2 column sort order (always nullable VARCHAR). - */ - private enum ViewPk2Config { - /** Nullable VARCHAR with ASC sort order (default) */ - NULLABLE_ASC(""), - /** Nullable VARCHAR with DESC sort order */ - NULLABLE_DESC(" DESC"); - - private final String sortOrder; - - ViewPk2Config(String sortOrder) { - this.sortOrder = sortOrder; - } - - public String getSortOrder() { - return sortOrder; - } - - @Override - public String toString() { - return name() + "[VARCHAR" + (sortOrder.isEmpty() ? "" : sortOrder) + "]"; - } - } - - /** - * Parameterized test for multi-point lookups on view with nullable PK columns. - * Tests all combinations of: - * - VIEW_PK1 config: FIXED_WIDTH_CHAR, NULLABLE_ASC, NULLABLE_DESC - * - VIEW_PK2 config: NULLABLE_ASC, NULLABLE_DESC + * Test for multi-point lookups on view with nullable PK columns. + * Uses class-level parameters: + * - columnConfig for VIEW_PK1 type and sort order + * - secondarySortDesc for VIEW_PK2 sort order */ @Test public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception { - for (ViewPk1Config viewPk1Config : ViewPk1Config.values()) { - for (ViewPk2Config viewPk2Config : ViewPk2Config.values()) { - String configDesc = "VIEW_PK1=" + viewPk1Config + ", VIEW_PK2=" + viewPk2Config; - try { - doTestMultiPointLookupsOnViewWithNullablePKColumns(viewPk1Config, viewPk2Config); - } catch (AssertionError e) { - throw new AssertionError("Failed for configuration: " + configDesc, e); - } catch (Exception e) { - throw new Exception("Failed for configuration: " + configDesc, e); - } - } - } - } - - private void doTestMultiPointLookupsOnViewWithNullablePKColumns( - ViewPk1Config viewPk1Config, ViewPk2Config viewPk2Config) throws Exception { String tableName = generateUniqueName(); String viewName = "VW_" + generateUniqueName(); @@ -569,17 +535,17 @@ private void doTestMultiPointLookupsOnViewWithNullablePKColumns( + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1, PK2)" - + ")"; + + ")" + getSaltClause(); conn.createStatement().execute(createTableDdl); conn.commit(); // Step 2: Create view that adds two nullable PK columns with configured types/sort orders String createViewDdl = "CREATE VIEW " + viewName + " (" - + "VIEW_PK1 " + viewPk1Config.getDataType() + ", " + + "VIEW_PK1 " + columnConfig.getDataType() + ", " + "VIEW_PK2 VARCHAR, " + "VIEW_COL1 VARCHAR, " - + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1" + viewPk1Config.getSortOrder() - + ", VIEW_PK2" + viewPk2Config.getSortOrder() + ")" + + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1" + columnConfig.getSortOrderClause() + + ", VIEW_PK2" + getSecondarySortOrder() + ")" + ") AS SELECT * FROM " + tableName; conn.createStatement().execute(createViewDdl); conn.commit(); @@ -685,26 +651,18 @@ private void doTestMultiPointLookupsOnViewWithNullablePKColumns( } } + /** + * Test for point lookup with IS NULL check on all PK columns. + * Uses class-level columnConfig.isDesc() for sort order of all PK columns. + * Note: columnConfig's fixed-width vs variable-width aspect is not applicable here, + * so FIXED_WIDTH and VARWIDTH_ASC behave the same (both use ASC sort order). + * The secondarySortDesc parameter is also not applicable for this test. + */ @Test public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { - for (boolean descSortOrder : new boolean[] { false, true }) { - for (boolean salted : new boolean[] { false, true }) { - String configDesc = "DESC=" + descSortOrder + ", SALTED=" + salted; - try { - doTestPointLookupWithIsNullCheckOnAllPKColumns(descSortOrder, salted); - } catch (AssertionError e) { - throw new AssertionError("Failed for configuration: " + configDesc, e); - } catch (Exception e) { - throw new Exception("Failed for configuration: " + configDesc, e); - } - } - } - } - - private void doTestPointLookupWithIsNullCheckOnAllPKColumns(boolean descSortOrder, boolean salted) - throws Exception { + Assume.assumeFalse(columnConfig.isFixedWidth() || secondarySortDesc); String tableName = generateUniqueName(); - String sortOrder = descSortOrder ? " DESC" : ""; + String sortOrder = columnConfig.isDesc() ? " DESC" : ""; // Create table with all nullable PK columns String ddl = "CREATE TABLE " + tableName + " (" @@ -714,7 +672,7 @@ private void doTestPointLookupWithIsNullCheckOnAllPKColumns(boolean descSortOrde + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1 " + sortOrder + ", PK2 " + sortOrder + ", PK3 " + sortOrder + ")" - + ")" + (salted ? " SALT_BUCKETS=3" : ""); + + ")" + getSaltClause(); try (Connection conn = DriverManager.getConnection(getUrl())) { try (java.sql.Statement stmt = conn.createStatement()) { stmt.execute(ddl); From 3314e6a21c1559a4baefd2f668e8f535336dfa35 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Wed, 18 Feb 2026 01:45:20 +0530 Subject: [PATCH 08/15] Identfied broken range scans with IS NULL --- .../apache/phoenix/compile/ScanRanges.java | 14 +- .../apache/phoenix/schema/MetaDataClient.java | 6 +- .../org/apache/phoenix/util/ScanUtil.java | 3 +- .../end2end/WhereOptimizerForArrayAnyIT.java | 10 - .../WhereOptimizerForArrayAnyITBase.java | 19 +- ...WhereOptimizerForArrayAnyNullablePKIT.java | 336 ++++++++++++------ .../phoenix/compile/WhereOptimizerTest.java | 4 +- 7 files changed, 255 insertions(+), 137 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index 71a1a222946..3aea36a4ad3 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -100,11 +100,7 @@ public static ScanRanges create(RowKeySchema schema, List> ranges } TimeRange rowTimestampRange = getRowTimestampColumnRange(ranges, schema, rowTimestampColIndex); boolean isPointLookup = isPointLookup(schema, ranges, slotSpan, useSkipScan); - boolean isPointLookupWithNull = false; if (isPointLookup) { - if (ranges.get(ranges.size() - 1).get(0) == KeyRange.IS_NULL_RANGE) { - isPointLookupWithNull = true; - } // TODO: consider keeping original to use for serialization as it would be smaller? List keys = ScanRanges.getPointKeys(ranges, slotSpan, schema, nBuckets); List keyRanges = Lists.newArrayListWithExpectedSize(keys.size()); @@ -149,9 +145,6 @@ public static ScanRanges create(RowKeySchema schema, List> ranges if (nBuckets == null || !isPointLookup || !useSkipScan) { byte[] minKey = ScanUtil.getMinKey(schema, sortedRanges, slotSpan); byte[] maxKey = ScanUtil.getMaxKey(schema, sortedRanges, slotSpan); - if (isPointLookupWithNull) { - System.out.println("IS_NULL_RANGE: "); - } // If the maxKey has crossed the salt byte boundary, then we do not // have anything to filter at the upper end of the range if (ScanUtil.crossesPrefixBoundary(maxKey, ScanUtil.getPrefix(minKey, offset), offset)) { @@ -631,12 +624,9 @@ private static boolean isPointLookup(RowKeySchema schema, List> r // 2. getPointKeys() generates the correct key without trailing null bytes // 3. The generated key matches exactly what's stored for rows with trailing NULL // 4. Not handling legcay tables with DESC sort order and impacted via bug PHOENIX-2067 - if ( - !keyRange.isSingleKey() - ) { + if (!keyRange.isSingleKey()) { return false; - } - else if (i == lastIndex && keyRange == KeyRange.IS_NULL_RANGE) { + } else if (i == lastIndex && keyRange == KeyRange.IS_NULL_RANGE) { Field lastField = schema.getField(schema.getFieldCount() - 1); SortOrder lastFieldSortOrder = lastField.getSortOrder(); if (lastFieldSortOrder == SortOrder.DESC && !schema.rowKeyOrderOptimizable()) { diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java index 5008e28f606..0da85f851dc 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java @@ -4870,9 +4870,9 @@ public MutationState addColumn(PTable table, List origColumnDefs, /** * To check if TTL is defined at any of the child below we are checking it at * {@link org.apache.phoenix.coprocessor.MetaDataEndpointImpl#mutateColumn(List, ColumnMutator, int, PTable, PTable, boolean)} - * level where in function {@link org.apache.phoenix.coprocessor.MetaDataEndpointImpl# - * validateIfMutationAllowedOnParent(PTable, List, PTableType, long, byte[], byte[], - * byte[], List, int)} we are already traversing through allDescendantViews. + * level where in function + * {@link org.apache.phoenix.coprocessor.MetaDataEndpointImpl# validateIfMutationAllowedOnParent(PTable, List, PTableType, long, byte[], byte[], byte[], List, int)} + * we are already traversing through allDescendantViews. */ } diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index e6be7abe19e..6173ef05ab0 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -610,7 +610,8 @@ public static int setKey(RowKeySchema schema, List> slots, int[] --i >= schemaStartIndex && offset > byteOffset && !(field = schema.getField(--fieldIndex)).getDataType().isFixedWidth() && hasSeparatorBytes(key, field, offset) - && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable()) || field.getSortOrder() == SortOrder.ASC) + && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable()) + || field.getSortOrder() == SortOrder.ASC) ) { if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { offset--; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java index 437861c2910..c9651f2113a 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.java @@ -782,16 +782,6 @@ private void assertPointLookupsAreGenerated(Statement stmt, String selectSql, assertPointLookupsAreGenerated(queryPlan, noOfPointLookups); } - private void assertSkipScanIsGenerated(PreparedStatement stmt, int skipListSize) - throws SQLException { - QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); - ExplainPlan explain = queryPlan.getExplainPlan(); - ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); - String expectedScanType = - "SKIP SCAN ON " + skipListSize + " KEY" + (skipListSize > 1 ? "S " : " "); - assertEquals(expectedScanType, planAttributes.getExplainScanType()); - } - private void createTableASCPkColumns(String tableName) throws SQLException { String ddl = "CREATE TABLE " + tableName + " (" + "pk1 INTEGER NOT NULL, " + "pk2 VARCHAR(3), " + "col1 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (pk1, pk2)" + ")"; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java index f090a2399e3..f7179d66216 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java @@ -22,7 +22,6 @@ import java.sql.PreparedStatement; import java.sql.SQLException; import java.util.HashMap; - import org.apache.phoenix.compile.ExplainPlan; import org.apache.phoenix.compile.ExplainPlanAttributes; import org.apache.phoenix.compile.QueryPlan; @@ -55,4 +54,22 @@ protected void assertPointLookupsAreGenerated(QueryPlan queryPlan, int noOfPoint "POINT LOOKUP ON " + noOfPointLookups + " KEY" + (noOfPointLookups > 1 ? "S " : " "); assertEquals(expectedScanType, planAttributes.getExplainScanType()); } + + protected void assertSkipScanIsGenerated(PreparedStatement stmt, int skipListSize) + throws SQLException { + QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); + ExplainPlan explain = queryPlan.getExplainPlan(); + ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); + String expectedScanType = + "SKIP SCAN ON " + skipListSize + " KEY" + (skipListSize > 1 ? "S " : " "); + assertEquals(expectedScanType, planAttributes.getExplainScanType()); + } + + protected void assertRangeScanIsGenerated(PreparedStatement stmt) throws SQLException { + QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); + ExplainPlan explain = queryPlan.getExplainPlan(); + ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); + String expectedScanType = "RANGE SCAN "; + assertEquals(expectedScanType, planAttributes.getExplainScanType()); + } } diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index ba9cbc81de2..2419371c3ab 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -32,7 +32,6 @@ import java.sql.Types; import java.util.Arrays; import java.util.Collection; - import org.apache.phoenix.compile.ExplainPlan; import org.apache.phoenix.compile.ExplainPlanAttributes; import org.apache.phoenix.compile.QueryPlan; @@ -49,9 +48,10 @@ *

* Parameterized at class level with the following parameters: *

    - *
  • {@code salted} - whether the table uses salt buckets
    • - *
  • {@code columnConfig} - primary column configuration (fixed-width, variable-width ASC/DESC)
    • - *
  • {@code secondarySortDesc} - sort order for secondary columns (true=DESC, false=ASC)
    • + *
  • {@code salted} - whether the table uses salt buckets
    • + *
  • {@code columnConfig} - primary column configuration (fixed-width, variable-width + * ASC/DESC)
    • + *
  • {@code secondarySortDesc} - sort order for secondary columns (true=DESC, false=ASC)
    • *
* This covers all combinations of column types and sort orders across all tests. */ @@ -63,8 +63,7 @@ public class WhereOptimizerForArrayAnyNullablePKIT extends WhereOptimizerForArra private static final int SALT_BUCKETS = 3; /** - * Configuration for column type and sort order. - * Used for primary configurable columns in tests. + * Configuration for column type and sort order. Used for primary configurable columns in tests. */ public enum ColumnConfig { /** Fixed-width CHAR(1) NOT NULL */ @@ -118,28 +117,21 @@ public String toString() { @Parameters(name = "salted={0}, columnConfig={1}, secondarySortDesc={2}") public static Collection data() { - return Arrays.asList(new Object[][] { - { false, ColumnConfig.FIXED_WIDTH, false }, - { false, ColumnConfig.FIXED_WIDTH, true }, - { false, ColumnConfig.VARWIDTH_ASC, false }, - { false, ColumnConfig.VARWIDTH_ASC, true }, - { false, ColumnConfig.VARWIDTH_DESC, false }, - { false, ColumnConfig.VARWIDTH_DESC, true }, - { true, ColumnConfig.FIXED_WIDTH, false }, - { true, ColumnConfig.FIXED_WIDTH, true }, - { true, ColumnConfig.VARWIDTH_ASC, false }, - { true, ColumnConfig.VARWIDTH_ASC, true }, - { true, ColumnConfig.VARWIDTH_DESC, false }, - { true, ColumnConfig.VARWIDTH_DESC, true } - }); + return Arrays.asList(new Object[][] { { false, ColumnConfig.FIXED_WIDTH, false }, + { false, ColumnConfig.FIXED_WIDTH, true }, { false, ColumnConfig.VARWIDTH_ASC, false }, + { false, ColumnConfig.VARWIDTH_ASC, true }, { false, ColumnConfig.VARWIDTH_DESC, false }, + { false, ColumnConfig.VARWIDTH_DESC, true }, { true, ColumnConfig.FIXED_WIDTH, false }, + { true, ColumnConfig.FIXED_WIDTH, true }, { true, ColumnConfig.VARWIDTH_ASC, false }, + { true, ColumnConfig.VARWIDTH_ASC, true }, { true, ColumnConfig.VARWIDTH_DESC, false }, + { true, ColumnConfig.VARWIDTH_DESC, true } }); } private final boolean salted; private final ColumnConfig columnConfig; private final boolean secondarySortDesc; - public WhereOptimizerForArrayAnyNullablePKIT(boolean salted, ColumnConfig columnConfig, - boolean secondarySortDesc) { + public WhereOptimizerForArrayAnyNullablePKIT(boolean salted, ColumnConfig columnConfig, + boolean secondarySortDesc) { this.salted = salted; this.columnConfig = columnConfig; this.secondarySortDesc = secondarySortDesc; @@ -160,32 +152,22 @@ private String getSecondarySortOrder() { } /** - * Creates a table with 5 PK columns (last one nullable) and inserts test data. - * Uses class-level columnConfig for PK4 and secondarySortDesc for PK5. - * Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 DECIMAL, PK4 (configurable), PK5 DECIMAL (nullable) - * COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR - * Inserts 5 rows: - * Row 1: (A, B, 100.5, X, NULL, val1, val2, val3) - * Row 2: (A, B, 100.5, Y, NULL, val4, val5, val6) - * Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) - * Row 4: (A, B, 100.5, Z, NULL, val10, val11, val12) - * Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) + * Creates a table with 5 PK columns (last one nullable) and inserts test data. Uses class-level + * columnConfig for PK4 and secondarySortDesc for PK5. Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 + * DECIMAL, PK4 (configurable), PK5 DECIMAL (nullable) COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR + * Inserts 5 rows: Row 1: (A, B, 100.5, X, NULL, val1, val2, val3) Row 2: (A, B, 100.5, Y, NULL, + * val4, val5, val6) Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) Row 4: (A, B, 100.5, Z, NULL, + * val10, val11, val12) Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) * @return the generated table name */ private String createTableAndInsertTestDataForNullablePKTests() throws Exception { String tableName = generateUniqueName(); - String ddl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR NOT NULL, " - + "PK2 VARCHAR NOT NULL, " - + "PK3 DECIMAL NOT NULL, " - + "PK4 " + columnConfig.getNotNullDataType() + ", " - + "PK5 DECIMAL, " - + "COL1 VARCHAR, " - + "COL2 VARCHAR, " - + "COL3 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + columnConfig.getSortOrderClause() - + ", PK5" + getSecondarySortOrder() + ")" - + ")" + getSaltClause(); + String ddl = + "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + "PK2 VARCHAR NOT NULL, " + + "PK3 DECIMAL NOT NULL, " + "PK4 " + columnConfig.getNotNullDataType() + ", " + + "PK5 DECIMAL, " + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "COL3 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + columnConfig.getSortOrderClause() + + ", PK5" + getSecondarySortOrder() + ")" + ")" + getSaltClause(); try (Connection conn = DriverManager.getConnection(getUrl())) { try (java.sql.Statement stmt = conn.createStatement()) { stmt.execute(ddl); @@ -193,8 +175,7 @@ private String createTableAndInsertTestDataForNullablePKTests() throws Exception } } - String upsertStmt = "UPSERT INTO " + tableName - + " (PK1, PK2, PK3, PK4, PK5, COL1, COL2, COL3) " + String upsertStmt = "UPSERT INTO " + tableName + " (PK1, PK2, PK3, PK4, PK5, COL1, COL2, COL3) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?)"; try (Connection conn = DriverManager.getConnection(getUrl())) { try (PreparedStatement stmt = conn.prepareStatement(upsertStmt)) { @@ -270,10 +251,8 @@ private void assertQueryUsesIndex(PreparedStatement stmt, String indexName) thro } /** - * Test for single point lookup with nullable PK. - * Uses class-level parameters: - * - columnConfig for PK4 type and sort order - * - secondarySortDesc for PK5 sort order + * Test for single point lookup with nullable PK. Uses class-level parameters: - columnConfig for + * PK4 type and sort order - secondarySortDesc for PK5 sort order */ @Test public void testSinglePointLookupWithNullablePK() throws Exception { @@ -285,8 +264,7 @@ public void testSinglePointLookupWithNullablePK() throws Exception { // - The generated lookup key is exactly what's stored for rows with trailing NULL try (Connection conn = DriverManager.getConnection(getUrl())) { String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName - + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " - + "AND PK4 = ? AND PK5 IS NULL"; + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " + "AND PK4 = ? AND PK5 IS NULL"; try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { stmt.setString(1, "A"); stmt.setString(2, "B"); @@ -324,10 +302,8 @@ public void testSinglePointLookupWithNullablePK() throws Exception { } /** - * Test for multi-point lookups with nullable PK. - * Uses class-level parameters: - * - columnConfig for PK4 type and sort order - * - secondarySortDesc for PK5 sort order + * Test for multi-point lookups with nullable PK. Uses class-level parameters: - columnConfig for + * PK4 type and sort order - secondarySortDesc for PK5 sort order */ @Test public void testMultiPointLookupsWithNullablePK() throws Exception { @@ -369,11 +345,9 @@ public void testMultiPointLookupsWithNullablePK() throws Exception { } /** - * Test for query with index after adding nullable PK column. - * Uses class-level parameters: - * - columnConfig.isDesc() for PK3 sort order - * - secondarySortDesc for PK4 sort order (added via ALTER TABLE) - * Note: columnConfig's fixed-width vs variable-width aspect is not applicable here, + * Test for query with index after adding nullable PK column. Uses class-level parameters: - + * columnConfig.isDesc() for PK3 sort order - secondarySortDesc for PK4 sort order (added via + * ALTER TABLE) Note: columnConfig's fixed-width vs variable-width aspect is not applicable here, * so FIXED_WIDTH and VARWIDTH_ASC behave the same (both use ASC sort order). */ @Test @@ -387,14 +361,10 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { try (Connection conn = DriverManager.getConnection(getUrl())) { // Step 1: Create table with one nullable PK column (PK3) at the end - String createTableDdl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR NOT NULL, " - + "PK2 VARCHAR NOT NULL, " - + "PK3 VARCHAR, " // Nullable PK column at end - + "COL1 VARCHAR, " - + "COL2 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3" + pk3SortOrder + ")" - + ")" + getSaltClause(); + String createTableDdl = "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + + "PK2 VARCHAR NOT NULL, " + "PK3 VARCHAR, " // Nullable PK column at end + + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3" + + pk3SortOrder + ")" + ")" + getSaltClause(); conn.createStatement().execute(createTableDdl); conn.commit(); @@ -406,8 +376,8 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { // Step 3: Insert initial data (before ALTER TABLE) // Row 1: PK3 is NULL - String upsertSql = "UPSERT INTO " + tableName - + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; + String upsertSql = + "UPSERT INTO " + tableName + " (PK1, PK2, PK3, COL1, COL2) VALUES (?, ?, ?, ?, ?)"; try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { stmt.setString(1, "A"); stmt.setString(2, "B"); @@ -435,13 +405,14 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { conn.commit(); // Step 4: Add a new nullable PK column (PK4) via ALTER TABLE with configured sort order - String alterTableDdl = "ALTER TABLE " + tableName + " ADD PK4 VARCHAR PRIMARY KEY " + pk4SortOrder; + String alterTableDdl = + "ALTER TABLE " + tableName + " ADD PK4 VARCHAR PRIMARY KEY " + pk4SortOrder; conn.createStatement().execute(alterTableDdl); conn.commit(); // Step 5: Insert more data with same PK prefix but different PK4 values - upsertSql = "UPSERT INTO " + tableName - + " (PK1, PK2, PK3, PK4, COL1, COL2) VALUES (?, ?, ?, ?, ?, ?)"; + upsertSql = + "UPSERT INTO " + tableName + " (PK1, PK2, PK3, PK4, COL1, COL2) VALUES (?, ?, ?, ?, ?, ?)"; try (PreparedStatement stmt = conn.prepareStatement(upsertSql)) { // Row 4: Same prefix as Row 1 (A, B, NULL) but PK4 = 'X' stmt.setString(1, "A"); @@ -485,7 +456,8 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { + "PK1, PK2, PK3, PK4, COL1, COL2 FROM " + tableName + " WHERE PK1 = ? AND PK2 = ? AND PK3 IS NULL AND (PK4 IS NULL OR PK4 = ANY(?)) AND COL1 = ANY(?)"; Array pk4Arr = conn.createArrayOf("VARCHAR", new String[] { "Z", "Y" }); - Array col1Arr = conn.createArrayOf("VARCHAR", new String[] { "indexed_val5", "indexed_val1", "indexed_val7", "indexed_val4" }); + Array col1Arr = conn.createArrayOf("VARCHAR", + new String[] { "indexed_val5", "indexed_val1", "indexed_val7", "indexed_val4" }); try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { stmt.setString(1, "A"); stmt.setString(2, "B"); @@ -516,10 +488,8 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { } /** - * Test for multi-point lookups on view with nullable PK columns. - * Uses class-level parameters: - * - columnConfig for VIEW_PK1 type and sort order - * - secondarySortDesc for VIEW_PK2 sort order + * Test for multi-point lookups on view with nullable PK columns. Uses class-level parameters: - + * columnConfig for VIEW_PK1 type and sort order - secondarySortDesc for VIEW_PK2 sort order */ @Test public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception { @@ -529,24 +499,26 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception try (Connection conn = DriverManager.getConnection(getUrl())) { // Step 1: Create parent table with fixed-width NOT NULL last PK // Using CHAR (fixed-width) for PK2 to allow view to add PK columns - String createTableDdl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR NOT NULL, " - + "PK2 CHAR(10) NOT NULL, " // Fixed-width NOT NULL - allows view to add PKs - + "COL1 VARCHAR, " - + "COL2 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1, PK2)" - + ")" + getSaltClause(); + String createTableDdl = + "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + "PK2 CHAR(10) NOT NULL, " // Fixed-width + // NOT + // NULL + // - + // allows + // view + // to + // add + // PKs + + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1, PK2)" + ")" + + getSaltClause(); conn.createStatement().execute(createTableDdl); conn.commit(); // Step 2: Create view that adds two nullable PK columns with configured types/sort orders - String createViewDdl = "CREATE VIEW " + viewName + " (" - + "VIEW_PK1 " + columnConfig.getDataType() + ", " - + "VIEW_PK2 VARCHAR, " - + "VIEW_COL1 VARCHAR, " - + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1" + columnConfig.getSortOrderClause() - + ", VIEW_PK2" + getSecondarySortOrder() + ")" - + ") AS SELECT * FROM " + tableName; + String createViewDdl = "CREATE VIEW " + viewName + " (" + "VIEW_PK1 " + + columnConfig.getDataType() + ", " + "VIEW_PK2 VARCHAR, " + "VIEW_COL1 VARCHAR, " + + "CONSTRAINT view_pk PRIMARY KEY (VIEW_PK1" + columnConfig.getSortOrderClause() + + ", VIEW_PK2" + getSecondarySortOrder() + ")" + ") AS SELECT * FROM " + tableName; conn.createStatement().execute(createViewDdl); conn.commit(); @@ -638,7 +610,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception // Verify VIEW_PK2 is NULL, P, or Q assertTrue(viewPk2 == null || "P".equals(viewPk2) || "Q".equals(viewPk2)); } - // Expected rows: + // Expected rows: // (A, BASE1, X, NULL), (A, BASE1, X, P), (A, BASE1, X, Q), // (A, BASE1, Y, NULL), (A, BASE1, Y, Q) assertEquals(5, rowCount); @@ -652,11 +624,11 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception } /** - * Test for point lookup with IS NULL check on all PK columns. - * Uses class-level columnConfig.isDesc() for sort order of all PK columns. - * Note: columnConfig's fixed-width vs variable-width aspect is not applicable here, - * so FIXED_WIDTH and VARWIDTH_ASC behave the same (both use ASC sort order). - * The secondarySortDesc parameter is also not applicable for this test. + * Test for point lookup with IS NULL check on all PK columns. Uses class-level + * columnConfig.isDesc() for sort order of all PK columns. Note: columnConfig's fixed-width vs + * variable-width aspect is not applicable here, so FIXED_WIDTH and VARWIDTH_ASC behave the same + * (both use ASC sort order). The secondarySortDesc parameter is also not applicable for this + * test. */ @Test public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { @@ -665,14 +637,9 @@ public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { String sortOrder = columnConfig.isDesc() ? " DESC" : ""; // Create table with all nullable PK columns - String ddl = "CREATE TABLE " + tableName + " (" - + "PK1 VARCHAR, " - + "PK2 VARCHAR, " - + "PK3 DECIMAL, " - + "COL1 VARCHAR, " - + "COL2 VARCHAR, " - + "CONSTRAINT pk PRIMARY KEY (PK1 " + sortOrder + ", PK2 " + sortOrder + ", PK3 " + sortOrder + ")" - + ")" + getSaltClause(); + String ddl = "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR, " + "PK2 VARCHAR, " + + "PK3 DECIMAL, " + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1 " + + sortOrder + ", PK2 " + sortOrder + ", PK3 " + sortOrder + ")" + ")" + getSaltClause(); try (Connection conn = DriverManager.getConnection(getUrl())) { try (java.sql.Statement stmt = conn.createStatement()) { stmt.execute(ddl); @@ -704,7 +671,7 @@ public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { // Row 3: All PKs have values (no nulls) stmt.setString(1, "A"); stmt.setString(2, "B"); - stmt.setBigDecimal(3, new BigDecimal("100.5")); + stmt.setBigDecimal(3, PK3_VAL); stmt.setString(4, "val7"); stmt.setString(5, "val8"); stmt.executeUpdate(); @@ -735,4 +702,157 @@ public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { } } } + + /** + * Test for RANGE SCAN and SKIP SCAN with IS NULL on second-to-last nullable PK column, without + * including the last PK column in the WHERE clause. Uses class-level parameters: - salted for + * table salting - columnConfig for PK3 sort order (fixed-width vs variable-width aspect not + * applicable for DECIMAL) - secondarySortDesc for PK4 sort order + */ + @Test + public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exception { + Assume.assumeFalse(columnConfig.isFixedWidth()); + String tableName = generateUniqueName(); + String pk4SortOrder = secondarySortDesc ? " DESC" : ""; + + // Create table with last two PK columns (PK4, PK5) nullable + // PK3 uses columnConfig for sort order, PK4 uses secondarySortDesc + String ddl = "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + + "PK2 VARCHAR NOT NULL, " + "PK3 DECIMAL NOT NULL, " + "PK4 VARCHAR, " + "PK5 DECIMAL, " + + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3" + + columnConfig.getSortOrderClause() + ", PK4" + pk4SortOrder + ", PK5)" + ")" + + getSaltClause(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + conn.createStatement().execute(ddl); + conn.commit(); + } + + // Insert test data + String upsertStmt = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, PK4, PK5, COL1, COL2) VALUES (?, ?, ?, ?, ?, ?, ?)"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (PreparedStatement stmt = conn.prepareStatement(upsertStmt)) { + // Row 1: PK4 IS NULL, PK5 IS NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setNull(4, Types.VARCHAR); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val1"); + stmt.setString(7, "val2"); + stmt.executeUpdate(); + + // Row 2: PK4 IS NULL, PK5 has value + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setNull(4, Types.VARCHAR); + stmt.setBigDecimal(5, new BigDecimal("2.0")); + stmt.setString(6, "val3"); + stmt.setString(7, "val4"); + stmt.executeUpdate(); + + // Row 3: PK4 has value, PK5 IS NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val5"); + stmt.setString(7, "val6"); + stmt.executeUpdate(); + + // Row 4: PK4 has value, PK5 has value + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Y"); + stmt.setBigDecimal(5, new BigDecimal("3.0")); + stmt.setString(6, "val7"); + stmt.setString(7, "val8"); + stmt.executeUpdate(); + + // Row 5: Different PK3 value, PK4 IS NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, new BigDecimal("200.5")); + stmt.setNull(4, Types.VARCHAR); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "val9"); + stmt.setString(7, "val10"); + stmt.executeUpdate(); + + conn.commit(); + } + } + + try (Connection conn = DriverManager.getConnection(getUrl())) { + // Test 1: RANGE SCAN - equality on PK1-PK3, IS NULL on PK4, no condition on PK5 + // This should produce a RANGE SCAN because trailing PK5 is not constrained + String selectSql = "SELECT COL1, COL2, PK4, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 IS NULL"; + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 2 rows where PK4 IS NULL: + // - Row 1: PK4 NULL, PK5 NULL (val1, val2) + // - Row 2: PK4 NULL, PK5=2.0 (val3, val4) + assertTrue(rs.next()); + assertNull(rs.getString("PK4")); + String col1Val1 = rs.getString("COL1"); + assertTrue("val1".equals(col1Val1) || "val3".equals(col1Val1)); + + assertTrue(rs.next()); + assertNull(rs.getString("PK4")); + String col1Val2 = rs.getString("COL1"); + assertTrue("val1".equals(col1Val2) || "val3".equals(col1Val2)); + + assertNotEquals(col1Val1, col1Val2); + assertFalse(rs.next()); + } + // Query plan should show RANGE SCAN since trailing PK5 is not constrained + assertRangeScanIsGenerated(stmt); + } + + // Test 2: SKIP SCAN - equality on PK1-PK2, =ANY on PK3, IS NULL on PK4, no condition on PK5 + // This should produce a SKIP SCAN due to =ANY on PK3 + String selectSql2 = "SELECT COL1, COL2, PK3, PK4, PK5 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ANY(?) AND PK4 IS NULL"; + Array pk3Arr = + conn.createArrayOf("DECIMAL", new BigDecimal[] { PK3_VAL, new BigDecimal("200.5") }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql2)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setArray(3, pk3Arr); + try (java.sql.ResultSet rs = stmt.executeQuery()) { + // Should return 3 rows where PK4 IS NULL but returns all 5 rows due to existing bug + // inolving skip scan with IS NULL in where clause. + // - Row 1: PK3=100.5, PK4 NULL, PK5 NULL (val1, val2) + // - Row 2: PK3=100.5, PK4 NULL, PK5=2.0 (val3, val4) + // - Row 5: PK3=200.5, PK4 NULL, PK5 NULL (val9, val10) + int rowCount = 0; + while (rs.next()) { + rowCount++; + } + if (columnConfig == ColumnConfig.VARWIDTH_DESC) { + assertEquals(3, rowCount); + } else if (salted && columnConfig != ColumnConfig.VARWIDTH_DESC && !secondarySortDesc) { + assertEquals(4, rowCount); + } else { + assertEquals(5, rowCount); + } + } + if (salted) { + // 2 * no. of salt buckets = 2 * 3 = 6 + assertSkipScanIsGenerated(stmt, 6); + } else { + assertSkipScanIsGenerated(stmt, 2); + } + } + } + } + } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java index 67d9d55f6fc..f38959c88df 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java @@ -2733,8 +2733,8 @@ public void testTrailingIsNullWithOr() throws Exception { // Key 1: "a" (for b IS NULL - trailing separator stripped) byte[] key1 = Bytes.toBytes("a"); // Key 2: "a\0b" (for b = 'b' - trailing separator stripped for last column) - byte[] key2 = ByteUtil.concat(Bytes.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, - Bytes.toBytes("b")); + byte[] key2 = + ByteUtil.concat(Bytes.toBytes("a"), QueryConstants.SEPARATOR_BYTE_ARRAY, Bytes.toBytes("b")); // Keys are sorted, so key1 comes before key2 assertEquals(KeyRange.getKeyRange(key1), slots.get(0).get(0)); assertEquals(KeyRange.getKeyRange(key2), slots.get(0).get(1)); From a44269eeb730bf747bf78949f7d057d6a5b9160e Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Wed, 18 Feb 2026 02:36:20 +0530 Subject: [PATCH 09/15] Added all ITs --- .../WhereOptimizerForArrayAnyITBase.java | 8 + ...WhereOptimizerForArrayAnyNullablePKIT.java | 142 ++++++++++++++++-- 2 files changed, 136 insertions(+), 14 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java index f7179d66216..b33362e48df 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.java @@ -18,6 +18,7 @@ package org.apache.phoenix.end2end; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import java.sql.PreparedStatement; import java.sql.SQLException; @@ -72,4 +73,11 @@ protected void assertRangeScanIsGenerated(PreparedStatement stmt) throws SQLExce String expectedScanType = "RANGE SCAN "; assertEquals(expectedScanType, planAttributes.getExplainScanType()); } + + protected void assertDegenerateScanIsGenerated(PreparedStatement stmt) throws SQLException { + QueryPlan queryPlan = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery(); + ExplainPlan explain = queryPlan.getExplainPlan(); + ExplainPlanAttributes planAttributes = explain.getPlanStepsAsAttributes(); + assertNull(planAttributes.getExplainScanType()); + } } diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index 2419371c3ab..7c8ab31784e 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -28,6 +28,7 @@ import java.sql.Connection; import java.sql.DriverManager; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Types; import java.util.Arrays; @@ -270,7 +271,7 @@ public void testSinglePointLookupWithNullablePK() throws Exception { stmt.setString(2, "B"); stmt.setBigDecimal(3, PK3_VAL); stmt.setString(4, "X"); - try (java.sql.ResultSet rs = stmt.executeQuery()) { + try (ResultSet rs = stmt.executeQuery()) { // Should return 1 row: PK4='X' with PK5 IS NULL assertTrue(rs.next()); assertEquals("X", rs.getString("PK4")); @@ -286,7 +287,7 @@ public void testSinglePointLookupWithNullablePK() throws Exception { assertPointLookupsAreGenerated(stmt, 1); stmt.setString(4, "Y"); - try (java.sql.ResultSet rs = stmt.executeQuery()) { + try (ResultSet rs = stmt.executeQuery()) { // Should return 1 row: PK4='Y' with PK5 IS NULL assertTrue(rs.next()); assertEquals("Y", rs.getString("PK4")); @@ -322,7 +323,7 @@ public void testMultiPointLookupsWithNullablePK() throws Exception { stmt.setString(2, "B"); stmt.setBigDecimal(3, PK3_VAL); stmt.setArray(4, pk4Arr); - try (java.sql.ResultSet rs = stmt.executeQuery()) { + try (ResultSet rs = stmt.executeQuery()) { // Should return 2 rows: PK4='X' and PK4='Y' with PK5 IS NULL assertTrue(rs.next()); String pk4Val1 = rs.getString("PK4"); @@ -463,7 +464,7 @@ public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { stmt.setString(2, "B"); stmt.setArray(3, pk4Arr); stmt.setArray(4, col1Arr); - try (java.sql.ResultSet rs = stmt.executeQuery()) { + try (ResultSet rs = stmt.executeQuery()) { assertTrue(rs.next()); String pk4Val = rs.getString("PK4"); assertNull(pk4Val); @@ -597,7 +598,7 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception stmt.setString(2, "BASE1"); stmt.setArray(3, viewPk1Arr); stmt.setArray(4, viewPk2Arr); - try (java.sql.ResultSet rs = stmt.executeQuery()) { + try (ResultSet rs = stmt.executeQuery()) { int rowCount = 0; while (rs.next()) { rowCount++; @@ -624,14 +625,14 @@ public void testMultiPointLookupsOnViewWithNullablePKColumns() throws Exception } /** - * Test for point lookup with IS NULL check on all PK columns. Uses class-level + * Test for degenerate scan with IS NULL check on all PK columns. Uses class-level * columnConfig.isDesc() for sort order of all PK columns. Note: columnConfig's fixed-width vs * variable-width aspect is not applicable here, so FIXED_WIDTH and VARWIDTH_ASC behave the same * (both use ASC sort order). The secondarySortDesc parameter is also not applicable for this * test. */ @Test - public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { + public void testDegenerateScanWithIsNullCheckOnAllPKColumns() throws Exception { Assume.assumeFalse(columnConfig.isFixedWidth() || secondarySortDesc); String tableName = generateUniqueName(); String sortOrder = columnConfig.isDesc() ? " DESC" : ""; @@ -693,12 +694,10 @@ public void testPointLookupWithIsNullCheckOnAllPKColumns() throws Exception { String selectSql = "SELECT COL1, COL2 FROM " + tableName + " WHERE PK1 IS NULL AND PK2 IS NULL AND PK3 IS NULL"; try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { - try (java.sql.ResultSet rs = stmt.executeQuery()) { + try (ResultSet rs = stmt.executeQuery()) { assertFalse(rs.next()); } - String scanType = stmt.unwrap(PhoenixPreparedStatement.class).optimizeQuery() - .getExplainPlan().getPlanStepsAsAttributes().getExplainScanType(); - assertNull(scanType); + assertDegenerateScanIsGenerated(stmt); } } } @@ -796,7 +795,7 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio stmt.setString(1, "A"); stmt.setString(2, "B"); stmt.setBigDecimal(3, PK3_VAL); - try (java.sql.ResultSet rs = stmt.executeQuery()) { + try (ResultSet rs = stmt.executeQuery()) { // Should return 2 rows where PK4 IS NULL: // - Row 1: PK4 NULL, PK5 NULL (val1, val2) // - Row 2: PK4 NULL, PK5=2.0 (val3, val4) @@ -827,8 +826,8 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio stmt.setString(1, "A"); stmt.setString(2, "B"); stmt.setArray(3, pk3Arr); - try (java.sql.ResultSet rs = stmt.executeQuery()) { - // Should return 3 rows where PK4 IS NULL but returns all 5 rows due to existing bug + try (ResultSet rs = stmt.executeQuery()) { + // Should return 3 rows where PK4 IS NULL but returns 4 or 5 rows due to existing bug // inolving skip scan with IS NULL in where clause. // - Row 1: PK3=100.5, PK4 NULL, PK5 NULL (val1, val2) // - Row 2: PK3=100.5, PK4 NULL, PK5=2.0 (val3, val4) @@ -855,4 +854,119 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio } } + /** + * Test for multi-point lookups with IS NULL on a fixed-width NOT NULL PK column. This test + * verifies that degenerate scan is generated when using IS NULL on a fixed-width column (INTEGER + * NOT NULL) in the primary key, and no rows are returned since a NOT NULL column can never have + * NULL values. Uses class-level parameters: - salted for table salting - columnConfig for PK3 + * (nullable, variable-width) type and sort order - secondarySortDesc for PK4 (INTEGER NOT NULL, + * fixed-width) sort order + */ + @Test + public void testDegenerateScanWithIsNullOnFixedWidthPK() throws Exception { + String tableName = generateUniqueName(); + String pk4SortOrder = secondarySortDesc ? " DESC" : ""; + boolean isPK3FixedWidth = columnConfig.isFixedWidth(); + + String ddl = "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + + "PK2 VARCHAR NOT NULL, " + "PK3 " + columnConfig.getDataType() + ", " // Nullable + // variable-width + // column + + "PK4 INTEGER NOT NULL, " // Fixed-width NOT NULL column + + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3" + + columnConfig.getSortOrderClause() + ", PK4" + pk4SortOrder + ")" + ")" + getSaltClause(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + conn.createStatement().execute(ddl); + conn.commit(); + } + + // Insert test data with various PK3 values (some NULL, some non-NULL) + // PK4 is always non-NULL since it's a fixed-width NOT NULL column + String upsertStmt = + "UPSERT INTO " + tableName + " (PK1, PK2, PK3, PK4, COL1, COL2) VALUES (?, ?, ?, ?, ?, ?)"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (PreparedStatement stmt = conn.prepareStatement(upsertStmt)) { + // Row 1: PK3='X', PK4=1 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setString(3, "X"); + stmt.setInt(4, 1); + stmt.setString(5, "val1"); + stmt.setString(6, "val2"); + stmt.executeUpdate(); + + // Row 2: PK3='Y', PK4=2 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setString(3, "Y"); + stmt.setInt(4, 2); + stmt.setString(5, "val3"); + stmt.setString(6, "val4"); + stmt.executeUpdate(); + + // Row 3: PK3 is NULL, PK4=1 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + if (isPK3FixedWidth) { + stmt.setString(3, "U"); + } else { + stmt.setNull(3, Types.VARCHAR); + } + stmt.setInt(4, 1); + stmt.setString(5, "val5"); + stmt.setString(6, "val6"); + stmt.executeUpdate(); + + // Row 4: PK3='Z', PK4=3 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setString(3, "Z"); + stmt.setInt(4, 3); + stmt.setString(5, "val7"); + stmt.setString(6, "val8"); + stmt.executeUpdate(); + + // Row 5: PK3 is NULL, PK4=2 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + if (isPK3FixedWidth) { + stmt.setString(3, "U"); + } else { + stmt.setNull(3, Types.VARCHAR); + } + stmt.setInt(4, 2); + stmt.setString(5, "val9"); + stmt.setString(6, "val10"); + stmt.executeUpdate(); + + // Row 6: Different PK1, PK3='X', PK4=1 + stmt.setString(1, "C"); + stmt.setString(2, "B"); + stmt.setString(3, "X"); + stmt.setInt(4, 1); + stmt.setString(5, "val11"); + stmt.setString(6, "val12"); + stmt.executeUpdate(); + + conn.commit(); + } + } + + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK3, PK4 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ANY(?) AND PK4 IS NULL"; + Array pk3Arr = conn.createArrayOf("VARCHAR", new String[] { "X", "Y" }); + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setArray(3, pk3Arr); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 0 rows since PK4 is NOT NULL and cannot have NULL values + assertFalse("Expected no rows since PK4 (INTEGER NOT NULL) cannot be NULL", rs.next()); + } + assertDegenerateScanIsGenerated(stmt); + } + } + } } From bd75008d9c83e9850b41f19b3bbdf07de3094537 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Wed, 18 Feb 2026 11:57:35 +0530 Subject: [PATCH 10/15] Account for sep byte due to overflow --- .../src/main/java/org/apache/phoenix/util/ScanUtil.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index 6173ef05ab0..1e45fbe5b75 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -461,6 +461,7 @@ public static int setKey(RowKeySchema schema, List> slots, int[] boolean lastInclusiveUpperSingleKey = false; boolean anyInclusiveUpperRangeKey = false; boolean lastUnboundUpper = false; + boolean trailingSepByteFromLowerExclusiveOverflow = false; // The index used for slots should be incremented by 1, // but the index for the field it represents in the schema // should be incremented by 1 + value in the current slotSpan index @@ -565,6 +566,9 @@ public static int setKey(RowKeySchema schema, List> slots, int[] // have an end key specified. return -byteOffset; } + if (offset > 0 && key[offset - 1] == QueryConstants.SEPARATOR_BYTE) { + trailingSepByteFromLowerExclusiveOverflow = true; + } // We're filtering on values being non null here, but we still need the 0xFF // terminator, since DESC keys ignore the last byte as it's expected to be // the terminator. Without this, we'd ignore the separator byte that was @@ -612,6 +616,7 @@ public static int setKey(RowKeySchema schema, List> slots, int[] && hasSeparatorBytes(key, field, offset) && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable()) || field.getSortOrder() == SortOrder.ASC) + && !trailingSepByteFromLowerExclusiveOverflow ) { if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { offset--; From f8bca18e52acae3fd66ebae3031f072ed5120657 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Wed, 18 Feb 2026 20:36:07 +0530 Subject: [PATCH 11/15] Fix failing ITs --- .../phoenix/compile/RVCOffsetCompiler.java | 20 +++++++++---------- .../org/apache/phoenix/util/ScanUtil.java | 1 + 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java index decd25d3136..62c2cacf329 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java @@ -37,6 +37,7 @@ import org.apache.phoenix.parse.ParseNode; import org.apache.phoenix.parse.RowValueConstructorParseNode; import org.apache.phoenix.query.KeyRange; +import org.apache.phoenix.query.QueryConstants; import org.apache.phoenix.schema.PColumn; import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.PTableType; @@ -247,17 +248,16 @@ public CompiledOffset getRVCOffset(StatementContext context, FilterableStatement // 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()) { - // 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(); + throw new RowValueConstructorOffsetNotCoercibleException( + "RVC Offset must be a point lookup."); + } + if (rowKeyColumnExpressionOutput.isTrailingNull()) { + key = scanRanges.getScanRange().getLowerRange(); + byte[] keyCopy = new byte[key.length + 1]; + System.arraycopy(key, 0, keyCopy, 0, key.length); + keyCopy[key.length] = QueryConstants.SEPARATOR_BYTE; + key = keyCopy; } else { RowKeySchema.RowKeySchemaBuilder builder = new RowKeySchema.RowKeySchemaBuilder(columns.size()); diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index 1e45fbe5b75..30ad8f580c5 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -512,6 +512,7 @@ public static int setKey(RowKeySchema schema, List> slots, int[] // key slots would cause the flag to become true. lastInclusiveUpperSingleKey = range.isSingleKey() && inclusiveUpper; anyInclusiveUpperRangeKey |= !range.isSingleKey() && inclusiveUpper; + trailingSepByteFromLowerExclusiveOverflow = false; if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { // A null or empty byte array is always represented as a zero byte byte sepByte = From 8fad64f644e1c8e2201d5d4c7aa2f2d27450fb4b Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Thu, 19 Feb 2026 09:40:08 +0530 Subject: [PATCH 12/15] Fix all tests --- .../apache/phoenix/compile/ScanRanges.java | 15 +++++++++- .../org/apache/phoenix/util/ScanUtil.java | 28 +++++++++++++----- ...WhereOptimizerForArrayAnyNullablePKIT.java | 29 +++++++++++++------ 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index 3aea36a4ad3..99349515524 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -101,6 +101,8 @@ public static ScanRanges create(RowKeySchema schema, List> ranges TimeRange rowTimestampRange = getRowTimestampColumnRange(ranges, schema, rowTimestampColIndex); boolean isPointLookup = isPointLookup(schema, ranges, slotSpan, useSkipScan); if (isPointLookup) { + // Do this before transforming the ranges into list of key ranges with single slot + boolean isPointLookupWithTrailingNulls = isPointLookupWithTrailingNulls(ranges); // TODO: consider keeping original to use for serialization as it would be smaller? List keys = ScanRanges.getPointKeys(ranges, slotSpan, schema, nBuckets); List keyRanges = Lists.newArrayListWithExpectedSize(keys.size()); @@ -117,7 +119,7 @@ public static ScanRanges create(RowKeySchema schema, List> ranges useSkipScan = keyRanges.size() > 1; // Treat as binary if descending because we've got a separator byte at the end // which is not part of the value. - if (keys.size() > 1 || hasTrailingDescSeparatorByte(schema)) { + if (keys.size() > 1 || hasTrailingDescSeparatorByte(schema) || isPointLookupWithTrailingNulls) { schema = SchemaUtil.VAR_BINARY_SCHEMA; slotSpan = ScanUtil.SINGLE_COLUMN_SLOT_SPAN; } else { @@ -638,6 +640,17 @@ private static boolean isPointLookup(RowKeySchema schema, List> r return true; } + private static boolean isPointLookupWithTrailingNulls(List> ranges) { + int lastIndex = ranges.size() - 1; + List lastRange = ranges.get(lastIndex); + for (KeyRange keyRange : lastRange) { + if (keyRange == KeyRange.IS_NULL_RANGE) { + return true; + } + } + return false; + } + private static boolean incrementKey(List> slots, int[] position) { int idx = slots.size() - 1; while (idx >= 0 && (position[idx] = (position[idx] + 1) % slots.get(idx).size()) == 0) { diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index 83255d7f48f..8c443d33ee5 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -484,14 +484,25 @@ public static int setKey(RowKeySchema schema, List> slots, int[] slotEndIndex, slotStartIndex); } + private static boolean doesSlotsCoverAllColumnsWithoutMultiSpan(RowKeySchema schema, List> slots, int[] slotSpan) { + long slotSpanSum = 0; + for (int i = 0; i < slotSpan.length; i++) { + slotSpanSum += slotSpan[i]; + } + return slotSpanSum == 0 && slots.size() == schema.getMaxFields(); + } + public static int setKey(RowKeySchema schema, List> slots, int[] slotSpan, int[] position, Bound bound, byte[] key, int byteOffset, int slotStartIndex, int slotEndIndex, int schemaStartIndex) { int offset = byteOffset; boolean lastInclusiveUpperSingleKey = false; + boolean allInclusiveLowerSingleKey = bound == Bound.LOWER; boolean anyInclusiveUpperRangeKey = false; boolean lastUnboundUpper = false; - boolean trailingSepByteFromLowerExclusiveOverflow = false; + boolean slotsCoverAllColumnsWithoutMultiSpan = + doesSlotsCoverAllColumnsWithoutMultiSpan(schema, slots, slotSpan); + int trailingNullCount = 0; // The index used for slots should be incremented by 1, // but the index for the field it represents in the schema // should be incremented by 1 + value in the current slotSpan index @@ -521,6 +532,14 @@ public static int setKey(RowKeySchema schema, List> slots, int[] byte[] bytes = range.getRange(bound); System.arraycopy(bytes, 0, key, offset, bytes.length); offset += bytes.length; + + if (bytes.length == 0) { + trailingNullCount++; + } + else { + trailingNullCount = 0; + } + allInclusiveLowerSingleKey &= range.isSingleKey(); /* * We must add a terminator to a variable length key even for the last PK column if the lower @@ -543,7 +562,6 @@ public static int setKey(RowKeySchema schema, List> slots, int[] // key slots would cause the flag to become true. lastInclusiveUpperSingleKey = range.isSingleKey() && inclusiveUpper; anyInclusiveUpperRangeKey |= !range.isSingleKey() && inclusiveUpper; - trailingSepByteFromLowerExclusiveOverflow = false; if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { // A null or empty byte array is always represented as a zero byte byte sepByte = @@ -598,9 +616,6 @@ public static int setKey(RowKeySchema schema, List> slots, int[] // have an end key specified. return -byteOffset; } - if (offset > 0 && key[offset - 1] == QueryConstants.SEPARATOR_BYTE) { - trailingSepByteFromLowerExclusiveOverflow = true; - } // We're filtering on values being non null here, but we still need the 0xFF // terminator, since DESC keys ignore the last byte as it's expected to be // the terminator. Without this, we'd ignore the separator byte that was @@ -646,9 +661,8 @@ public static int setKey(RowKeySchema schema, List> slots, int[] --i >= schemaStartIndex && offset > byteOffset && !(field = schema.getField(--fieldIndex)).getDataType().isFixedWidth() && hasSeparatorBytes(key, field, offset) - && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable()) + && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable() && slotsCoverAllColumnsWithoutMultiSpan && allInclusiveLowerSingleKey && trailingNullCount-- > 0) || field.getSortOrder() == SortOrder.ASC) - && !trailingSepByteFromLowerExclusiveOverflow ) { if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { offset--; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index 7c8ab31784e..b58fe658a43 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -796,20 +796,25 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio stmt.setString(2, "B"); stmt.setBigDecimal(3, PK3_VAL); try (ResultSet rs = stmt.executeQuery()) { - // Should return 2 rows where PK4 IS NULL: + // Should return 2 rows where PK4 IS NULL if secondarySortDesc is false, otherwise 1 row due to bug involving DESC sort order and trailing IS NULL when doing range scan. // - Row 1: PK4 NULL, PK5 NULL (val1, val2) // - Row 2: PK4 NULL, PK5=2.0 (val3, val4) assertTrue(rs.next()); assertNull(rs.getString("PK4")); String col1Val1 = rs.getString("COL1"); - assertTrue("val1".equals(col1Val1) || "val3".equals(col1Val1)); + if (!secondarySortDesc) { + assertTrue("val1".equals(col1Val1) || "val3".equals(col1Val1)); - assertTrue(rs.next()); - assertNull(rs.getString("PK4")); - String col1Val2 = rs.getString("COL1"); - assertTrue("val1".equals(col1Val2) || "val3".equals(col1Val2)); + assertTrue(rs.next()); + assertNull(rs.getString("PK4")); + String col1Val2 = rs.getString("COL1"); + assertTrue("val1".equals(col1Val2) || "val3".equals(col1Val2)); - assertNotEquals(col1Val1, col1Val2); + assertNotEquals(col1Val1, col1Val2); + } + else { + assertTrue("val3".equals(col1Val1)); + } assertFalse(rs.next()); } // Query plan should show RANGE SCAN since trailing PK5 is not constrained @@ -836,10 +841,16 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio while (rs.next()) { rowCount++; } - if (columnConfig == ColumnConfig.VARWIDTH_DESC) { + if (columnConfig == ColumnConfig.VARWIDTH_DESC && !secondarySortDesc) { assertEquals(3, rowCount); - } else if (salted && columnConfig != ColumnConfig.VARWIDTH_DESC && !secondarySortDesc) { + } else if (columnConfig == ColumnConfig.VARWIDTH_DESC && secondarySortDesc) { + assertEquals(2, rowCount); + } else if (salted && columnConfig == ColumnConfig.VARWIDTH_ASC && !secondarySortDesc) { assertEquals(4, rowCount); + } else if (salted && columnConfig == ColumnConfig.VARWIDTH_ASC && secondarySortDesc) { + assertEquals(2, rowCount); + } else if (!salted && columnConfig == ColumnConfig.VARWIDTH_ASC && secondarySortDesc) { + assertEquals(1, rowCount); } else { assertEquals(5, rowCount); } From d17d7892a06426572812c28e1ed19ba1852ef1e2 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Thu, 19 Feb 2026 09:43:51 +0530 Subject: [PATCH 13/15] spotless apply --- .../org/apache/phoenix/compile/ScanRanges.java | 4 +++- .../java/org/apache/phoenix/util/ScanUtil.java | 15 ++++++++------- .../WhereOptimizerForArrayAnyNullablePKIT.java | 6 +++--- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index 99349515524..c6158b4b962 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -119,7 +119,9 @@ public static ScanRanges create(RowKeySchema schema, List> ranges useSkipScan = keyRanges.size() > 1; // Treat as binary if descending because we've got a separator byte at the end // which is not part of the value. - if (keys.size() > 1 || hasTrailingDescSeparatorByte(schema) || isPointLookupWithTrailingNulls) { + if ( + keys.size() > 1 || hasTrailingDescSeparatorByte(schema) || isPointLookupWithTrailingNulls + ) { schema = SchemaUtil.VAR_BINARY_SCHEMA; slotSpan = ScanUtil.SINGLE_COLUMN_SLOT_SPAN; } else { diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index 8c443d33ee5..6c24314df59 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -484,14 +484,15 @@ public static int setKey(RowKeySchema schema, List> slots, int[] slotEndIndex, slotStartIndex); } - private static boolean doesSlotsCoverAllColumnsWithoutMultiSpan(RowKeySchema schema, List> slots, int[] slotSpan) { + private static boolean doesSlotsCoverAllColumnsWithoutMultiSpan(RowKeySchema schema, + List> slots, int[] slotSpan) { long slotSpanSum = 0; for (int i = 0; i < slotSpan.length; i++) { slotSpanSum += slotSpan[i]; } return slotSpanSum == 0 && slots.size() == schema.getMaxFields(); } - + public static int setKey(RowKeySchema schema, List> slots, int[] slotSpan, int[] position, Bound bound, byte[] key, int byteOffset, int slotStartIndex, int slotEndIndex, int schemaStartIndex) { @@ -532,11 +533,10 @@ public static int setKey(RowKeySchema schema, List> slots, int[] byte[] bytes = range.getRange(bound); System.arraycopy(bytes, 0, key, offset, bytes.length); offset += bytes.length; - + if (bytes.length == 0) { trailingNullCount++; - } - else { + } else { trailingNullCount = 0; } allInclusiveLowerSingleKey &= range.isSingleKey(); @@ -661,8 +661,9 @@ public static int setKey(RowKeySchema schema, List> slots, int[] --i >= schemaStartIndex && offset > byteOffset && !(field = schema.getField(--fieldIndex)).getDataType().isFixedWidth() && hasSeparatorBytes(key, field, offset) - && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable() && slotsCoverAllColumnsWithoutMultiSpan && allInclusiveLowerSingleKey && trailingNullCount-- > 0) - || field.getSortOrder() == SortOrder.ASC) + && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable() + && slotsCoverAllColumnsWithoutMultiSpan && allInclusiveLowerSingleKey + && trailingNullCount-- > 0) || field.getSortOrder() == SortOrder.ASC) ) { if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { offset--; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index b58fe658a43..c8701038b92 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -796,7 +796,8 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio stmt.setString(2, "B"); stmt.setBigDecimal(3, PK3_VAL); try (ResultSet rs = stmt.executeQuery()) { - // Should return 2 rows where PK4 IS NULL if secondarySortDesc is false, otherwise 1 row due to bug involving DESC sort order and trailing IS NULL when doing range scan. + // Should return 2 rows where PK4 IS NULL if secondarySortDesc is false, otherwise 1 row + // due to bug involving DESC sort order and trailing IS NULL when doing range scan. // - Row 1: PK4 NULL, PK5 NULL (val1, val2) // - Row 2: PK4 NULL, PK5=2.0 (val3, val4) assertTrue(rs.next()); @@ -811,8 +812,7 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio assertTrue("val1".equals(col1Val2) || "val3".equals(col1Val2)); assertNotEquals(col1Val1, col1Val2); - } - else { + } else { assertTrue("val3".equals(col1Val1)); } assertFalse(rs.next()); From 75221d20d94d601045fb8a96539b61917aca9c97 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Thu, 19 Feb 2026 19:32:03 +0530 Subject: [PATCH 14/15] Fix doc comments, checkstyle, add more edge case coverage --- .../phoenix/compile/RVCOffsetCompiler.java | 3 + .../apache/phoenix/compile/ScanRanges.java | 16 +- .../org/apache/phoenix/util/ScanUtil.java | 6 +- ...WhereOptimizerForArrayAnyNullablePKIT.java | 229 ++++++++++++++++-- 4 files changed, 217 insertions(+), 37 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java index 62c2cacf329..739f7f019a5 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/RVCOffsetCompiler.java @@ -248,11 +248,14 @@ public CompiledOffset getRVCOffset(StatementContext context, FilterableStatement // check to see if this was a single key expression ScanRanges scanRanges = context.getScanRanges(); + // Always generate point lookup for RVC Offset if (!scanRanges.isPointLookup()) { throw new RowValueConstructorOffsetNotCoercibleException( "RVC Offset must be a point lookup."); } if (rowKeyColumnExpressionOutput.isTrailingNull()) { + // Handle trailing nulls in RVC offset by appending null byte at the end to generate immediate + // next key key = scanRanges.getScanRange().getLowerRange(); byte[] keyCopy = new byte[key.length + 1]; System.arraycopy(key, 0, keyCopy, 0, key.length); diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index c6158b4b962..7f7e21ec179 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -102,6 +102,7 @@ public static ScanRanges create(RowKeySchema schema, List> ranges boolean isPointLookup = isPointLookup(schema, ranges, slotSpan, useSkipScan); if (isPointLookup) { // Do this before transforming the ranges into list of key ranges with single slot + // Once the list is transformed into singleton list, IS_NULL_RANGE is no longer retained. boolean isPointLookupWithTrailingNulls = isPointLookupWithTrailingNulls(ranges); // TODO: consider keeping original to use for serialization as it would be smaller? List keys = ScanRanges.getPointKeys(ranges, slotSpan, schema, nBuckets); @@ -627,15 +628,12 @@ private static boolean isPointLookup(RowKeySchema schema, List> r // 1. Trailing nulls are stripped when storing the key (no trailing separator bytes) // 2. getPointKeys() generates the correct key without trailing null bytes // 3. The generated key matches exactly what's stored for rows with trailing NULL - // 4. Not handling legcay tables with DESC sort order and impacted via bug PHOENIX-2067 - if (!keyRange.isSingleKey()) { + // 4. Not handling legcay tables impacted via bug PHOENIX-2067 + if ( + !keyRange.isSingleKey() || (lastIndex == i && keyRange == KeyRange.IS_NULL_RANGE + && !schema.rowKeyOrderOptimizable()) + ) { return false; - } else if (i == lastIndex && keyRange == KeyRange.IS_NULL_RANGE) { - Field lastField = schema.getField(schema.getFieldCount() - 1); - SortOrder lastFieldSortOrder = lastField.getSortOrder(); - if (lastFieldSortOrder == SortOrder.DESC && !schema.rowKeyOrderOptimizable()) { - return false; - } } } } @@ -681,7 +679,7 @@ private static List getPointKeys(List> ranges, int[] slot do { length = ScanUtil.setKey(schema, ranges, slotSpan, position, Bound.LOWER, key, offset, offset, ranges.size(), offset); - // Handle case when generated single point key is empty byte array + // Handle case someone specify IS NULL on all PK columns. if (length == 0) { continue; } diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java index 6c24314df59..352b0a8a67f 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ScanUtil.java @@ -657,13 +657,15 @@ public static int setKey(RowKeySchema schema, List> slots, int[] // after the table has data, in which case there won't be a separator // byte. if (bound == Bound.LOWER) { + // Remove trailing separator bytes for DESC keys only if they are trailing nulls for point + // lookups and schema is rowKeyOrderOptimizable. while ( --i >= schemaStartIndex && offset > byteOffset && !(field = schema.getField(--fieldIndex)).getDataType().isFixedWidth() && hasSeparatorBytes(key, field, offset) - && ((field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable() + && (field.getSortOrder() == SortOrder.DESC && schema.rowKeyOrderOptimizable() && slotsCoverAllColumnsWithoutMultiSpan && allInclusiveLowerSingleKey - && trailingNullCount-- > 0) || field.getSortOrder() == SortOrder.ASC) + && trailingNullCount-- > 0 || field.getSortOrder() == SortOrder.ASC) ) { if (field.getDataType() != PVarbinaryEncoded.INSTANCE) { offset--; diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java index c8701038b92..c96a985cec1 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.java @@ -45,7 +45,7 @@ import org.junit.runners.Parameterized.Parameters; /** - * Integration tests for WHERE clause optimization with nullable PK columns. + * Integration tests for generating point lookups with trailing nulls. *

* Parameterized at class level with the following parameters: *

    @@ -152,15 +152,6 @@ private String getSecondarySortOrder() { return secondarySortDesc ? " DESC" : ""; } - /** - * Creates a table with 5 PK columns (last one nullable) and inserts test data. Uses class-level - * columnConfig for PK4 and secondarySortDesc for PK5. Schema: PK1 VARCHAR, PK2 VARCHAR, PK3 - * DECIMAL, PK4 (configurable), PK5 DECIMAL (nullable) COL1 VARCHAR, COL2 VARCHAR, COL3 VARCHAR - * Inserts 5 rows: Row 1: (A, B, 100.5, X, NULL, val1, val2, val3) Row 2: (A, B, 100.5, Y, NULL, - * val4, val5, val6) Row 3: (A, B, 100.5, X, 1.0, val7, val8, val9) Row 4: (A, B, 100.5, Z, NULL, - * val10, val11, val12) Row 5: (C, B, 100.5, X, NULL, val13, val14, val15) - * @return the generated table name - */ private String createTableAndInsertTestDataForNullablePKTests() throws Exception { String tableName = generateUniqueName(); String ddl = @@ -252,17 +243,14 @@ private void assertQueryUsesIndex(PreparedStatement stmt, String indexName) thro } /** - * Test for single point lookup with nullable PK. Uses class-level parameters: - columnConfig for - * PK4 type and sort order - secondarySortDesc for PK5 sort order + * Test for single point lookup with IS NULL on single trailing nullable PK column. Uses + * class-level parameters: - columnConfig for PK4 type and sort order - secondarySortDesc for PK5 + * sort order */ @Test public void testSinglePointLookupWithNullablePK() throws Exception { String tableName = createTableAndInsertTestDataForNullablePKTests(); - // Query with = for PK4 and IS NULL for PK5 - // IS NULL on trailing nullable PK column generates POINT LOOKUP because: - // - Trailing nulls are stripped when storing, so key for NULL matches stored key - // - The generated lookup key is exactly what's stored for rows with trailing NULL try (Connection conn = DriverManager.getConnection(getUrl())) { String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " + "AND PK4 = ? AND PK5 IS NULL"; @@ -310,10 +298,6 @@ public void testSinglePointLookupWithNullablePK() throws Exception { public void testMultiPointLookupsWithNullablePK() throws Exception { String tableName = createTableAndInsertTestDataForNullablePKTests(); - // Query with =ANY(?) for PK4 and IS NULL for PK5 - // IS NULL on trailing nullable PK column generates POINT LOOKUPS because: - // - Trailing nulls are stripped when storing, so key for NULL matches stored key - // - The generated lookup key is exactly what's stored for rows with trailing NULL try (Connection conn = DriverManager.getConnection(getUrl())) { String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5 FROM " + tableName + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? AND PK4 = ANY(?) AND PK5 IS NULL"; @@ -346,10 +330,10 @@ public void testMultiPointLookupsWithNullablePK() throws Exception { } /** - * Test for query with index after adding nullable PK column. Uses class-level parameters: - - * columnConfig.isDesc() for PK3 sort order - secondarySortDesc for PK4 sort order (added via - * ALTER TABLE) Note: columnConfig's fixed-width vs variable-width aspect is not applicable here, - * so FIXED_WIDTH and VARWIDTH_ASC behave the same (both use ASC sort order). + * Test for querying index with trailing nulls after adding nullable PK column. Uses class-level + * parameters: - columnConfig.isDesc() for PK3 sort order - secondarySortDesc for PK4 sort order + * (added via ALTER TABLE) Note: columnConfig's fixed-width vs variable-width aspect is not + * applicable here, so FIXED_WIDTH and VARWIDTH_ASC behave the same (both use ASC sort order). */ @Test public void testQueryWithIndexAfterAddingNullablePKColumn() throws Exception { @@ -690,7 +674,7 @@ public void testDegenerateScanWithIsNullCheckOnAllPKColumns() throws Exception { } try (Connection conn = DriverManager.getConnection(getUrl())) { - // Query 1: All PKs are NULL - should match no rows + // Query: All PKs are NULL - should match no rows String selectSql = "SELECT COL1, COL2 FROM " + tableName + " WHERE PK1 IS NULL AND PK2 IS NULL AND PK3 IS NULL"; try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { @@ -832,7 +816,7 @@ public void testRangeScanAndSkipScanWithIsNullOnSecondToLastPK() throws Exceptio stmt.setString(2, "B"); stmt.setArray(3, pk3Arr); try (ResultSet rs = stmt.executeQuery()) { - // Should return 3 rows where PK4 IS NULL but returns 4 or 5 rows due to existing bug + // Should return 3 rows where PK4 IS NULL but returns >=1 and <=5 rows due to existing bug // inolving skip scan with IS NULL in where clause. // - Row 1: PK3=100.5, PK4 NULL, PK5 NULL (val1, val2) // - Row 2: PK3=100.5, PK4 NULL, PK5=2.0 (val3, val4) @@ -980,4 +964,197 @@ public void testDegenerateScanWithIsNullOnFixedWidthPK() throws Exception { } } } + + private String createTableAndInsertTestDataForMultipleTrailingNullablePKTests() throws Exception { + String tableName = generateUniqueName(); + String ddl = + "CREATE TABLE " + tableName + " (" + "PK1 VARCHAR NOT NULL, " + "PK2 VARCHAR NOT NULL, " + + "PK3 DECIMAL NOT NULL, " + "PK4 " + columnConfig.getNotNullDataType() + ", " + + "PK5 DECIMAL, " + "PK6 VARCHAR, " + "COL1 VARCHAR, " + "COL2 VARCHAR, " + "COL3 VARCHAR, " + + "CONSTRAINT pk PRIMARY KEY (PK1, PK2, PK3, PK4" + columnConfig.getSortOrderClause() + + ", PK5" + getSecondarySortOrder() + ", PK6" + getSecondarySortOrder() + ")" + ")" + + getSaltClause(); + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (java.sql.Statement stmt = conn.createStatement()) { + stmt.execute(ddl); + conn.commit(); + } + } + + String upsertStmt = "UPSERT INTO " + tableName + + " (PK1, PK2, PK3, PK4, PK5, PK6, COL1, COL2, COL3) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"; + try (Connection conn = DriverManager.getConnection(getUrl())) { + try (PreparedStatement stmt = conn.prepareStatement(upsertStmt)) { + // Row 1: Both PK5 and PK6 are NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setNull(6, Types.VARCHAR); + stmt.setString(7, "val1"); + stmt.setString(8, "val2"); + stmt.setString(9, "val3"); + stmt.executeUpdate(); + + // Row 2: PK5 is NOT NULL, PK6 is NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setBigDecimal(5, new BigDecimal("1.0")); + stmt.setNull(6, Types.VARCHAR); + stmt.setString(7, "val4"); + stmt.setString(8, "val5"); + stmt.setString(9, "val6"); + stmt.executeUpdate(); + + // Row 3: PK5 is NULL, PK6 is NOT NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setString(6, "P"); + stmt.setString(7, "val7"); + stmt.setString(8, "val8"); + stmt.setString(9, "val9"); + stmt.executeUpdate(); + + // Row 4: Neither PK5 nor PK6 are NULL + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setBigDecimal(5, new BigDecimal("1.0")); + stmt.setString(6, "P"); + stmt.setString(7, "val10"); + stmt.setString(8, "val11"); + stmt.setString(9, "val12"); + stmt.executeUpdate(); + + // Row 5: Both PK5 and PK6 are NULL, different PK4 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Y"); + stmt.setNull(5, Types.DECIMAL); + stmt.setNull(6, Types.VARCHAR); + stmt.setString(7, "val13"); + stmt.setString(8, "val14"); + stmt.setString(9, "val15"); + stmt.executeUpdate(); + + // Row 6: Neither PK5 nor PK6 are NULL, different PK4 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Y"); + stmt.setBigDecimal(5, new BigDecimal("2.0")); + stmt.setString(6, "Q"); + stmt.setString(7, "val16"); + stmt.setString(8, "val17"); + stmt.setString(9, "val18"); + stmt.executeUpdate(); + + // Row 7: Both PK5 and PK6 are NULL, different PK4 + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "Z"); + stmt.setNull(5, Types.DECIMAL); + stmt.setNull(6, Types.VARCHAR); + stmt.setString(7, "val19"); + stmt.setString(8, "val20"); + stmt.setString(9, "val21"); + stmt.executeUpdate(); + + // Row 8: Both PK5 and PK6 are NULL, different PK1 + stmt.setString(1, "C"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + stmt.setNull(5, Types.DECIMAL); + stmt.setNull(6, Types.VARCHAR); + stmt.setString(7, "val22"); + stmt.setString(8, "val23"); + stmt.setString(9, "val24"); + stmt.executeUpdate(); + + conn.commit(); + } + } + return tableName; + } + + /** + * Test for single point lookup with multiple trailing IS NULL PK columns. Uses class-level + * parameters: - columnConfig for PK4 type and sort order - secondarySortDesc for both PK5 and PK6 + * sort order (same sort order for both) Verifies that IS NULL on both trailing nullable PK + * columns generates a single POINT LOOKUP. + */ + @Test + public void testSinglePointLookupWithMultipleTrailingNullablePKs() throws Exception { + String tableName = createTableAndInsertTestDataForMultipleTrailingNullablePKTests(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + String selectSql = "SELECT COL1, COL2, PK4, COL3, PK5, PK6 FROM " + tableName + + " WHERE PK1 = ? AND PK2 = ? AND PK3 = ? " + "AND PK4 = ? AND PK5 IS NULL AND PK6 IS NULL"; + try (PreparedStatement stmt = conn.prepareStatement(selectSql)) { + stmt.setString(1, "A"); + stmt.setString(2, "B"); + stmt.setBigDecimal(3, PK3_VAL); + stmt.setString(4, "X"); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='X' with both PK5 and PK6 IS NULL + assertTrue(rs.next()); + assertEquals("X", rs.getString("PK4")); + assertEquals("val1", rs.getString("COL1")); + assertEquals("val2", rs.getString("COL2")); + assertEquals("val3", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + assertNull(rs.getString("PK6")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on both trailing nullable PK columns generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + + stmt.setString(4, "Y"); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='Y' with both PK5 and PK6 IS NULL + assertTrue(rs.next()); + assertEquals("Y", rs.getString("PK4")); + assertEquals("val13", rs.getString("COL1")); + assertEquals("val14", rs.getString("COL2")); + assertEquals("val15", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + assertNull(rs.getString("PK6")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on both trailing nullable PK columns generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + + stmt.setString(4, "Z"); + try (ResultSet rs = stmt.executeQuery()) { + // Should return 1 row: PK4='Z' with both PK5 and PK6 IS NULL + assertTrue(rs.next()); + assertEquals("Z", rs.getString("PK4")); + assertEquals("val19", rs.getString("COL1")); + assertEquals("val20", rs.getString("COL2")); + assertEquals("val21", rs.getString("COL3")); + assertNull(rs.getBigDecimal("PK5")); + assertNull(rs.getString("PK6")); + + // No more rows + assertFalse(rs.next()); + } + // IS NULL on both trailing nullable PK columns generates single POINT LOOKUP + assertPointLookupsAreGenerated(stmt, 1); + } + } + } } From ab95c308eb407af4e775dcb96d3fe5e78aad39d6 Mon Sep 17 00:00:00 2001 From: Sanjeet Malhotra Date: Fri, 20 Feb 2026 00:56:56 +0530 Subject: [PATCH 15/15] Fix checkstyle --- .../src/main/java/org/apache/phoenix/compile/ScanRanges.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java index 7f7e21ec179..26494730120 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -630,8 +630,8 @@ private static boolean isPointLookup(RowKeySchema schema, List> r // 3. The generated key matches exactly what's stored for rows with trailing NULL // 4. Not handling legcay tables impacted via bug PHOENIX-2067 if ( - !keyRange.isSingleKey() || (lastIndex == i && keyRange == KeyRange.IS_NULL_RANGE - && !schema.rowKeyOrderOptimizable()) + !keyRange.isSingleKey() || lastIndex == i && keyRange == KeyRange.IS_NULL_RANGE + && !schema.rowKeyOrderOptimizable() ) { return false; }