[
https://issues.apache.org/jira/browse/PHOENIX-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17714442#comment-17714442
]
ASF GitHub Bot commented on PHOENIX-6560:
-----------------------------------------
stoty commented on code in PR #1586:
URL: https://github.com/apache/phoenix/pull/1586#discussion_r1172165997
##########
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java:
##########
@@ -604,24 +604,19 @@ private long getMaxRebuildAsyncDate(String schemaName,
List<String> disableIndex
if (disableIndexes == null || disableIndexes.isEmpty()) {
return 0;
}
- List<String> quotedIndexes = new
ArrayList<String>(disableIndexes.size());
- for (String index : disableIndexes) {
- quotedIndexes.add("'" + index + "'");
- }
-
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 ",
- QueryUtil.generateInListParams(quotedIndexes.size()));
+ QueryUtil.generateInListParams(disableIndexes.size()));
try (PreparedStatement selSyscat =
connection.prepareStatement(query)) {
int param = 0;
if (schemaName != null && schemaName.length() > 0) {
selSyscat.setString(++param, schemaName);
}
- for (int i = 0; i < quotedIndexes.size(); i++) {
- selSyscat.setString(++param, quotedIndexes.get(i));
+ for (int i = 0; i < disableIndexes.size(); i++) {
Review Comment:
While this works, it would be much cleaner if we pushed this whole for loop
into the utility function.
> 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)
