Skip to content

Commit

Permalink
FIX: Contains filter on lookup column returned too many values (#2059)
Browse files Browse the repository at this point in the history
* Contains filter on a lookup column returned too many results because the SQL behind it applied the filter on the ids instead of the lookup values. The time rendered in the SQL was also wrong, not respecting culture and formatting of the values from UIService/GetLookupLabelsEx (that is what is displayed in the grid).
* PgSql Format method throws NotImplementedException because in is not implemented and implementing it is not a priority now
* Code formatting.

---------

Co-authored-by: Petr Hrehorovsky <83349812+washibana@users.noreply.github.com>
  • Loading branch information
JindrichSusen and washibana committed Oct 19, 2023
1 parent 01db289 commit 5940c90
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 15 deletions.
77 changes: 62 additions & 15 deletions backend/Origam.DA.Service/Generators/AbstractSqlCommandGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
using Origam.DA.Service.CustomCommandParser;
using Origam.DA.Service.Generators;
using Origam.Extensions;
Expand Down Expand Up @@ -1858,21 +1859,9 @@ private record GroupByData(DataStructureColumn Column, string Expression);
string orderByExpression="";
if (concatScalarColumns && columnsInfo != null && columnsInfo.Count > 1)
{
List<ColumnRenderItem> columnRenderData = new List<ColumnRenderItem>();
foreach (string columnName in columnsInfo.ColumnNames)
{
DataStructureColumn column = entity.Column(columnName);
columnRenderData.Add(
new ColumnRenderItem
{
SchemaItem = column.Field,
Entity = column.Entity ?? entity,
RenderSqlForDetachedFields = columnsInfo.RenderSqlForDetachedFields
});
}
sqlExpression.Append(" ");
sqlExpression.Append(RenderConcat(columnRenderData, sqlValueFormatter.RenderString(", "),
replaceParameterTexts, dynamicParameters, selectParameterReferences));
sqlExpression.Append(RenderConcat(selectParameters, isInRecursion, forceDatabaseCalculation, sqlValueFormatter.RenderString(", "),
replaceParameterTexts, dynamicParameters, selectParameterReferences, filterCommandParser, orderByCommandParser));
return false;
}
i = 0;
Expand Down Expand Up @@ -3986,9 +3975,66 @@ internal string RenderConcat(ArrayList concatSchemaItems, DataStructureEntity en
concatBuilder.Append(")");
}
return concatBuilder.ToString();
}

internal string RenderConcat(SelectParameters selectParameters, bool isInRecursion, bool forceDatabaseCalculation,
string separator, Hashtable replaceParameterTexts, Hashtable dynamicParameters, Hashtable parameterReferences,
FilterCommandParser filterCommandParser,
OrderByCommandParser orderByCommandParser)
{
int i = 0;
StringBuilder concatBuilder = new StringBuilder();
bool shouldTrimSeparator = !string.IsNullOrEmpty(separator) &&
selectParameters.ColumnsInfo.ColumnNames.Count > 1;
if (shouldTrimSeparator)
{
concatBuilder.Append($"TRIM( {separator} FROM ");
}
SortedList<int, SortOrder> order = new SortedList<int, SortOrder>();
foreach (string columnName in selectParameters.ColumnsInfo.ColumnNames)
{
DataStructureColumn column = selectParameters.Entity.Column(columnName);
if (i > 0)
{
concatBuilder.Append(" " + sqlRenderer.StringConcatenationChar + " ");
if (separator != null)
{
concatBuilder.Append(separator);
concatBuilder.Append(" " + sqlRenderer.StringConcatenationChar + " ");
}
}
string groupByExpression = "";
bool groupByNeeded = false;
LookupOrderingInfo customOrderingInfo =
LookupOrderingInfo.TryCreate(selectParameters.CustomOrderings.Orderings, column.Name);
ColumnRenderData columnRenderData = RenderDataStructureColumn(
selectParameters.DataStructure,
selectParameters.Entity,
replaceParameterTexts, dynamicParameters,
selectParameters.SortSet, parameterReferences, isInRecursion,
forceDatabaseCalculation, ref groupByExpression, order, ref groupByNeeded,
selectParameters.ColumnsInfo ?? ColumnsInfo.Empty, column,
customOrderingInfo, filterCommandParser, orderByCommandParser,
selectParameters.RowOffset);
if (column.DataType == OrigamDataType.Date)
{
string nonNullExpression = $"{sqlRenderer.IsNull()} ({sqlRenderer.Format(columnRenderData.Expression, Thread.CurrentThread.CurrentCulture.Name)}, '')";
concatBuilder.Append(nonNullExpression);
}
else
{
string nonNullExpression = $"{sqlRenderer.IsNull()} ({columnRenderData.Expression}, '')";
concatBuilder.Append(nonNullExpression);
}
i++;
}
if (shouldTrimSeparator)
{
concatBuilder.Append(")");
}
return concatBuilder.ToString();
}


internal string PostProcessCustomCommandParserWhereClauseSegment(
string input, DataStructureEntity entity,
Hashtable replaceParameterTexts, Hashtable dynamicParameters,
Expand Down Expand Up @@ -4145,6 +4191,7 @@ internal class ColumnRenderItem
{
public bool RenderSqlForDetachedFields { get; set; }
public ISchemaItem SchemaItem { get; set; }
public DataStructureColumn Column { get; set; }
public DataStructureEntity Entity { get; set; }
}

Expand Down
6 changes: 6 additions & 0 deletions backend/Origam.DA.Service/Generators/MsSqlRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ internal override string IsNull()
{
return "ISNULL";
}

internal override string Format(string date, string culture)
{
return @$" FORMAT({date}, IIF (FORMAT({date}, 'HH:mm:ss tt', 'en-US' ) = '00:00:00 AM', 'd', ''), '{culture}') ";
}

internal override string CountAggregate()
{
return "COUNT_BIG";
Expand Down
6 changes: 6 additions & 0 deletions backend/Origam.DA.Service/Generators/PgSqlRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,12 @@ internal override string IsNull()
{
return "COALESCE";
}

internal override string Format(string date, string culture)
{
throw new NotImplementedException();
}

internal override string CountAggregate()
{
return "COUNT";
Expand Down
5 changes: 5 additions & 0 deletions backend/Origam.DA.Service/Generators/SqlRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public abstract class SqlRenderer
public abstract string ConvertGeoToTextClause(string argument);
internal abstract string Sequence(string entityName, string primaryKeyName);
internal abstract string IsNull();

// Will omit the time part if the time is 00:00:00.
// The result should be the same as the the result of C# DateTime.ToString() / DateTime.ToShortDateString()
// Because that is what is used in C# code in the UIService/GetLookupLabelsEx endpoint
internal abstract string Format(string date, string culture);
internal abstract string CountAggregate();
internal abstract string DeclareAsSql();
internal abstract string FunctionPrefix();
Expand Down

0 comments on commit 5940c90

Please sign in to comment.