Skip to content

Commit

Permalink
#429 check compatibility of aggregate for Datetime column
Browse files Browse the repository at this point in the history
  • Loading branch information
ldhasson committed Oct 30, 2019
1 parent 0866e12 commit af261f2
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 13 deletions.
17 changes: 16 additions & 1 deletion src/tilda/enums/AggregateType.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ public ColumnType getType(ColumnType T)
case COUNT:
return ColumnType.LONG;
case MIN:
case FIRST:
case MAX:
if (T != ColumnType.DATETIME)
return T;
case FIRST:
case LAST:
return T;
case SUM:
Expand Down Expand Up @@ -128,5 +130,18 @@ public boolean isOrderable()
throw new Error("Incomplete Switch statment: unknown ColumnType " + this.name() + ";");
}
}

/**
* Tests whether an aggregate is friendly with DateTime columns. Because DateTime columns have a companion
* TZ column to maintain the timezone information, aggregates that cannot be ordered essentially are not usable.
* This method simply wraps isOrderable() in case we later need to make some modifications, i.e., the
* isTimeZoneCompatible is semantically different enough from isOrderable that we didn't want to alias them.
* @return
*/
public boolean isZonedDateTimeCompatible()
{
return isOrderable() == true;
}


}
3 changes: 3 additions & 0 deletions src/tilda/generation/Generator.java
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,9 @@ protected static void genTildaData(GeneratorSession G, File GenFolder, Object O)
Out.println();
DG.MustNotBeModified(Out, G);
Out.println();
if (C.getName().equals("a9") == true)
LOG.debug("xxx");


if (C._Mode != ColumnMode.CALCULATED || C._Mode == ColumnMode.CALCULATED && C._MapperDef != null)
{
Expand Down
22 changes: 13 additions & 9 deletions src/tilda/parsing/parts/ViewColumn.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,13 @@ public boolean Validate(ParserSession PS, View ParentView)
{
if ((_Aggregate = AggregateType.parse(_AggregateStr)) == null)
return PS.AddError("View Column '" + getFullName() + "' defined an invalid 'aggregate' '" + _AggregateStr + "'.");
// if (_SameAsObj != null && _SameAsObj._Type == ColumnType.DATETIME)
// return PS.AddError("View Column '" + getFullName() + "' defined an aggregate on DATETIME column '" + _SameAsObj.getName() + "' which is not supported as timezone
// information would not be retrievable.");
if (_SameAsObj != null && _SameAsObj._Type == ColumnType.DATETIME && _Aggregate.isZonedDateTimeCompatible() == false)
{
StringBuilder Str = new StringBuilder("View Column '" + getFullName() + "' declares a nonsensical " + _Aggregate.name() + " aggregate over type " + ColumnType.DATETIME.name() + ".");
if (_Aggregate == AggregateType.MIN || _Aggregate == AggregateType.MAX)
Str.append(" Because of the way ZonedDateTimes are represented in the database as two columns, Min/Max are not supported as aggregates but you can use First/Last with orderBy instead to the same effect.");
PS.AddError(Str.toString());
}
}
if (_Aggregate == null)
{
Expand Down Expand Up @@ -222,14 +226,14 @@ else if (_Distinct == true)
if (_TypeStr != null)
{
_Type = new TypeDef(_TypeStr, _Size, _Precision, _Scale);
_Type.Validate(PS, "View Column '"+getFullName()+"'", true, false);
_Type.Validate(PS, "View Column '" + getFullName() + "'", true, false);
}
// Checking that type information is only present when expression is specified and vice-versa.
if (TextUtil.isNullOrEmpty(_Expression) == false && _Type == null)
PS.AddError("View Column '" + getFullName() + "' defined an 'expression' but neglected to specify type information and optionally, size.");
if (TextUtil.isNullOrEmpty(_Expression) == true && _Type != null)
PS.AddError("View Column '" + getFullName() + "' defined extra type/size information without an 'expression': type and size are for expressions only.");

return Errs == PS.getErrorCount();
}

Expand Down Expand Up @@ -340,15 +344,15 @@ public static String PrintColumnList(List<ViewColumn> L)
}

/**
* A view column of type 'DATETIME' needs an extra timezone support field if the underlying column needs one, and the
* A view column of type 'DATETIME' needs an extra timezone support field if the underlying column needs one, and the
* view column is not an aggregate, and does not have an expression unless it's of type datetime.
*
* @return
*/
public boolean needsTZ()
{
return (_SameAsObj == null || _SameAsObj.needsTZ() == true)
&& _Aggregate == null
&& (TextUtil.isNullOrEmpty(_Expression) == true || _Type._Type == ColumnType.DATETIME)
;
&& (_Aggregate == null || _Aggregate.isZonedDateTimeCompatible() == true)
&& (TextUtil.isNullOrEmpty(_Expression) == true || _Type._Type == ColumnType.DATETIME);
}
}
2 changes: 1 addition & 1 deletion ut/tilda/data_test/_tilda.TildaTest.json
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@
,{ "sameas": "Testing.a2" , "name":"a2Min" , "aggregate":"MIN" , "filter":"a2 is not null" , "coalesce":"AAA" }
,{ "sameas": "Testing.a2" , "name":"a2Max" , "aggregate":"MAX" , "filter":"a2 is not null" , "coalesce":"ZZZ"}
,{ "sameas": "Testing.a9" , "aggregate":"ARRAY" , "orderBy":["lastUpdated asc"] }
,{ "sameas": "Testing.a9c" , "aggregate":"ARRAY"}
,{ "sameas": "Testing.a9c" , "aggregate":"ARRAY" }
,{ "sameas": "Testing.a6" , "name":"a6First", "aggregate":"FIRST" , "orderBy":["lastUpdated asc"] }
,{ "sameas": "Testing.a6" , "name":"a6Last" , "aggregate":"LAST" , "orderBy":["lastUpdated asc"] }
]
Expand Down
5 changes: 3 additions & 2 deletions ut/tilda/data_tutorial/_tilda.TildaTutorial.json
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@
,"description": "A pivoted view of SAT_01 forms"
,"columns":[
{ "sameas": "Form.type" }
,{ "sameas": "Form.fillDate", "joinOnly":true }
,{ "sameas": "Form.refnum" , "name":"testCount" , "aggregate":"COUNT", "distinct":true }
,{ "sameas": "Form.fillDate", "name":"testFirst" , "aggregate":"MIN" }
,{ "sameas": "Form.fillDate", "name":"testLast" , "aggregate":"LAST" }
,{ "sameas": "Form.fillDate", "name":"testFirst" , "aggregate":"FIRST", "orderBy":["fillDate"] }
,{ "sameas": "Form.fillDate", "name":"testLast" , "aggregate":"LAST" , "orderBy":["fillDate"] }
,{ "sameas": "User.refnum" , "name":"userCount" , "aggregate":"COUNT", "distinct":true }
,{ "sameas": "TestAnswer.correct" , "name":"answerCount" , "aggregate":"COUNT" }
,{ "sameas": "TestAnswer.correct" , "name":"answerCountCorrect" , "aggregate":"COUNT", "filter":"correct = true" }
Expand Down

0 comments on commit af261f2

Please sign in to comment.