Skip to content

Commit eb5938d

Browse files
rohit01010rohit01010
authored
Fix issue with truncation of result of arithmetic computations to correct scale when result is of numeric datatype (#3648)
* Fix issue with truncation of result of arithmetic computations to correct scale when result is of numeric datatype (#3455) Currently, whenever an arithmetic operation is performed which results in numeric/decimal, we are storing the result till maximum possible scale instead of the correct scale. This is causing incorrect results in some arithmetic operations such as numeric division, numeric multiplication etc. This PR will fix this issue by adding a hooks in engine which truncates/rounds the result value to the correct scale and also sets the correct typmod of multiple nodes whose base type is NUMERIC. This PR also fixes precision overflow adjustment logic in case NUMERIC ADDITION, SUBTRACTION and MULTIPLICATION. Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#529 Issues Resolved BABEL-5467, BABEL-5685 Signed-off-by: Rohit Bhagat <rohitbgt@amazon.com>
1 parent bc7785f commit eb5938d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+2019
-176
lines changed

‎contrib/babelfishpg_tds/src/backend/tds/tdsresponse.c‎

Lines changed: 224 additions & 83 deletions
Large diffs are not rendered by default.

‎contrib/babelfishpg_tds/src/include/tds_response.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,6 @@ extern void TDSStatementExceptionCallback(PLtsql_execstate *estate, PLtsql_stmt
9898
bool terminate_batch);
9999
extern void SendColumnMetadata(TupleDesc typeinfo, List *targetlist, int16 *formats);
100100
extern bool GetTdsEstateErrorData(int *number, int *severity, int *state);
101-
extern int32 resolve_numeric_typmod_from_exp(Plan *plan, Node *expr);
101+
extern int32 resolve_numeric_typmod_from_exp(Plan *plan, Node *expr, bool *found);
102102

103103
#endif /* TDS_H */

‎contrib/babelfishpg_tsql/src/hooks.c‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ static void pltsql_ExecutorRun(QueryDesc *queryDesc, ScanDirection direction, ui
164164
static void pltsql_ExecutorFinish(QueryDesc *queryDesc);
165165
static void pltsql_ExecutorEnd(QueryDesc *queryDesc);
166166
static bool pltsql_bbfViewHasInsteadofTrigger(Relation view, CmdType event);
167+
static Datum adjust_numeric_result(Plan *plan, Node *expr, Datum result, bool result_isnull, Oid result_type, int32 result_typmod);
168+
static void pltsql_ExecUpdateResultTypeTL(PlanState *planstate, TupleDesc desc);
167169

168170
static bool plsql_TriggerRecursiveCheck(ResultRelInfo *resultRelInfo);
169171
static bool bbf_check_rowcount_hook(int es_processed);
@@ -196,6 +198,7 @@ static void revoke_func_permission_from_public(Oid objectId);
196198
*****************************************/
197199
static PlannedStmt *pltsql_planner_hook(Query *parse, const char *query_string, int cursorOptions, ParamListInfo boundParams);
198200
static char* pltsql_get_object_identity_event_trigger(ObjectAddress *addr);
201+
static int32 pltsql_exprTypmod(Plan *plan, Node *expr);
199202

200203
/* Save hook values in case of unload */
201204
static core_yylex_hook_type prev_core_yylex_hook = NULL;
@@ -221,10 +224,13 @@ static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
221224
static GetNewObjectId_hook_type prev_GetNewObjectId_hook = NULL;
222225
static inherit_view_constraints_from_table_hook_type prev_inherit_view_constraints_from_table = NULL;
223226
static bbfViewHasInsteadofTrigger_hook_type prev_bbfViewHasInsteadofTrigger_hook = NULL;
227+
static adjust_numeric_result_hook_type prev_adjust_numeric_result_hook = NULL;
228+
static ExecUpdateResultTypeTL_hook_type prev_ExecUpdateResultTypeTL_hook = NULL;
224229
static detect_numeric_overflow_hook_type prev_detect_numeric_overflow_hook = NULL;
225230
static match_pltsql_func_call_hook_type prev_match_pltsql_func_call_hook = NULL;
226231
static insert_pltsql_function_defaults_hook_type prev_insert_pltsql_function_defaults_hook = NULL;
227232
static replace_pltsql_function_defaults_hook_type prev_replace_pltsql_function_defaults_hook = NULL;
233+
static exprTypmod_hook_type prev_exprTypmod_hook = NULL;
228234
static print_pltsql_function_arguments_hook_type prev_print_pltsql_function_arguments_hook = NULL;
229235
static planner_hook_type prev_planner_hook = NULL;
230236
static transform_check_constraint_expr_hook_type prev_transform_check_constraint_expr_hook = NULL;
@@ -343,6 +349,12 @@ InstallExtendedHooks(void)
343349
prev_bbfViewHasInsteadofTrigger_hook = bbfViewHasInsteadofTrigger_hook;
344350
bbfViewHasInsteadofTrigger_hook = pltsql_bbfViewHasInsteadofTrigger;
345351

352+
prev_adjust_numeric_result_hook = adjust_numeric_result_hook;
353+
adjust_numeric_result_hook = adjust_numeric_result;
354+
355+
prev_ExecUpdateResultTypeTL_hook = ExecUpdateResultTypeTL_hook;
356+
ExecUpdateResultTypeTL_hook = pltsql_ExecUpdateResultTypeTL;
357+
346358
prev_detect_numeric_overflow_hook = detect_numeric_overflow_hook;
347359
detect_numeric_overflow_hook = pltsql_detect_numeric_overflow;
348360

@@ -355,6 +367,9 @@ InstallExtendedHooks(void)
355367
prev_replace_pltsql_function_defaults_hook = replace_pltsql_function_defaults_hook;
356368
replace_pltsql_function_defaults_hook = replace_pltsql_function_defaults;
357369

370+
prev_exprTypmod_hook = exprTypmod_hook;
371+
exprTypmod_hook = pltsql_exprTypmod;
372+
358373
prev_print_pltsql_function_arguments_hook = print_pltsql_function_arguments_hook;
359374
print_pltsql_function_arguments_hook = print_pltsql_function_arguments;
360375

@@ -476,10 +491,13 @@ UninstallExtendedHooks(void)
476491
GetNewObjectId_hook = prev_GetNewObjectId_hook;
477492
inherit_view_constraints_from_table_hook = prev_inherit_view_constraints_from_table;
478493
bbfViewHasInsteadofTrigger_hook = prev_bbfViewHasInsteadofTrigger_hook;
494+
adjust_numeric_result_hook = prev_adjust_numeric_result_hook;
495+
ExecUpdateResultTypeTL_hook = prev_ExecUpdateResultTypeTL_hook;
479496
detect_numeric_overflow_hook = prev_detect_numeric_overflow_hook;
480497
match_pltsql_func_call_hook = prev_match_pltsql_func_call_hook;
481498
insert_pltsql_function_defaults_hook = prev_insert_pltsql_function_defaults_hook;
482499
replace_pltsql_function_defaults_hook = prev_replace_pltsql_function_defaults_hook;
500+
exprTypmod_hook = prev_exprTypmod_hook;
483501
print_pltsql_function_arguments_hook = prev_print_pltsql_function_arguments_hook;
484502
planner_hook = prev_planner_hook;
485503
transform_check_constraint_expr_hook = prev_transform_check_constraint_expr_hook;
@@ -902,6 +920,58 @@ pltsql_bbfViewHasInsteadofTrigger(Relation view, CmdType event)
902920
return false;
903921
}
904922

923+
/*
924+
* contains_truncation_functions_checker
925+
* For Numeric Division and Numeric Average, truncation of
926+
* results needs to be done. Following function identifies
927+
* all such functions which requires result truncation.
928+
*/
929+
static bool
930+
contains_truncation_functions_checker(Oid func_id, void *context)
931+
{
932+
return (func_id == F_NUMERIC_DIV || func_id == F_AVG_NUMERIC);
933+
934+
}
935+
936+
/*
937+
* adjust_numeric_result
938+
* truncates/rounds the result value to the correct scale based on result_typmod.
939+
* for result_typmod = -1, computes the result_typmod using pltsql_exprTypmod function.
940+
*/
941+
static Datum
942+
adjust_numeric_result(Plan *plan, Node *expr, Datum result, bool result_isnull, Oid result_type, int32 result_typmod)
943+
{
944+
int32 scale;
945+
946+
if (sql_dialect != SQL_DIALECT_TSQL || result_isnull)
947+
return result;
948+
949+
if (!OidIsValid(result_type))
950+
result_type = exprType(expr);
951+
952+
if (result && OidIsValid(result_type) && getBaseType(result_type) == NUMERICOID)
953+
{
954+
if (expr != NULL && result_typmod == -1)
955+
result_typmod = pltsql_exprTypmod(plan, expr);
956+
957+
if (result_typmod != -1)
958+
{
959+
scale = (result_typmod - VARHDRSZ) & 0xffff;
960+
961+
/*
962+
* For Numeric Division and Numeric Average,
963+
* we need to do a truncation and for rest we need to do rounding
964+
*/
965+
if (check_functions_in_node(expr, contains_truncation_functions_checker, NULL))
966+
return DirectFunctionCall2(numeric_trunc, result, Int32GetDatum(scale));
967+
else
968+
return DirectFunctionCall2(numeric_round, result, Int32GetDatum(scale));
969+
}
970+
}
971+
972+
return result;
973+
}
974+
905975
/*
906976
* Wrapper function that calls the initilization function.
907977
* Calls the pre function call hook on the procname
@@ -5077,3 +5147,80 @@ pltsql_get_object_identity_event_trigger(ObjectAddress* address)
50775147
}
50785148
return identity;
50795149
}
5150+
5151+
/*
5152+
* pltsql_exprTypmod -
5153+
* returns the type-specific modifier of the expression's result type,
5154+
* if it can be determined, else we return -1.
5155+
*/
5156+
static int32
5157+
pltsql_exprTypmod(Plan *plan, Node *expr)
5158+
{
5159+
int32 result_typmod = -1;
5160+
Oid expr_type;
5161+
5162+
if (sql_dialect != SQL_DIALECT_TSQL || expr == NULL)
5163+
return -1;
5164+
5165+
expr_type = exprType(expr);
5166+
5167+
if (!OidIsValid(expr_type))
5168+
return -1;
5169+
5170+
if (getBaseType(expr_type) == NUMERICOID)
5171+
{
5172+
bool found_typmod;
5173+
5174+
/*
5175+
* use get_numeric_typmod_from_exp function to get the typmod
5176+
* from the expression node, when the expression type is numeric.
5177+
*/
5178+
if (*pltsql_protocol_plugin_ptr && (*pltsql_protocol_plugin_ptr)->get_numeric_typmod_from_exp)
5179+
{
5180+
result_typmod = (*pltsql_protocol_plugin_ptr)->get_numeric_typmod_from_exp(plan, expr, &found_typmod);
5181+
if (!found_typmod)
5182+
return -1;
5183+
}
5184+
}
5185+
return result_typmod;
5186+
}
5187+
5188+
/*
5189+
* pltsql_ExecUpdateResultTypeTL
5190+
*
5191+
* Update typmod of all the entries of a previously initialized tuple descriptor,
5192+
* using pltsql_exprTypmod when attribute type is NUMERIC, DECIMAL and any UDT on NUMERIC, DECIMAL.
5193+
*/
5194+
void
5195+
pltsql_ExecUpdateResultTypeTL(PlanState *planstate, TupleDesc desc)
5196+
{
5197+
ListCell *l;
5198+
int cur_resno = 1;
5199+
List *targetList;
5200+
5201+
/*
5202+
* sanity checks
5203+
*/
5204+
if (sql_dialect != SQL_DIALECT_TSQL ||
5205+
!PointerIsValid(desc) ||
5206+
!PointerIsValid(planstate) ||
5207+
!PointerIsValid(planstate->plan) ||
5208+
!PointerIsValid(planstate->plan->targetlist))
5209+
return;
5210+
5211+
targetList = planstate->plan->targetlist;
5212+
5213+
foreach(l, targetList)
5214+
{
5215+
TargetEntry *tle = lfirst(l);
5216+
Form_pg_attribute attr = TupleDescAttr(desc, cur_resno - 1);
5217+
5218+
if (attr->atttypid == NUMERICOID || getBaseType(attr->atttypid) == NUMERICOID)
5219+
{
5220+
attr->atttypmod = pltsql_exprTypmod(planstate->plan, (Node *) tle->expr);
5221+
}
5222+
5223+
cur_resno++;
5224+
}
5225+
}
5226+

‎contrib/babelfishpg_tsql/src/pltsql.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1760,7 +1760,7 @@ typedef struct PLtsql_protocol_plugin
17601760
bool (*get_reset_tds_connection_flag) ();
17611761
void (*get_tvp_typename_typeschemaname) (char *proc_name, char *target_arg_name,
17621762
char **tvp_type_name, char **tvp_type_schema_name);
1763-
int32 (*get_numeric_typmod_from_exp) (Plan *plan, Node *expr);
1763+
int32 (*get_numeric_typmod_from_exp) (Plan *plan, Node *expr, bool *found);
17641764
/* Session level GUCs */
17651765
bool quoted_identifier;
17661766
bool arithabort;

‎contrib/babelfishpg_tsql/src/pltsql_coerce.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2007,7 +2007,7 @@ tsql_select_common_typmod_hook(ParseState *pstate, List *exprs, Oid common_type)
20072007
type = getBaseTypeAndTypmod(type, &typmod);
20082008

20092009
if (typmod == -1 && (*pltsql_protocol_plugin_ptr))
2010-
typmod = (*pltsql_protocol_plugin_ptr)->get_numeric_typmod_from_exp(NULL, expr);
2010+
typmod = (*pltsql_protocol_plugin_ptr)->get_numeric_typmod_from_exp(NULL, expr, NULL);
20112011

20122012
if (typmod == -1 || !is_tsql_exact_numeric_type(type))
20132013
continue;

‎test/JDBC/expected/BABEL-1000.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,14 @@ SELECT * FROM babel_1000_test8(1)
162162
GO
163163
~~START~~
164164
numeric
165-
2.06000000
165+
2.06
166166
~~END~~
167167

168168
SELECT * FROM babel_1000_test8(12.345678)
169169
GO
170170
~~START~~
171171
numeric
172-
13.06000000
172+
13.06
173173
~~END~~
174174

175175
-- overflow, expect error

‎test/JDBC/expected/BABEL-3066-vu-verify.out‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ SELECT CAST(CAST(12465781.46792 as real) as numeric(38,10)) + CAST(CAST(12465781
466466
GO
467467
~~START~~
468468
numeric
469-
24931562.000000000000000
469+
24931562.0000000000
470470
~~END~~
471471

472472

@@ -504,15 +504,15 @@ SELECT CAST(CAST(12465781.46792 as real) as numeric(38,10)) - CAST(CAST(12465781
504504
GO
505505
~~START~~
506506
numeric
507-
0
507+
0E-10
508508
~~END~~
509509

510510

511511
SELECT CAST(CAST(12465781.46792 as real) as numeric(38,10)) - CAST(CAST(12465781.4679213254 as real) as numeric(38,10));
512512
GO
513513
~~START~~
514514
numeric
515-
0
515+
0E-10
516516
~~END~~
517517

518518

@@ -540,9 +540,10 @@ GO
540540

541541
SELECT CAST(CAST(12465781.46792 as real) as numeric(38,10)) * CAST(CAST(12465781.4679213254 as real) as numeric(38,15));
542542
GO
543-
~~ERROR (Code: 33557097)~~
544-
545-
~~ERROR (Message: Arithmetic overflow error for data type numeric.)~~
543+
~~START~~
544+
numeric
545+
155395695939961.000000
546+
~~END~~
546547

547548

548549
SELECT CAST(CAST(CAST(CAST(12465781.46792 as real) as numeric(38,10)) * CAST(CAST(12465781.4679213254 as real) as numeric(38,15)) as real) as numeric(38,6));
@@ -557,15 +558,15 @@ SELECT CAST(CAST(12465781.46792 as real) as numeric(38,10)) * CAST(CAST(12465781
557558
GO
558559
~~START~~
559560
numeric
560-
155395695939961.00000000000000000000
561+
155395695939961.000000
561562
~~END~~
562563

563564

564565
SELECT CAST(CAST(12465781.46792 as real) as numeric(38,0)) / CAST(CAST(12465781.4679213254 as real) as numeric(38,0));
565566
GO
566567
~~START~~
567568
numeric
568-
1.00000000000000000000
569+
1.000000
569570
~~END~~
570571

571572

@@ -587,14 +588,14 @@ SELECT CAST(CAST(12465781.46792 as real) as numeric(38,10)) / CAST(CAST(12465781
587588
GO
588589
~~START~~
589590
numeric
590-
1.00000000000000000000
591+
1.000000
591592
~~END~~
592593

593594

594595
SELECT CAST(CAST(12465781.46792 as real) as numeric(38,10)) / CAST(CAST(12465781.4679213254 as real) as numeric(38,10));
595596
GO
596597
~~START~~
597598
numeric
598-
1.00000000000000000000
599+
1.000000
599600
~~END~~
600601

‎test/JDBC/expected/BABEL-381.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ select 2.0/1.5;
1212
go
1313
~~START~~
1414
numeric
15-
1.3333333333333333
15+
1.333333
1616
~~END~~
1717

1818

1919
select 2.0, 2.0/1.5, 1.0/1.5;
2020
go
2121
~~START~~
2222
numeric#!#numeric#!#numeric
23-
2.0#!#1.3333333333333333#!#0.66666666666666666667
23+
2.0#!#1.333333#!#0.666666
2424
~~END~~
2525

0 commit comments

Comments
 (0)