[
https://issues.apache.org/jira/browse/PHOENIX-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17711330#comment-17711330
]
ASF GitHub Bot commented on PHOENIX-6560:
-----------------------------------------
stoty commented on code in PR #1586:
URL: https://github.com/apache/phoenix/pull/1586#discussion_r1164002752
##########
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:
Thanks.
I don't quite get what is happening with conn/globalConn here, but you are
not chaning that part, so it's fine to leave it as it is.
> Rewrite dynamic SQL queries to use Preparedstatement
> ----------------------------------------------------
>
> Key: PHOENIX-6560
> URL: https://issues.apache.org/jira/browse/PHOENIX-6560
> Project: Phoenix
> Issue Type: Improvement
> Components: core
> Reporter: Istvan Toth
> Assignee: Abhishek Kothalikar
> Priority: Major
>
> Most of the Phoenix code base already uses PreparedStatements, and adds all
> potentially vulnerable data as parameters.
> However, there are some places where we concatenate potentially problematic
> strings into the query.
> While most of those are constants and such, we should preferably pass all
> data as parameters to be on the safe side.
> (We still have to use dynamic strings for the preparedstatement strings, for
> handling things as is null, empty in clauses and such)
> Spotbugs marks these with SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE, so
> they're easy to find.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
