Skip to content

Commit

Permalink
SQ issues fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
luciano-fiandesio committed Jan 17, 2025
1 parent 388d271 commit c1a8dc6
Show file tree
Hide file tree
Showing 10 changed files with 378 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ public class CteDefinition {
// The program stage uid
@Getter private final String programStageUid;
// The program indicator uid
private String programIndicatorUid;
@Getter private String programIndicatorUid;
// The CTE definition (the SQL query)
@Getter private final String cteDefinition;
// The calculated offset
@Getter private final List<Integer> offsets = new ArrayList<>();
// The alias of the CTE
private final String alias;
// Whether the CTE is a row context (TODO this need a better explanation)
// Whether the CTE is a row context
@Getter private boolean rowContext;
// Whether the CTE is a program indicator
@Getter private boolean programIndicator = false;
Expand All @@ -61,9 +61,6 @@ public class CteDefinition {

@Getter private String aggregateWhereClause;

private static final String PS_PREFIX = "ps";
private static final String PI_PREFIX = "pi";

public CteDefinition setExists(boolean exists) {
this.isExists = exists;
return this;
Expand Down Expand Up @@ -109,7 +106,6 @@ public CteDefinition(String cteDefinition, String aggregateWhereClause) {
this(null, null, cteDefinition, 0, false);
this.rowContext = false;
this.aggregationBase = true;
assert aggregateWhereClause != null;
this.aggregateWhereClause = aggregateWhereClause;
}

Expand Down Expand Up @@ -137,24 +133,6 @@ public CteDefinition(
this.filter = isFilter;
}

/**
* @param uid the uid of an dimension item or ProgramIndicator
* @return the name of the CTE
*/
public String asCteName(String uid) {
if (isExists) {
return uid.toLowerCase();
}
if (programIndicator) {
return "%s_%s".formatted(PI_PREFIX, programIndicatorUid.toLowerCase());
}
if (filter) {
return uid.toLowerCase();
}

return "%s_%s_%s".formatted(PS_PREFIX, programStageUid.toLowerCase(), uid.toLowerCase());
}

public boolean isProgramStage() {
return !filter && !programIndicator && !isExists;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ public static String computeKey(QueryItem queryItem) {
} else if (queryItem.isProgramIndicator()) {
return queryItem.getItemId();
}

// TODO continue with the rest of the method
return "";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,9 @@ public static Set<String> getHeaderColumns(List<GridHeader> headers, EventQueryP

if (!headerName.equalsIgnoreCase(COL_VALUE)
&& !headerName.equalsIgnoreCase(PERIOD_DIM_ID)
&& !headerName.equalsIgnoreCase(ORGUNIT_DIM_ID)) {
if (!itemsToSkip.contains(headerName)) {
headerColumns.add(headerName);
}
&& !headerName.equalsIgnoreCase(ORGUNIT_DIM_ID)
&& !itemsToSkip.contains(headerName)) {
headerColumns.add(headerName);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,7 @@ private void buildProgramStageCte(
String colName = quote(item.getItemName());

if (params.isAggregatedEnrollments()) {
handleAggregatedEnrollments(cteContext, item, params, eventTableName, colName);
handleAggregatedEnrollments(cteContext, item, eventTableName, colName);
return;
}

Expand Down Expand Up @@ -1170,11 +1170,7 @@ private String buildMainCteSql(
* @param colName the quoted column name for the item
*/
private void handleAggregatedEnrollments(
CteContext cteContext,
QueryItem item,
EventQueryParams params,
String eventTableName,
String colName) {
CteContext cteContext, QueryItem item, String eventTableName, String colName) {
CteDefinition baseAggregatedCte = cteContext.getBaseAggregatedCte();
assert baseAggregatedCte != null;

Expand Down Expand Up @@ -1225,7 +1221,7 @@ private String buildAggregatedCteSql(
values.put("enrollmentAggrBase", ENROLLMENT_AGGR_BASE);
values.put("programStageUid", item.getProgramStage().getUid());
values.put(
"aggregateWhereClause", baseAggregatedCte.getAggregateWhereClause().replaceAll("%s", "eb"));
"aggregateWhereClause", baseAggregatedCte.getAggregateWhereClause().replace("%s", "eb"));

return new StringSubstitutor(values).replace(template);
}
Expand Down Expand Up @@ -1531,6 +1527,7 @@ private void addSortingAndPaging(SelectBuilder builder, EventQueryParams params)
}
}

@Override
protected String getSortClause(EventQueryParams params) {
if (params.isSorting()) {
return super.getSortClause(params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public void contributeCte(
earliestStartDate,
latestDate)
// FIXME this is a bit of an hack
.replaceAll("subax\\.", "");
.replace("subax\\.", "");
filter = "where " + piResolvedSqlFilter;
}

Expand All @@ -128,7 +128,7 @@ public void contributeCte(
earliestStartDate,
latestDate)
// FIXME this is a bit of an hack
.replaceAll("subax\\.", "");
.replace("subax\\.", "");

String cteSql =
"select enrollment, %s(%s) as value from %s %s group by enrollment"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public SelectBuilder addColumn(String expression, String tablePrefix) {
}

public List<String> getColumnNames() {
return columns.stream().map(Column::expression).collect(Collectors.toList());
return columns.stream().map(Column::expression).toList();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import lombok.experimental.UtilityClass;
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.expression.ExpressionVisitorAdapter;
import net.sf.jsqlparser.expression.Function;
Expand All @@ -40,6 +41,7 @@
import net.sf.jsqlparser.statement.select.SelectExpressionItem;
import net.sf.jsqlparser.statement.select.SubSelect;

@UtilityClass
public class SqlAliasReplacer {

public static String replaceTableAliases(String whereClause, List<String> columns) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@
*/
package org.hisp.dhis.analytics.util.sql;

import lombok.experimental.UtilityClass;
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
import net.sf.jsqlparser.schema.Column;

@UtilityClass
public class SqlColumnParser {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@
import java.util.Set;
import net.sf.jsqlparser.expression.BinaryExpression;
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.expression.Function;
import net.sf.jsqlparser.expression.Parenthesis;
import net.sf.jsqlparser.expression.operators.relational.Between;
import net.sf.jsqlparser.expression.operators.relational.ExpressionList;
import net.sf.jsqlparser.expression.operators.relational.InExpression;
import net.sf.jsqlparser.expression.operators.relational.IsNullExpression;
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
import net.sf.jsqlparser.statement.Statement;
import net.sf.jsqlparser.statement.select.PlainSelect;
Expand Down Expand Up @@ -71,6 +75,13 @@ private static void extractColumnsFromExpression(Expression expression, Set<Stri
if (expression instanceof net.sf.jsqlparser.schema.Column column) {

Check notice

Code scanning / CodeQL

Chain of 'instanceof' tests Note

This if block performs a chain of 7 type tests - consider alternatives, e.g. polymorphism or the visitor pattern.
// Add the column name without table alias to the set
String columnName = column.getColumnName();
// Remove surrounding quotes if present and handle escaped quotes
if (columnName.startsWith("\"") && columnName.endsWith("\"")) {
columnName =
columnName
.substring(1, columnName.length() - 1)
.replace("\"\"", "\""); // Handle escaped quotes
}
columns.add(columnName);
} else if (expression instanceof BinaryExpression binaryExpression) {
// Recursively process left and right expressions
Expand All @@ -82,6 +93,22 @@ private static void extractColumnsFromExpression(Expression expression, Set<Stri
} else if (expression instanceof Parenthesis parenthesis) {
// Process the expression inside parentheses
extractColumnsFromExpression(parenthesis.getExpression(), columns);
} else if (expression instanceof IsNullExpression isNullExpression) {
// Process IS NULL expressions
extractColumnsFromExpression(isNullExpression.getLeftExpression(), columns);
} else if (expression instanceof Function function) {
// Process function parameters to extract column names
ExpressionList parameters = function.getParameters();
if (parameters != null) {
for (Expression parameter : parameters.getExpressions()) {
extractColumnsFromExpression(parameter, columns);
}
}
} else if (expression instanceof Between between) {
// Process BETWEEN expressions
extractColumnsFromExpression(between.getLeftExpression(), columns);
extractColumnsFromExpression(between.getBetweenExpressionStart(), columns);
extractColumnsFromExpression(between.getBetweenExpressionEnd(), columns);
}
}
}
Loading

0 comments on commit c1a8dc6

Please sign in to comment.