Skip to content

Commit 7f1264a

Browse files
authored
Merge pull request #411 from DataObjects-NET/7.1-pgsql-decimal-aggregate-issue
Improves aggregates over decimal translation for PostgreSQL
2 parents a945ff6 + 8f1fd18 commit 7f1264a

File tree

14 files changed

+687
-285
lines changed

14 files changed

+687
-285
lines changed

ChangeLog/7.1.3_dev.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
[main] Addressed race condition issue with TranslatorState.NonVisitableExpressions
22
[postgresql] Improved database structucture extraction
3+
[postgresql] Addressed certain cases of decimal results of aggregate operations being unable to fit to .NET decimal

Orm/Xtensive.Orm.PostgreSql/Orm.Providers.PostgreSql/SqlCompiler.cs

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// Created by: Denis Krjuchkov
55
// Created: 2009.04.27
66

7+
using System;
78
using System.Collections.Generic;
89
using System.Linq;
910
using Xtensive.Core;
@@ -18,6 +19,8 @@ namespace Xtensive.Orm.Providers.PostgreSql
1819
{
1920
internal class SqlCompiler : Providers.SqlCompiler
2021
{
22+
private const int MaxDotnetDecimalPrecision = 28;
23+
2124
protected override SqlProvider VisitFreeText(FreeTextProvider provider)
2225
{
2326
var rankColumnName = provider.Header.Columns[provider.Header.Columns.Count - 1].Name;
@@ -55,13 +58,83 @@ protected override SqlProvider VisitFreeText(FreeTextProvider provider)
5558
protected override SqlExpression ProcessAggregate(SqlProvider source, List<SqlExpression> sourceColumns, AggregateColumn aggregateColumn)
5659
{
5760
var result = base.ProcessAggregate(source, sourceColumns, aggregateColumn);
58-
if (aggregateColumn.AggregateType == AggregateType.Sum || aggregateColumn.AggregateType == AggregateType.Avg) {
61+
var aggregateType = aggregateColumn.AggregateType;
62+
var originCalculateColumn = source.Origin.Header.Columns[aggregateColumn.SourceIndex];
63+
if (AggregateRequiresDecimalAdjustments(aggregateColumn)) {
64+
if (!IsCalculatedColumn(originCalculateColumn)) {
65+
// this is aggregate by one column, result will be defined by the precision and scale of the column
66+
return result;
67+
}
68+
69+
// For expressions we had to try to guess result type parameters to avoid overflow exception
70+
// on reading something like 12.000000000000000000000000000000 (30 zeros) which in practice can be reduced
71+
// to 12.0 on reading but Npgsql does not allow us neither to turn on such conversion inside Npgsql (as it was in v3.x.x) nor
72+
// to get raw data and make conversion by ourselves (because nothing similar to SqlDecimal has provided by the library).
73+
74+
// Official answer of the Npgsql team is to either cast to DECIMAL with proper parameters or read all parameters as
75+
// strings and then convert :-)
76+
// Reading strings is not an option so we try to tell fortunes in a teacup :-(
77+
var resultType = (!TryAdjustPrecisionScale(aggregateColumn.Descriptor.DecimalParametersHint, aggregateType, out var newPrecision, out var newScale))
78+
? Driver.MapValueType(aggregateColumn.Type)
79+
: Driver.MapValueType(aggregateColumn.Type, null, newPrecision, newScale);
80+
return SqlDml.Cast(result, resultType);
81+
}
82+
else if (aggregateType != AggregateType.Count) {
5983
result = SqlDml.Cast(result, Driver.MapValueType(aggregateColumn.Type));
6084
}
61-
6285
return result;
6386
}
6487

88+
private bool AggregateRequiresDecimalAdjustments(AggregateColumn aggregateColumn)
89+
{
90+
var aggregateType = aggregateColumn.AggregateType;
91+
return (aggregateType is AggregateType.Sum or AggregateType.Avg
92+
or AggregateType.Min or AggregateType.Min) && aggregateColumn.Type == WellKnownTypes.DecimalType;
93+
}
94+
95+
private bool TryAdjustPrecisionScale(
96+
in (int precision, int scale)? typeHint,
97+
in AggregateType aggregateType,
98+
out int precision, out int scale)
99+
{
100+
if (!typeHint.HasValue) {
101+
precision = -1;
102+
scale = -1;
103+
return false;
104+
}
105+
var typeHintValue = typeHint.Value;
106+
107+
if (typeHintValue.precision == MaxDotnetDecimalPrecision) {
108+
// No room for adjust, otherwise we'll lose floor part data
109+
precision = typeHintValue.precision;
110+
scale = typeHintValue.scale;
111+
return true;
112+
}
113+
114+
// choose max available precision for .net or let it be the one user declared
115+
precision = (typeHintValue.precision < 28) ? 28 : typeHintValue.precision;
116+
117+
// It is benefitial to increase scale but for how much? It is open question,
118+
// sometimes we need bigger floor part, and sometimes bigger fractional part.
119+
// This algorithm is a trade-off.
120+
scale = aggregateType switch {
121+
AggregateType.Avg =>
122+
(typeHintValue.precision < MaxDotnetDecimalPrecision)
123+
? typeHintValue.scale + Math.Max((precision - typeHintValue.precision) / 2, 1)
124+
: typeHintValue.scale + 1,
125+
AggregateType.Sum or
126+
AggregateType.Min or
127+
AggregateType.Max =>
128+
(typeHintValue.precision < MaxDotnetDecimalPrecision - 1)
129+
? typeHintValue.scale + 2
130+
: typeHintValue.scale + 1,
131+
_ => typeHintValue.scale,
132+
};
133+
return true;
134+
}
135+
136+
private bool IsCalculatedColumn(Column column) => column is CalculatedColumn;
137+
65138
public SqlCompiler(HandlerAccessor handlers, CompilerConfiguration configuration)
66139
: base(handlers, configuration)
67140
{

Orm/Xtensive.Orm.PostgreSql/WellKnownTypes.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ internal static class WellKnownTypes
1616
public static readonly Type GuidType = typeof(Guid);
1717
public static readonly Type ByteArrayType = typeof(byte[]);
1818
public static readonly Type StringType = typeof(string);
19+
public static readonly Type DecimalType = typeof(decimal);
1920

2021
public static readonly Type NpgsqlPointType = typeof(NpgsqlPoint);
2122
public static readonly Type NpgsqlLSegType = typeof(NpgsqlLSeg);

0 commit comments

Comments
 (0)