shahrs87 commented on code in PR #1859:
URL: https://github.com/apache/phoenix/pull/1859#discussion_r1556441800
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpdateCacheAcrossDifferentClientsIT.java:
##########
@@ -104,17 +105,19 @@ public void testUpdateCacheFrequencyWithAddAndDropTable()
throws Exception {
} catch (TableNotFoundException e) {
//Expected
}
- rs = conn2.createStatement().executeQuery("select * from
"+tableName);
try {
+ rs = conn2.createStatement().executeQuery("select * from
"+tableName);
rs.next();
fail("Should throw
org.apache.hadoop.hbase.TableNotFoundException since the latest metadata " +
"wasn't fetched");
- } catch (PhoenixIOException ex) {
Review Comment:
Why is this change required?
##########
phoenix-core-client/src/main/java/org/apache/phoenix/util/ValidateLastDDLTimestampUtil.java:
##########
@@ -67,6 +67,7 @@ public static String getInfoString(PName tenantId,
List<TableRef> tableRefs) {
/**
* Get whether last ddl timestamp validation is enabled on the connection
+ *
Review Comment:
nit: remove extra line.
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpgradeIT.java:
##########
@@ -731,13 +730,14 @@ public void testLastDDLTimestampBootstrap() throws
Exception {
}
private void nullDDLTimestamps(Connection conn) throws SQLException {
+ //ignore system tables since that can interfere with other tests.
String pkCols = TENANT_ID + ", " + TABLE_SCHEM +
", " + TABLE_NAME + ", " + COLUMN_NAME + ", " + COLUMN_FAMILY;
String upsertSql =
"UPSERT INTO " + SYSTEM_CATALOG_NAME + " (" + pkCols + ", " +
LAST_DDL_TIMESTAMP + ")" + " " +
"SELECT " + pkCols + ", NULL FROM " + SYSTEM_CATALOG_NAME + "
" +
- "WHERE " + TABLE_TYPE + " IS NOT NULL";
+ "WHERE " + TABLE_TYPE + " " + " != '" +
PTableType.SYSTEM.getSerializedValue() + "'";
Review Comment:
why is this change required?
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java:
##########
@@ -660,6 +661,14 @@ public void testCreateChildViewWithBaseTableIndex(boolean
localIndex)
fail();
} catch (Exception ignore) {
}
+
+ PTable view = PhoenixRuntime.getTable(conn, fullViewName);
Review Comment:
why is this change required?
##########
phoenix-core-client/src/main/java/org/apache/phoenix/util/ValidateLastDDLTimestampUtil.java:
##########
@@ -172,6 +173,8 @@ public static void
validateLastDDLTimestamp(PhoenixConnection conn,
requestBuilder.addLastDDLTimestampRequests(innerBuilder);
}
}
+
Review Comment:
nit: remove extra couple of lines.
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexFailureIT.java:
##########
@@ -176,6 +178,7 @@ protected static Map<String,String> getServerProps(){
* because we want to control it's execution ourselves
*/
serverProps.put(QueryServices.INDEX_REBUILD_TASK_INITIAL_DELAY,
Long.toString(Long.MAX_VALUE));
+ serverProps.put(REGIONSERVER_COPROCESSOR_CONF_KEY,
PhoenixRegionServerEndpoint.class.getName());
Review Comment:
Why are we setting `REGIONSERVER_COPROCESSOR_CONF_KEY` to
`PhoenixRegionServerEndpoint` and not `PhoenixRegionServerEndpointTestImpl` ?
--
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]
