Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public DefaultProgramIndicatorService(
this.analyticsSqlCache = cacheProvider.createAnalyticsSqlCache();
this.sqlBuilder = sqlBuilder;

this.programIndicatorItems = new ExpressionMapBuilder(sqlBuilder).getExpressionItemMap();
this.programIndicatorItems = new ExpressionMapBuilder().getExpressionItemMap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m curious — was there a particular reason we opted not to use dependency injection in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of a long story, in reality this new ExpressionMapBuilder is no longer needed, because @jimgrace showed me a better/simpler way to access the SqlBuilder. But since it was there, I figured that we can keep the list of expressions encapsulated into its own class, and yes, this could use DoI. Not sure, shall I just get rid of ExpressionMapBuilder altogether?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK. In DefaultExpressionService these maps are still generated in the service (for all the expression types that are not program indicators), so this is not consistent with that. But all this ANLR code will be ripped out when we transition to Jan B's new parser. (Did you know about that?) I'm OK with keeping it as a separate class or putting it back where it was.

}

// -------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class ExpressionMapBuilder {

private final ImmutableMap<Integer, ExpressionItem> expressionItemMap;

public ExpressionMapBuilder(SqlBuilder sqlBuilder) {
public ExpressionMapBuilder() {
expressionItemMap =
ImmutableMap.<Integer, ExpressionItem>builder()

Expand All @@ -82,22 +82,22 @@ public ExpressionMapBuilder(SqlBuilder sqlBuilder) {

// Program functions

.put(D2_CONDITION, new D2Condition().withSqlBuilder(sqlBuilder))
.put(D2_COUNT, new D2Count().withSqlBuilder(sqlBuilder))
.put(D2_COUNT_IF_CONDITION, new D2CountIfCondition().withSqlBuilder(sqlBuilder))
.put(D2_COUNT_IF_VALUE, new D2CountIfValue().withSqlBuilder(sqlBuilder))
.put(D2_DAYS_BETWEEN, new D2DaysBetween().withSqlBuilder(sqlBuilder))
.put(D2_HAS_VALUE, new D2HasValue().withSqlBuilder(sqlBuilder))
.put(D2_MAX_VALUE, new D2MaxValue().withSqlBuilder(sqlBuilder))
.put(D2_MINUTES_BETWEEN, new D2MinutesBetween().withSqlBuilder(sqlBuilder))
.put(D2_MIN_VALUE, new D2MinValue().withSqlBuilder(sqlBuilder))
.put(D2_MONTHS_BETWEEN, new D2MonthsBetween().withSqlBuilder(sqlBuilder))
.put(D2_OIZP, new D2Oizp().withSqlBuilder(sqlBuilder))
.put(D2_RELATIONSHIP_COUNT, new D2RelationshipCount().withSqlBuilder(sqlBuilder))
.put(D2_WEEKS_BETWEEN, new D2WeeksBetween().withSqlBuilder(sqlBuilder))
.put(D2_YEARS_BETWEEN, new D2YearsBetween().withSqlBuilder(sqlBuilder))
.put(D2_ZING, new D2Zing().withSqlBuilder(sqlBuilder))
.put(D2_ZPVC, new D2Zpvc().withSqlBuilder(sqlBuilder))
.put(D2_CONDITION, new D2Condition())
.put(D2_COUNT, new D2Count())
.put(D2_COUNT_IF_CONDITION, new D2CountIfCondition())
.put(D2_COUNT_IF_VALUE, new D2CountIfValue())
.put(D2_DAYS_BETWEEN, new D2DaysBetween())
.put(D2_HAS_VALUE, new D2HasValue())
.put(D2_MAX_VALUE, new D2MaxValue())
.put(D2_MINUTES_BETWEEN, new D2MinutesBetween())
.put(D2_MIN_VALUE, new D2MinValue())
.put(D2_MONTHS_BETWEEN, new D2MonthsBetween())
.put(D2_OIZP, new D2Oizp())
.put(D2_RELATIONSHIP_COUNT, new D2RelationshipCount())
.put(D2_WEEKS_BETWEEN, new D2WeeksBetween())
.put(D2_YEARS_BETWEEN, new D2YearsBetween())
.put(D2_ZING, new D2Zing())
.put(D2_ZPVC, new D2Zpvc())

// Program functions for custom aggregation

Expand All @@ -111,13 +111,13 @@ public ExpressionMapBuilder(SqlBuilder sqlBuilder) {

// Data items

.put(HASH_BRACE, new ProgramItemStageElement().withSqlBuilder(sqlBuilder))
.put(A_BRACE, new ProgramItemAttribute().withSqlBuilder(sqlBuilder))
.put(PS_EVENTDATE, new ProgramItemPsEventdate().withSqlBuilder(sqlBuilder))
.put(HASH_BRACE, new ProgramItemStageElement())
.put(A_BRACE, new ProgramItemAttribute())
.put(PS_EVENTDATE, new ProgramItemPsEventdate())

// Program variables

.put(V_BRACE, new ProgramVariableItem().withSqlBuilder(sqlBuilder))
.put(V_BRACE, new ProgramVariableItem())

// . functions
.put(STAGE_OFFSET, new RepeatableProgramStageOffset())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.hisp.dhis.analytics.DataType;
import org.hisp.dhis.antlr.ParserExceptionWithoutContext;
import org.hisp.dhis.common.ValueType;
import org.hisp.dhis.db.sql.SqlBuilder;
import org.hisp.dhis.parser.expression.CommonExpressionVisitor;
import org.hisp.dhis.parser.expression.ExpressionItem;
import org.hisp.dhis.program.dataitem.ProgramItemAttribute;
Expand All @@ -58,8 +57,6 @@
*/
public abstract class ProgramExpressionItem implements ExpressionItem {

private SqlBuilder sqlBuilder;

@Override
public final Object getExpressionInfo(ExprContext ctx, CommonExpressionVisitor visitor) {
throw new ParserExceptionWithoutContext(
Expand Down Expand Up @@ -124,11 +121,6 @@ protected String replaceNullSqlValues(
if (dataType == NUMERIC || dataType == BOOLEAN) {
dataType = visitor.getParams().getDataType() == BOOLEAN ? BOOLEAN : NUMERIC;
}
return replaceSqlNull(sqlBuilder.cast(column, dataType), dataType);
}

protected ExpressionItem withSqlBuilder(SqlBuilder sqlBuilder) {
this.sqlBuilder = sqlBuilder;
return this;
return replaceSqlNull(visitor.getSqlBuilder().cast(column, dataType), dataType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,18 @@
*/
package org.hisp.dhis.program.function;

import org.hisp.dhis.db.sql.SqlBuilder;
import org.hisp.dhis.parser.expression.CommonExpressionVisitor;

/**
* Program indicator function: d2 days between
*
* @author Jim Grace
*/
public class D2DaysBetween extends ProgramBetweenFunction {
@Override
public Object getSqlBetweenDates(String startDate, String endDate) {
return "(cast(" + endDate + " as date) - cast(" + startDate + " as date))";
public Object getSqlBetweenDates(
String startDate, String endDate, CommonExpressionVisitor visitor) {
return visitor.getSqlBuilder().dateDifference(startDate, endDate, SqlBuilder.DatePart.DAYS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,19 @@
*/
package org.hisp.dhis.program.function;

import static org.hisp.dhis.db.sql.SqlBuilder.DatePart.MINUTES;

import org.hisp.dhis.parser.expression.CommonExpressionVisitor;

/**
* Program indicator function: d2 minutes between
*
* @author Jim Grace
*/
public class D2MinutesBetween extends ProgramBetweenFunction {
@Override
public Object getSqlBetweenDates(String startDate, String endDate) {
return "(extract(epoch from (cast("
+ endDate
+ " as timestamp) - cast("
+ startDate
+ " as timestamp))) / 60)";
public Object getSqlBetweenDates(
String startDate, String endDate, CommonExpressionVisitor visitor) {
return visitor.getSqlBuilder().dateDifference(startDate, endDate, MINUTES);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,19 @@
*/
package org.hisp.dhis.program.function;

import static org.hisp.dhis.db.sql.SqlBuilder.DatePart.MONTHS;

import org.hisp.dhis.parser.expression.CommonExpressionVisitor;

/**
* Program indicator function: d2 months between
*
* @author Jim Grace
*/
public class D2MonthsBetween extends ProgramBetweenFunction {
@Override
public Object getSqlBetweenDates(String startDate, String endDate) {
return "((date_part('year',age(cast("
+ endDate
+ " as date), cast("
+ startDate
+ " as date)))) * 12 + "
+ "date_part('month',age(cast("
+ endDate
+ " as date), cast("
+ startDate
+ " as date))))";
public Object getSqlBetweenDates(
String startDate, String endDate, CommonExpressionVisitor visitor) {
return visitor.getSqlBuilder().dateDifference(startDate, endDate, MONTHS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,19 @@
*/
package org.hisp.dhis.program.function;

import static org.hisp.dhis.db.sql.SqlBuilder.DatePart.WEEKS;

import org.hisp.dhis.parser.expression.CommonExpressionVisitor;

/**
* Program indicator function: d2 weeks between
*
* @author Jim Grace
*/
public class D2WeeksBetween extends ProgramBetweenFunction {
@Override
public Object getSqlBetweenDates(String startDate, String endDate) {
return "((cast(" + endDate + " as date) - cast(" + startDate + " as date))/7)";
public Object getSqlBetweenDates(
String startDate, String endDate, CommonExpressionVisitor visitor) {
return visitor.getSqlBuilder().dateDifference(startDate, endDate, WEEKS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,19 @@
*/
package org.hisp.dhis.program.function;

import static org.hisp.dhis.db.sql.SqlBuilder.DatePart.YEARS;

import org.hisp.dhis.parser.expression.CommonExpressionVisitor;

/**
* Program indicator function: d2 years between
*
* @author Jim Grace
*/
public class D2YearsBetween extends ProgramBetweenFunction {
@Override
public Object getSqlBetweenDates(String startDate, String endDate) {
return "(date_part('year',age(cast("
+ endDate
+ " as date), cast("
+ startDate
+ " as date))))";
public Object getSqlBetweenDates(
String startDate, String endDate, CommonExpressionVisitor visitor) {
return visitor.getSqlBuilder().dateDifference(startDate, endDate, YEARS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public final Object getSql(ExprContext ctx, CommonExpressionVisitor visitor) {
String startDate = castString(visitor.visitAllowingNulls(ctx.expr(0)));
String endDate = castString(visitor.visitAllowingNulls(ctx.expr(1)));

return getSqlBetweenDates(startDate, endDate);
return getSqlBetweenDates(startDate, endDate, visitor);
}

/**
Expand All @@ -64,5 +64,6 @@ public final Object getSql(ExprContext ctx, CommonExpressionVisitor visitor) {
* @param endDate ending date
* @return the SQL to compare the dates based on the time/date unit
*/
public abstract Object getSqlBetweenDates(String startDate, String endDate);
public abstract Object getSqlBetweenDates(
String startDate, String endDate, CommonExpressionVisitor visitor);
}
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ private Object test(
.programIndicatorService(programIndicatorService)
.programStageService(programStageService)
.i18nSupplier(() -> new I18n(null, null))
.itemMap(new ExpressionMapBuilder(sqlBuilder).getExpressionItemMap())
.itemMap(new ExpressionMapBuilder().getExpressionItemMap())
.itemMethod(itemMethod)
.params(params)
.progParams(progParams)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private Object test(
.programStageService(programStageService)
.i18nSupplier(() -> new I18n(null, null))
.constantMap(constantMap)
.itemMap(new ExpressionMapBuilder(sqlBuilder).getExpressionItemMap())
.itemMap(new ExpressionMapBuilder().getExpressionItemMap())
.itemMethod(itemMethod)
.params(params)
.progParams(progParams)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ private Object test(
.programIndicatorService(programIndicatorService)
.programStageService(programStageService)
.i18nSupplier(() -> new I18n(null, null))
.itemMap(new ExpressionMapBuilder(sqlBuilder).getExpressionItemMap())
.itemMap(new ExpressionMapBuilder().getExpressionItemMap())
.itemMethod(ITEM_GET_SQL)
.params(params)
.progParams(progParams)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright (c) 2004-2024, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
* Neither the name of the HISP project nor the names of its contributors may
* be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.program.function;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.when;

import org.hisp.dhis.db.sql.DorisSqlBuilder;
import org.hisp.dhis.db.sql.PostgreSqlBuilder;
import org.hisp.dhis.db.sql.SqlBuilder;
import org.hisp.dhis.parser.expression.CommonExpressionVisitor;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith({MockitoExtension.class})
class D2DaysBetweenTest {

@Mock private CommonExpressionVisitor visitor;

@Nested
class PostgresTests {
private D2DaysBetween function;
private SqlBuilder sqlBuilder;

@BeforeEach
void setUp() {
function = new D2DaysBetween();
sqlBuilder = new PostgreSqlBuilder();
when(visitor.getSqlBuilder()).thenReturn(sqlBuilder);
}

@Test
void shouldGenerateSqlForLiteralDates() {
String startDate = "'2023-01-01'";
String endDate = "'2023-01-02'";

String result = (String) function.getSqlBetweenDates(startDate, endDate, visitor);

assertEquals("(cast('2023-01-02' as date) - cast('2023-01-01' as date))", result);
}

@Test
void shouldGenerateSqlForColumnReferences() {
String startDate = "enrollment_date";
String endDate = "incident_date";

String result = (String) function.getSqlBetweenDates(startDate, endDate, visitor);

assertEquals("(cast(incident_date as date) - cast(enrollment_date as date))", result);
}
}

@Nested
class DorisTests {
private D2DaysBetween function;
private SqlBuilder sqlBuilder;

@BeforeEach
void setUp() {
function = new D2DaysBetween();
sqlBuilder = new DorisSqlBuilder("doris", "doris");
when(visitor.getSqlBuilder()).thenReturn(sqlBuilder);
}

@Test
void shouldGenerateSqlForLiteralDates() {
String startDate = "'2023-01-01'";
String endDate = "'2023-01-02'";

String result = (String) function.getSqlBetweenDates(startDate, endDate, visitor);

assertEquals("DATEDIFF('2023-01-02', '2023-01-01')", result);
}

@Test
void shouldGenerateSqlForColumnReferences() {
String startDate = "enrollment_date";
String endDate = "incident_date";

String result = (String) function.getSqlBetweenDates(startDate, endDate, visitor);

assertEquals("DATEDIFF(incident_date, enrollment_date)", result);
}
}
}
Loading