shahrs87 commented on code in PR #1666:
URL: https://github.com/apache/phoenix/pull/1666#discussion_r1357251653


##########
phoenix-core/src/test/java/org/apache/phoenix/cache/ServerMetadataCacheTest.java:
##########
@@ -678,6 +677,139 @@ public void testSelectQueryOnSystemTables() throws 
Exception {
         }
     }
 
+    /**
+     * Test query on index with stale last ddl timestamp.
+     * Client-1 creates a table and an index on it. Client-2 queries table 
(with index hint) to populate its cache.
+     * Client-1 alters a property on the index. Client-2 queries the table 
again.
+     * Verify that the second query works and the index metadata was updated 
in the client cache.
+     */
+    @Test
+    public void testSelectQueryAfterAlterIndex() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String url1 = QueryUtil.getConnectionUrl(props, config, "client1");
+        String url2 = QueryUtil.getConnectionUrl(props, config, "client2");
+        String tableName = generateUniqueName();
+        String indexName = generateUniqueName();
+        ConnectionQueryServices spyCqs1 = 
Mockito.spy(driver.getConnectionQueryServices(url1, props));
+        ConnectionQueryServices spyCqs2 = 
Mockito.spy(driver.getConnectionQueryServices(url2, props));
+
+        try (Connection conn1 = spyCqs1.connect(url1, props);
+             Connection conn2 = spyCqs2.connect(url2, props)) {
+
+            //client-1 creates a table and an index on it
+            createTable(conn1, tableName, NEVER);
+            createIndex(conn1, tableName, indexName, "v1");
+            TestUtil.waitForIndexState(conn1, indexName, PIndexState.ACTIVE);
+
+            //client-2 populates its cache, 1 getTable&addTable call for the 
table
+            //no getTable calls for Index since we add all indexes in the 
cache from a table's PTable object
+            queryWithNoIndexHint(conn2, tableName);
+
+            //client-1 updates index property
+            alterIndexChangeStateToRebuild(conn1, tableName, indexName);
+
+            //client-2's query using the index should work
+            queryWithIndexHint(conn2, tableName, indexName);
+
+            //verify client-2 cache was updated with the index's base table 
metadata
+            //this would have also updated the index metadata in its cache
+            Mockito.verify(spyCqs2, Mockito.times(2)).getTable(eq(null),
+                    any(byte[].class), 
eq(PVarchar.INSTANCE.toBytes(tableName)),
+                    anyLong(), anyLong());
+            Mockito.verify(spyCqs2, Mockito.times(2))
+                    .addTable(any(PTable.class), anyLong());
+
+            //client-2 queries again with latest metadata
+            //verify no more getTable/addTable calls
+            queryWithIndexHint(conn2, tableName, indexName);
+            Mockito.verify(spyCqs2, Mockito.times(2)).getTable(eq(null),
+                    any(byte[].class), 
eq(PVarchar.INSTANCE.toBytes(tableName)),
+                    anyLong(), anyLong());
+            Mockito.verify(spyCqs2, Mockito.times(2))
+                    .addTable(any(PTable.class), anyLong());
+        }
+    }
+
+    /**
+     * Test that a client can learn about a newly created index.
+     * Client-1 creates a table, client-2 queries the table to populate its 
cache.
+     * Client-1 creates an index on the table. Client-2 queries the table 
using the index.
+     * Verify that client-2 uses the index for the query.
+     */
+    @Test
+    public void testSelectQueryAddIndex() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String url1 = QueryUtil.getConnectionUrl(props, config, "client1");
+        String url2 = QueryUtil.getConnectionUrl(props, config, "client2");
+        String tableName = generateUniqueName();
+        String indexName = generateUniqueName();
+        ConnectionQueryServices spyCqs1 = 
Mockito.spy(driver.getConnectionQueryServices(url1, props));
+        ConnectionQueryServices spyCqs2 = 
Mockito.spy(driver.getConnectionQueryServices(url2, props));
+
+        try (Connection conn1 = spyCqs1.connect(url1, props);
+             Connection conn2 = spyCqs2.connect(url2, props)) {
+
+            //client-1 creates table
+            createTable(conn1, tableName, NEVER);
+
+            //client-2 populates its cache
+            query(conn2, tableName);
+
+            //client-1 creates an index on the table
+            createIndex(conn1, tableName, indexName, "v1");
+            TestUtil.waitForIndexState(conn1, indexName, PIndexState.ACTIVE);
+
+            //simulate changing table's ddl timestamp by altering it for now
+            //TODO: remove this when PHOENIX-7056 is committed.
+            alterTableAddColumn(conn1, tableName, "col1");
+
+            //client-2 query should be able to use this index
+            PhoenixStatement stmt = 
conn2.createStatement().unwrap(PhoenixStatement.class);
+            ResultSet rs = stmt.executeQuery("SELECT /*+ INDEX(" + tableName + 
" " + indexName + ") */ * FROM " + tableName + " WHERE v1=1");
+            Assert.assertEquals("", indexName, 
stmt.getQueryPlan().getContext().getCurrentTable().getTable().getName().getString());
+        }
+    }
+
+    /**
+     * Test that a client can learn about a dropped index.
+     * Client-1 creates a table and an index, client-2 queries the table to 
populate its cache.
+     * Client-1 drops the index. Client-2 queries the table with index hint.
+     * Verify that client-2 uses the data table for the query.
+     */
+    @Test
+    public void testSelectQueryDropIndex() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String url1 = QueryUtil.getConnectionUrl(props, config, "client1");
+        String url2 = QueryUtil.getConnectionUrl(props, config, "client2");
+        String tableName = generateUniqueName();
+        String indexName = generateUniqueName();
+        ConnectionQueryServices spyCqs1 = 
Mockito.spy(driver.getConnectionQueryServices(url1, props));
+        ConnectionQueryServices spyCqs2 = 
Mockito.spy(driver.getConnectionQueryServices(url2, props));
+
+        try (Connection conn1 = spyCqs1.connect(url1, props);
+             Connection conn2 = spyCqs2.connect(url2, props)) {
+
+            //client-1 creates table and index on it
+            createTable(conn1, tableName, NEVER);
+            createIndex(conn1, tableName, indexName, "v1");
+
+            //client-2 populates its cache
+            query(conn2, tableName);
+
+            //client-1 drops the index
+            dropIndex(conn1, tableName, indexName);
+
+            //simulate changing table's ddl timestamp by altering it for now
+            //TODO: remove this when PHOENIX-7056 is committed.

Review Comment:
   @palashc  PHOENIX-7056 is committed now. Can you rebase this PR?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to