stoty commented on code in PR #1586:
URL: https://github.com/apache/phoenix/pull/1586#discussion_r1162919144
##########
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/OrphanViewTool.java:
##########
@@ -500,18 +522,44 @@ else if (!clean){
}
}
}
+
+
private void forcefullyDropView(PhoenixConnection phoenixConnection,
Key key) throws Exception {
- String deleteRowsFromCatalog = "DELETE FROM " + SYSTEM_CATALOG_NAME +
- " WHERE " + TENANT_ID + (key.getTenantId() == null ? " IS
NULL" : " = '" + key.getTenantId() + "'") + " AND " +
- TABLE_SCHEM + (key.getSchemaName() == null ? " IS NULL " : " =
'" + key.getSchemaName() + "'") + " AND " +
- TABLE_NAME + " = '" + key.getTableName() + "'";
- String deleteRowsFromChildLink = "DELETE FROM " +
SYSTEM_CHILD_LINK_NAME +
- " WHERE " + COLUMN_NAME + (key.getTenantId() == null ? " IS
NULL" : " = '" + key.getTenantId() + "'") + " AND " +
- COLUMN_FAMILY + " = '" + (key.getSchemaName() == null ?
key.getTableName() : key.getSchemaName() + "." + key.getTableName()) + "'";
+ String deleteRowsFromCatalog = String.format("DELETE FROM " +
SYSTEM_CATALOG_NAME
+ + " WHERE " + TENANT_ID + " %s AND " + TABLE_SCHEM + " %s
AND "
+ + TABLE_NAME + " = ? ",
+ key.getTenantId() == null ? " IS NULL" : " = ? ",
+ key.getSchemaName() == null ? " IS NULL " : " = ? ");
+ String deleteRowsFromChildLink = String.format("DELETE FROM " +
SYSTEM_CHILD_LINK_NAME
+ + " WHERE " + COLUMN_NAME + " %s AND " + COLUMN_FAMILY + " =
? ",
+ key.getTenantId() == null ? " IS NULL" : " = ? ");
try {
- phoenixConnection.createStatement().execute(deleteRowsFromCatalog);
-
phoenixConnection.createStatement().execute(deleteRowsFromChildLink);
+ try (PreparedStatement delSysCat =
+ phoenixConnection.prepareStatement(deleteRowsFromCatalog)) {
+ int param = 0;
+ if (key.getTenantId() != null) {
+ delSysCat.setString(++param, key.getTenantId());
+ }
+ if (key.getSchemaName() != null) {
+ delSysCat.setString(++param, key.getSchemaName());
+ }
+ delSysCat.setString(++param, key.getTableName());
+ delSysCat.execute();
+ }
+ try (PreparedStatement delChLink =
+ phoenixConnection.prepareStatement(deleteRowsFromChildLink)) {
+ int param = 0;
+ if (key.getTenantId() != null) {
+ delChLink.setString(++param, key.getTenantId());
+ }
+ if (key.getSchemaName() == null) {
+ delChLink.setString(++param, key.getTableName());
Review Comment:
This looks looks too similar to the is null case.
It would be more readable without the if, and using the original tenary
operator for the value instead.
##########
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/OrphanViewTool.java:
##########
@@ -500,18 +522,44 @@ else if (!clean){
}
}
}
+
+
private void forcefullyDropView(PhoenixConnection phoenixConnection,
Key key) throws Exception {
- String deleteRowsFromCatalog = "DELETE FROM " + SYSTEM_CATALOG_NAME +
- " WHERE " + TENANT_ID + (key.getTenantId() == null ? " IS
NULL" : " = '" + key.getTenantId() + "'") + " AND " +
- TABLE_SCHEM + (key.getSchemaName() == null ? " IS NULL " : " =
'" + key.getSchemaName() + "'") + " AND " +
- TABLE_NAME + " = '" + key.getTableName() + "'";
- String deleteRowsFromChildLink = "DELETE FROM " +
SYSTEM_CHILD_LINK_NAME +
- " WHERE " + COLUMN_NAME + (key.getTenantId() == null ? " IS
NULL" : " = '" + key.getTenantId() + "'") + " AND " +
- COLUMN_FAMILY + " = '" + (key.getSchemaName() == null ?
key.getTableName() : key.getSchemaName() + "." + key.getTableName()) + "'";
+ String deleteRowsFromCatalog = String.format("DELETE FROM " +
SYSTEM_CATALOG_NAME
+ + " WHERE " + TENANT_ID + " %s AND " + TABLE_SCHEM + " %s
AND "
+ + TABLE_NAME + " = ? ",
+ key.getTenantId() == null ? " IS NULL" : " = ? ",
+ key.getSchemaName() == null ? " IS NULL " : " = ? ");
+ String deleteRowsFromChildLink = String.format("DELETE FROM " +
SYSTEM_CHILD_LINK_NAME
+ + " WHERE " + COLUMN_NAME + " %s AND " + COLUMN_FAMILY + " =
? ",
+ key.getTenantId() == null ? " IS NULL" : " = ? ");
try {
- phoenixConnection.createStatement().execute(deleteRowsFromCatalog);
-
phoenixConnection.createStatement().execute(deleteRowsFromChildLink);
+ try (PreparedStatement delSysCat =
+ phoenixConnection.prepareStatement(deleteRowsFromCatalog)) {
+ int param = 0;
+ if (key.getTenantId() != null) {
+ delSysCat.setString(++param, key.getTenantId());
+ }
+ if (key.getSchemaName() != null) {
+ delSysCat.setString(++param, key.getSchemaName());
+ }
+ delSysCat.setString(++param, key.getTableName());
+ delSysCat.execute();
+ }
+ try (PreparedStatement delChLink =
+ phoenixConnection.prepareStatement(deleteRowsFromChildLink)) {
+ int param = 0;
+ if (key.getTenantId() != null) {
Review Comment:
These are in the wrong order.
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/transform/Transform.java:
##########
@@ -472,25 +473,41 @@ public static void doCutover(PhoenixConnection
connection, SystemTransformRecord
getMetadataDifference(connection, systemTransformRecord, columnNames,
columnValues);
// TODO In the future, we need to handle rowkey changes and column
type changes as well
- String
- changeViewStmt = "UPSERT INTO SYSTEM.CATALOG (TENANT_ID,
TABLE_SCHEM, TABLE_NAME %s) VALUES (%s, %s, '%s' %s)";
+ String changeViewStmt = "UPSERT INTO SYSTEM.CATALOG "
+ + "(TENANT_ID, TABLE_SCHEM, TABLE_NAME %s) VALUES (?, ?, ? %s)";
String
- changeTable = String.format(
- "UPSERT INTO SYSTEM.CATALOG (TENANT_ID, TABLE_SCHEM,
TABLE_NAME, PHYSICAL_TABLE_NAME %s) VALUES (%s, %s, '%s','%s' %s)",
- (columnNames.size() > 0? "," + String.join(",",
columnNames):""),
- (tenantId==null? null: ("'" + tenantId + "'")),
- (schema==null ? null : ("'" + schema + "'")), tableName,
newTableName,
- (columnValues.size() > 0? "," + String.join(",",
columnValues):""));
+ changeTable = String.format("UPSERT INTO SYSTEM.CATALOG "
+ + "(TENANT_ID, TABLE_SCHEM, TABLE_NAME, PHYSICAL_TABLE_NAME %s
) "
+ + "VALUES(?, ?, ?, ? %s)", columnNames.size() > 0 ? ","
+ + String.join(",", columnNames) : "",
Review Comment:
This is another case, when we should generate ? s and call setString for
each element,
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -1164,19 +1164,23 @@ private long updateStatisticsInternal(PName
physicalName, PTable logicalTable, M
private long updateStatisticsInternal(PName physicalName, PTable
logicalTable, Map<String, Object> statsProps, List<byte[]> cfs, boolean
checkLastStatsUpdateTime) throws SQLException {
ReadOnlyProps props = connection.getQueryServices().getProps();
final long msMinBetweenUpdates = props
- .getLong(QueryServices.MIN_STATS_UPDATE_FREQ_MS_ATTRIB,
QueryServicesOptions.DEFAULT_MIN_STATS_UPDATE_FREQ_MS);
+ .getLong(QueryServices.MIN_STATS_UPDATE_FREQ_MS_ATTRIB,
+ QueryServicesOptions.DEFAULT_MIN_STATS_UPDATE_FREQ_MS);
Long scn = connection.getSCN();
// Always invalidate the cache
long clientTimeStamp = connection.getSCN() == null ?
HConstants.LATEST_TIMESTAMP : scn;
long msSinceLastUpdate = Long.MAX_VALUE;
if (checkLastStatsUpdateTime) {
- String query = "SELECT CURRENT_DATE()," + LAST_STATS_UPDATE_TIME +
" FROM " + PhoenixDatabaseMetaData.SYSTEM_STATS_NAME
- + " WHERE " + PHYSICAL_NAME + "='" +
physicalName.getString() + "' AND " + COLUMN_FAMILY
- + " IS NULL AND " + LAST_STATS_UPDATE_TIME + " IS NOT
NULL";
- ResultSet rs = connection.createStatement().executeQuery(query);
-
- if (rs.next()) {
- msSinceLastUpdate = rs.getLong(1) - rs.getLong(2);
+ String query = "SELECT CURRENT_DATE()," + LAST_STATS_UPDATE_TIME +
" FROM "
+ + PhoenixDatabaseMetaData.SYSTEM_STATS_NAME
+ + " WHERE " + PHYSICAL_NAME + "= ? AND " + COLUMN_FAMILY
+ + " IS NULL AND " + LAST_STATS_UPDATE_TIME + " IS NOT NULL";
+ try (PreparedStatement stmt = connection.prepareStatement(query)) {
Review Comment:
please use a unique name
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/transform/Transform.java:
##########
@@ -472,25 +473,41 @@ public static void doCutover(PhoenixConnection
connection, SystemTransformRecord
getMetadataDifference(connection, systemTransformRecord, columnNames,
columnValues);
// TODO In the future, we need to handle rowkey changes and column
type changes as well
- String
- changeViewStmt = "UPSERT INTO SYSTEM.CATALOG (TENANT_ID,
TABLE_SCHEM, TABLE_NAME %s) VALUES (%s, %s, '%s' %s)";
+ String changeViewStmt = "UPSERT INTO SYSTEM.CATALOG "
+ + "(TENANT_ID, TABLE_SCHEM, TABLE_NAME %s) VALUES (?, ?, ? %s)";
String
- changeTable = String.format(
- "UPSERT INTO SYSTEM.CATALOG (TENANT_ID, TABLE_SCHEM,
TABLE_NAME, PHYSICAL_TABLE_NAME %s) VALUES (%s, %s, '%s','%s' %s)",
- (columnNames.size() > 0? "," + String.join(",",
columnNames):""),
- (tenantId==null? null: ("'" + tenantId + "'")),
- (schema==null ? null : ("'" + schema + "'")), tableName,
newTableName,
- (columnValues.size() > 0? "," + String.join(",",
columnValues):""));
+ changeTable = String.format("UPSERT INTO SYSTEM.CATALOG "
+ + "(TENANT_ID, TABLE_SCHEM, TABLE_NAME, PHYSICAL_TABLE_NAME %s
) "
+ + "VALUES(?, ?, ?, ? %s)", columnNames.size() > 0 ? ","
+ + String.join(",", columnNames) : "",
+ columnNames.size() > 0 ? "," +
QueryUtil.getDynamicParams(columnValues.size()) : "");
LOGGER.info("About to do cutover via " + changeTable);
TableViewFinderResult childViewsResult =
ViewUtil.findChildViews(connection, tenantId, schema, tableName);
boolean wasCommit = connection.getAutoCommit();
connection.setAutoCommit(false);
List<TableInfo> viewsToUpdateCache = new ArrayList<>();
try {
- connection.createStatement().execute(changeTable);
-
+ try (PreparedStatement stmt =
connection.prepareStatement(changeTable)) {
Review Comment:
use a uniqie name
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/transform/Transform.java:
##########
@@ -472,25 +473,41 @@ public static void doCutover(PhoenixConnection
connection, SystemTransformRecord
getMetadataDifference(connection, systemTransformRecord, columnNames,
columnValues);
// TODO In the future, we need to handle rowkey changes and column
type changes as well
- String
- changeViewStmt = "UPSERT INTO SYSTEM.CATALOG (TENANT_ID,
TABLE_SCHEM, TABLE_NAME %s) VALUES (%s, %s, '%s' %s)";
+ String changeViewStmt = "UPSERT INTO SYSTEM.CATALOG "
+ + "(TENANT_ID, TABLE_SCHEM, TABLE_NAME %s) VALUES (?, ?, ? %s)";
String
- changeTable = String.format(
- "UPSERT INTO SYSTEM.CATALOG (TENANT_ID, TABLE_SCHEM,
TABLE_NAME, PHYSICAL_TABLE_NAME %s) VALUES (%s, %s, '%s','%s' %s)",
- (columnNames.size() > 0? "," + String.join(",",
columnNames):""),
- (tenantId==null? null: ("'" + tenantId + "'")),
- (schema==null ? null : ("'" + schema + "'")), tableName,
newTableName,
- (columnValues.size() > 0? "," + String.join(",",
columnValues):""));
+ changeTable = String.format("UPSERT INTO SYSTEM.CATALOG "
+ + "(TENANT_ID, TABLE_SCHEM, TABLE_NAME, PHYSICAL_TABLE_NAME %s
) "
+ + "VALUES(?, ?, ?, ? %s)", columnNames.size() > 0 ? ","
+ + String.join(",", columnNames) : "",
+ columnNames.size() > 0 ? "," +
QueryUtil.getDynamicParams(columnValues.size()) : "");
LOGGER.info("About to do cutover via " + changeTable);
TableViewFinderResult childViewsResult =
ViewUtil.findChildViews(connection, tenantId, schema, tableName);
boolean wasCommit = connection.getAutoCommit();
connection.setAutoCommit(false);
List<TableInfo> viewsToUpdateCache = new ArrayList<>();
try {
- connection.createStatement().execute(changeTable);
-
+ try (PreparedStatement stmt =
connection.prepareStatement(changeTable)) {
+ int param = 0;
+ if (tenantId == null) {
+ stmt.setNull(++param, Types.VARCHAR);
Review Comment:
Interesting.
setString(x, null) doesn't generate a "null" string ?
##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -2163,45 +2210,53 @@ public static void
upgradeDescVarLengthRowKeys(PhoenixConnection conn, List<Stri
* @return true if any upgrades were performed and false otherwise.
*/
private static boolean upgradeSharedIndex(PhoenixConnection upgradeConn,
PhoenixConnection globalConn, String physicalName, boolean bypassUpgrade)
throws SQLException {
- String query =
- "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" +
- "FROM SYSTEM.CATALOG cat1\n" +
- "WHERE COLUMN_NAME IS NULL\n" +
- "AND COLUMN_FAMILY = '" + physicalName + "'\n" +
- "AND LINK_TYPE = " +
LinkType.PHYSICAL_TABLE.getSerializedValue() + "\n" +
- "ORDER BY TENANT_ID";
- ResultSet rs = globalConn.createStatement().executeQuery(query);
- String lastTenantId = null;
- Connection conn = globalConn;
- String url = globalConn.getURL();
- boolean wasUpgraded = false;
- while (rs.next()) {
- String fullTableName = SchemaUtil.getTableName(
+ String query = String.format(
+ "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME"
+ + "FROM SYSTEM.CATALOG cat1"
+ + "WHERE COLUMN_NAME IS NULL"
+ + "AND COLUMN_FAMILY = ? "
+ + "AND LINK_TYPE = ? "
+ + "ORDER BY TENANT_ID", physicalName,
Review Comment:
format parameters not used
##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -1823,6 +1823,25 @@ private ViewKey(String tenantId, String schema, String
viewName) {
}
}
+ private static String getTableRVCWithParam(List<String> tableNames) {
+ StringBuilder query = new StringBuilder("(");
+ for (int i = 0; i < tableNames.size(); i += 3) {
+ String tenantId = tableNames.get(i);
+ String schemaName = tableNames.get(i + 1);
+ String tableName = tableNames.get(i + 2);
+ query.append('(');
+ query.append(tenantId == null ? "null" : " ? ");
+ query.append(',');
+ query.append(schemaName == null ? "null" : " ? ");
+ query.append(',');
+ query.append("'" + tableName + "'");
Review Comment:
Why is this not parametrized ?
##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -2163,45 +2210,53 @@ public static void
upgradeDescVarLengthRowKeys(PhoenixConnection conn, List<Stri
* @return true if any upgrades were performed and false otherwise.
*/
private static boolean upgradeSharedIndex(PhoenixConnection upgradeConn,
PhoenixConnection globalConn, String physicalName, boolean bypassUpgrade)
throws SQLException {
- String query =
- "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" +
- "FROM SYSTEM.CATALOG cat1\n" +
- "WHERE COLUMN_NAME IS NULL\n" +
- "AND COLUMN_FAMILY = '" + physicalName + "'\n" +
- "AND LINK_TYPE = " +
LinkType.PHYSICAL_TABLE.getSerializedValue() + "\n" +
- "ORDER BY TENANT_ID";
- ResultSet rs = globalConn.createStatement().executeQuery(query);
- String lastTenantId = null;
- Connection conn = globalConn;
- String url = globalConn.getURL();
- boolean wasUpgraded = false;
- while (rs.next()) {
- String fullTableName = SchemaUtil.getTableName(
+ String query = String.format(
+ "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME"
+ + "FROM SYSTEM.CATALOG cat1"
+ + "WHERE COLUMN_NAME IS NULL"
+ + "AND COLUMN_FAMILY = ? "
+ + "AND LINK_TYPE = ? "
+ + "ORDER BY TENANT_ID", physicalName,
+ LinkType.PHYSICAL_TABLE.getSerializedValue());
+ try (PreparedStatement selSysCatstmt =
globalConn.prepareStatement(query)) {
+ selSysCatstmt.setString(1, physicalName);
+ selSysCatstmt.setByte(2,
LinkType.PHYSICAL_TABLE.getSerializedValue());
+ ResultSet rs = selSysCatstmt.executeQuery();
+ String lastTenantId = null;
+ Connection conn = globalConn;
+ String url = globalConn.getURL();
+ boolean wasUpgraded = false;
+ while (rs.next()) {
+ String fullTableName = SchemaUtil.getTableName(
rs.getString(PhoenixDatabaseMetaData.TABLE_SCHEM),
rs.getString(PhoenixDatabaseMetaData.TABLE_NAME));
- String tenantId = rs.getString(1);
- if (tenantId != null && !tenantId.equals(lastTenantId)) {
- if (lastTenantId != null) {
- conn.close();
+ String tenantId = rs.getString(1);
+ if (tenantId != null && !tenantId.equals(lastTenantId)) {
+ if (lastTenantId != null) {
+ conn.close();
+ }
+ // Open tenant-specific connection when we find a new one
+ Properties props = new
Properties(globalConn.getClientInfo());
+ props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB,
tenantId);
+ conn = DriverManager.getConnection(url, props);
+ lastTenantId = tenantId;
+ }
+ PTable table = PhoenixRuntime.getTable(conn, fullTableName);
+ String tableTenantId =
+ table.getTenantId() == null ? null :
table.getTenantId().getString();
+ if (Objects.equal(lastTenantId, tableTenantId) &&
!table.rowKeyOrderOptimizable()) {
+ upgradeDescVarLengthRowKeys(upgradeConn, globalConn,
+ table.getSchemaName().getString(),
table.getTableName().getString(), false,
+ bypassUpgrade);
+ wasUpgraded = true;
}
- // Open tenant-specific connection when we find a new one
- Properties props = new Properties(globalConn.getClientInfo());
- props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, tenantId);
- conn = DriverManager.getConnection(url, props);
- lastTenantId = tenantId;
}
- PTable table = PhoenixRuntime.getTable(conn, fullTableName);
- String tableTenantId = table.getTenantId() == null ? null :
table.getTenantId().getString();
- if (Objects.equal(lastTenantId, tableTenantId) &&
!table.rowKeyOrderOptimizable()) {
- upgradeDescVarLengthRowKeys(upgradeConn, globalConn,
table.getSchemaName().getString(), table.getTableName().getString(), false,
bypassUpgrade);
- wasUpgraded = true;
+ rs.close();
+ if (lastTenantId != null) {
+ conn.close();
Review Comment:
Could this be replaced with a try-with-resource ?
##########
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java:
##########
@@ -598,18 +599,27 @@ private Job configureJobForPartialBuild() throws
Exception {
}
private long getMaxRebuildAsyncDate(String schemaName, List<String>
disableIndexes) throws SQLException {
- Long maxRebuilAsyncDate=HConstants.LATEST_TIMESTAMP;
- Long maxDisabledTimeStamp=0L;
- if (disableIndexes == null || disableIndexes.isEmpty()) { return
0; }
+ Long maxRebuilAsyncDate = HConstants.LATEST_TIMESTAMP;
+ Long maxDisabledTimeStamp = 0L;
+ if (disableIndexes == null || disableIndexes.isEmpty()) {
+ return 0;
+ }
List<String> quotedIndexes = new
ArrayList<String>(disableIndexes.size());
for (String index : disableIndexes) {
quotedIndexes.add("'" + index + "'");
}
- try (ResultSet rs = connection.createStatement()
- .executeQuery("SELECT MAX(" + ASYNC_REBUILD_TIMESTAMP +
"),MAX("+INDEX_DISABLE_TIMESTAMP+") FROM " + SYSTEM_CATALOG_NAME + " ("
- + ASYNC_REBUILD_TIMESTAMP + " BIGINT) WHERE " +
TABLE_SCHEM
- + (schemaName != null && schemaName.length() > 0 ?
"='" + schemaName + "'" : " IS NULL")
- + " and " + TABLE_NAME + " IN (" +
StringUtils.join(",", quotedIndexes) + ")")) {
+
+ String query = String.format("SELECT MAX(" +
ASYNC_REBUILD_TIMESTAMP + "), "
+ + "MAX(" + INDEX_DISABLE_TIMESTAMP + ") FROM "
+ + SYSTEM_CATALOG_NAME + " (" + ASYNC_REBUILD_TIMESTAMP
+ + " BIGINT) WHERE " + TABLE_SCHEM + " %s AND " +
TABLE_NAME + " IN ( %s )",
+ (schemaName != null && schemaName.length() > 0) ? " =
? " : " IS NULL ",
+ StringUtils.join(",", quotedIndexes));
Review Comment:
The safe thing to do here is to generate as many ? as there elements in the
list, and call setXXX for each one.
##########
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -3994,21 +3994,36 @@ protected PhoenixConnection
upgradeSystemCatalogIfRequired(PhoenixConnection met
MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_14_0,
PhoenixDatabaseMetaData.TRANSACTION_PROVIDER + " "
+ PTinyint.INSTANCE.getSqlTypeName());
- metaConnection.createStatement().executeUpdate("ALTER TABLE " +
- PhoenixDatabaseMetaData.SYSTEM_CATALOG + " SET " +
- HConstants.VERSIONS + "= " +
props.getInt(DEFAULT_SYSTEM_MAX_VERSIONS_ATTRIB,
QueryServicesOptions.DEFAULT_SYSTEM_MAX_VERSIONS) + ",\n" +
- ColumnFamilyDescriptorBuilder.KEEP_DELETED_CELLS + "=" +
props.getBoolean(DEFAULT_SYSTEM_KEEP_DELETED_CELLS_ATTRIB,
QueryServicesOptions.DEFAULT_SYSTEM_KEEP_DELETED_CELLS)
- );
- metaConnection.createStatement().executeUpdate("ALTER TABLE " +
- PhoenixDatabaseMetaData.SYSTEM_FUNCTION + " SET " +
- TableDescriptorBuilder.SPLIT_POLICY + "='" +
SystemFunctionSplitPolicy.class.getName() + "',\n" +
- HConstants.VERSIONS + "= " +
props.getInt(DEFAULT_SYSTEM_MAX_VERSIONS_ATTRIB,
QueryServicesOptions.DEFAULT_SYSTEM_MAX_VERSIONS) + ",\n" +
- ColumnFamilyDescriptorBuilder.KEEP_DELETED_CELLS + "=" +
props.getBoolean(DEFAULT_SYSTEM_KEEP_DELETED_CELLS_ATTRIB,
QueryServicesOptions.DEFAULT_SYSTEM_KEEP_DELETED_CELLS)
- );
- metaConnection.createStatement().executeUpdate("ALTER TABLE " +
- PhoenixDatabaseMetaData.SYSTEM_STATS_NAME + " SET " +
- TableDescriptorBuilder.SPLIT_POLICY + "='" +
SystemStatsSplitPolicy.class.getName() +"'"
- );
+ String altCatQry = "ALTER TABLE "
+ + PhoenixDatabaseMetaData.SYSTEM_CATALOG + " SET "
+ + HConstants.VERSIONS + "= " +
props.getInt(DEFAULT_SYSTEM_MAX_VERSIONS_ATTRIB,
+ QueryServicesOptions.DEFAULT_SYSTEM_MAX_VERSIONS) + ",\n"
+ + ColumnFamilyDescriptorBuilder.KEEP_DELETED_CELLS + "="
+ + props.getBoolean(DEFAULT_SYSTEM_KEEP_DELETED_CELLS_ATTRIB,
+ QueryServicesOptions.DEFAULT_SYSTEM_KEEP_DELETED_CELLS);
+ try (PreparedStatement altCatQryStmt =
metaConnection.prepareStatement(altCatQry)) {
Review Comment:
No need for PreparedStatements if we don't set any parameters.
We could just put each query in a try-with-resources block that creates a
(non prepared) metaStmt, to make sure that we don't let it hanging.
Just be careful to always re-create metaStmt when replace metaConnection.
##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -1823,6 +1823,25 @@ private ViewKey(String tenantId, String schema, String
viewName) {
}
}
+ private static String getTableRVCWithParam(List<String> tableNames) {
+ StringBuilder query = new StringBuilder("(");
+ for (int i = 0; i < tableNames.size(); i += 3) {
+ String tenantId = tableNames.get(i);
+ String schemaName = tableNames.get(i + 1);
+ String tableName = tableNames.get(i + 2);
+ query.append('(');
+ query.append(tenantId == null ? "null" : " ? ");
Review Comment:
wouldn't setNull work here ?
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -4482,8 +4486,9 @@ private String dropColumnMutations(PTable table,
List<PColumn> columnsToDrop) th
}
buf.setCharAt(buf.length()-1, ')');
- connection.createStatement().execute(buf.toString());
-
+ try (PreparedStatement delCol =
connection.prepareStatement(buf.toString())) {
Review Comment:
no need to prepare if we don't use parameters.
##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -2031,16 +2067,27 @@ private static void
upgradeDescVarLengthRowKeys(PhoenixConnection upgradeConn, P
String theTenantId = tableNames.get(i);
String theSchemaName = tableNames.get(i+1);
String theTableName = tableNames.get(i+2);
- globalConn.createStatement().execute("UPSERT INTO " +
PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME +
- " (" + PhoenixDatabaseMetaData.TENANT_ID + "," +
- PhoenixDatabaseMetaData.TABLE_SCHEM + "," +
- PhoenixDatabaseMetaData.TABLE_NAME + "," +
- MetaDataEndpointImpl.ROW_KEY_ORDER_OPTIMIZABLE + "
BOOLEAN"
- + ") VALUES (" +
- "'" + (theTenantId == null ?
StringUtil.EMPTY_STRING : theTenantId) + "'," +
- "'" + (theSchemaName == null ?
StringUtil.EMPTY_STRING : theSchemaName) + "'," +
- "'" + theTableName + "'," +
- "TRUE)");
+ String upsSyscat = String.format("UPSERT INTO "
+ + PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME
+ + " (" + PhoenixDatabaseMetaData.TENANT_ID + ","
+ + PhoenixDatabaseMetaData.TABLE_SCHEM + ","
+ + PhoenixDatabaseMetaData.TABLE_NAME + ","
+ + MetaDataEndpointImpl.ROW_KEY_ORDER_OPTIMIZABLE + "
BOOLEAN"
+ + ") VALUES ( ?, ?," + "'" + theTableName + "'," +
"TRUE)");
Review Comment:
Why is this not parametrized ?
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/transform/Transform.java:
##########
@@ -520,13 +537,28 @@ public static void doCutover(PhoenixConnection
connection, SystemTransformRecord
int batchSize = 0;
for (TableInfo view : childViewsResult.getLinks()) {
String changeView = String.format(changeViewStmt,
- (columnNames.size() > 0? "," + String.join(",",
columnNames):""),
- (view.getTenantId()==null || view.getTenantId().length
== 0? null: ("'" + Bytes.toString(view.getTenantId()) + "'")),
- (view.getSchemaName()==null ||
view.getSchemaName().length == 0? null : ("'" +
Bytes.toString(view.getSchemaName()) + "'")),
- Bytes.toString(view.getTableName()),
- (columnValues.size() > 0? "," + String.join(",",
columnValues):""));
+ columnNames.size() > 0 ? "," + String.join(",",
columnNames) : "",
Review Comment:
Same list handling as before.
(I think we may already have some helper functions for that)
##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -2563,13 +2618,17 @@ private static void
updateIndexesSequenceIfPresent(PhoenixConnection connection,
String newSchemaName =
MetaDataUtil.getViewIndexSequenceSchemaName(physicalName, true);
String newSequenceName =
MetaDataUtil.getViewIndexSequenceName(physicalName, tenantId, true);
// create new entry with new schema format
- String upsert = "UPSERT INTO " +
PhoenixDatabaseMetaData.SYSTEM_SEQUENCE + " SELECT NULL,\'" + newSchemaName +
- "\',\'" + newSequenceName
- + "\'," + START_WITH + "," + CURRENT_VALUE + "," +
INCREMENT_BY + "," + CACHE_SIZE + "," + MIN_VALUE
- + "," + MAX_VALUE + "," + CYCLE_FLAG + "," +
LIMIT_REACHED_FLAG + " FROM "
- + PhoenixDatabaseMetaData.SYSTEM_SEQUENCE + " WHERE " +
PhoenixDatabaseMetaData.TENANT_ID
- + " IS NULL AND " + PhoenixDatabaseMetaData.SEQUENCE_SCHEMA +
" = '" + oldSchemaName + "'";
- connection.createStatement().executeUpdate(upsert);
+ String upsert = "UPSERT INTO " +
PhoenixDatabaseMetaData.SYSTEM_SEQUENCE
+ + " SELECT NULL,\'" + newSchemaName + "\',\'" + newSequenceName
Review Comment:
shouldn't these be parametrized ?
--
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]
