[MCOL-4656] Analyse wrong and possibly redundant code in Decimal::integralWideRound Created: 2021-04-01  Updated: 2023-12-15

Status: Open
Project: MariaDB ColumnStore
Component/s: PrimProc
Affects Version/s: 6.1.1
Fix Version/s: 23.10

Type: Task Priority: Major
Reporter: Alexander Barkov Assignee: Roman
Resolution: Unresolved Votes: 0
Labels: beginner-friendly

Issue Links:
PartOf
is part of MCOL-4820 Umbrella for tasks related to the dec... Open
Epic Link: ColumnStore Compatibility Improvements

 Description   

This code:

                return Decimal(value,
 
                               scale,
 
                               precision,
 
                               (div.quot < 0) ? div.quot-- : div.quot++);

looks wrong. The increment and decrement happens AFTER the value is passed to the constructor. So the supposed rounding does not actually happen.

The intended code was most likely as follows:

                return Decimal(value,
 
                               scale,
 
                               precision,
 
                               (div.quot < 0) ? div.quot - 1 : div.quot + 1);

However, there is a chance that the entire Decimal::integralWideRound() is a dead code.

This method is used only once in functioncolumn.h in FunctionColumn::getDecimalVal(). This is a snippet:

            if (scaleMultiplier > 1)
 
            {
 
                if (scaleDiff > 0)
 
                {
 
                    // WIP MCOL-641 Unconditional overflow check
 
                    datatypes::MultiplicationNoOverflowCheck mul;
 
                    mul(decimal.s128Value, scaleMultiplier, decimal.s128Value);
 
                }
 
                else
 
                {
 
                    decimal = decimal.integralWideRound();

tntnatbry and bar could not find an SQL script which would make the execution enter the branch calling integralWideRound().

The code should be further analysed what we should do:

  • either remove the redundant method
  • or fix it to perform rounding correctly

 Comments   
Comment by Roman [ 2021-07-03 ]

The function Decimal::integralWideRound() and the corresponding branch in FunctionColumn::getDecimalVal() must be removed.
With the changes made to CAST in utils/funcexp the only branch that uses Decimal::integralWideRound() became obsolete and it should be removed with the mentioned changes in the CAST code.

Generated at Thu Feb 08 02:52:00 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.