Fixed some defect on STR function

[kelepcera]

Description

This MR aligns STR() with SQL Server in several major correctness areas (carry propagation, decimal-point placement, and negative rounding with decimal = 0). It also documents the remaining compatibility gaps for (a) 17-significant-digits normalization and (b) float tie boundary literals.

Fixed: major correctness issues

-- 1) Negative rounding, carry/sign handling
SELECT STR(-999.9, 6, 0) AS result10;           -- Expected (SQL Server): ' -1000'
GO
-- Before (Babelfish): -1090
-- After  (Babelfish): -1000

-- 2) Carry across the decimal; decimal-point placement
SELECT STR(1.9999, 6, 3) AS rounding_boundary;  -- Expected (SQL Server): '  2.000'
GO
-- Before (Babelfish): 10.000
-- After  (Babelfish):  2.000

-- 3) Additional boundary inputs (now consistent with SQL Server)
SELECT STR(2.9999, 6, 3) AS rounding_boundary;  -- Expected: '  3.000'
GO
-- Before (Babelfish):  10.000
-- After  (Babelfish):  3.000

SELECT STR(3.9999, 6, 3) AS rounding_boundary;  -- Expected: '  4.000'
GO
-- Before (Babelfish): 10.000 (mis-placed carry)
-- After  (Babelfish):  4.000

Why this matters: the old behavior produced numerically wrong strings (-1090 instead of -1000, 10.000 instead of 2.000/4.000) and broke SQL Server parity in common edge cases.

Still different (not fixed in this MR)

SQL Server I guess normalizes to at most 17 significant digits (round at digit 17 and zero-fill beyond) before any length/decimal formatting. We don’t yet enforce that step:

SELECT STR(12345678901234567890, 20) AS result_big1;
GO
-- SQL Server: 12345678901234567000
-- Babelfish : 12345678901234568000   <-- differs

SELECT STR(1234567890.1234567890, 22, 20) AS result_frac_prec;
GO
-- SQL Server: 1234567890.12345670000
-- Babelfish : 1234567890.12345680000 <-- differs

What changed (implementation)

• I have added several comment to find_round_pos() function to increase readability so these changes will not affect the code.
• In round_float_char() function I have also include 0. index to fix "STR(1.9999, 6, 3) " this type of inputs. In the old implementation the code will miss very first digit and it will always turn into a 10. By now it will catch and handle this kind of situations.
• I also added "++float_char;" line to float_str() funtion to handle "STR(-999.9, 6, 0)" kind of inputs.

Corrected post-round carry propagation and decimal-point placement, including cases where carry expands the integer part (e.g., 1.9999 --> 2.000, 3.9999 --> 4.000) without misplacing the decimal or producing 10.000.

Fixed negative rounding with decimal = 0 so sign + carry produce the correct integer (e.g., -999.9 --> -1000) and align with SQL Server formatting.

Issues Resolved

[Issue Link]](#4103)

Test Scenarios Covered

• Use case based -

• Boundary conditions -

• Arbitrary inputs -

• Negative test cases -

• Minor version upgrade tests -

• Major version upgrade tests -

• Performance tests -

• Tooling impact -

• Client tests -

Check List

• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[coveralls]

Pull Request Test Coverage Report for Build 20502144051

Details

• 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 76.871%

---

Totals
Change from base Build 20489420094:	0.01%
Covered Lines:	52489
Relevant Lines:	68282

---

💛 - Coveralls

[Yvinayak07]

Let's add test cases also.

[kelepcera]

Let's add test cases also.

Hello, I have added several regression tests — you can check their results.
You will notice some differences in certain inputs, but most of them are normal. According to the official Microsoft documentation, float_expression is defined as:

"float_expression is an expression of approximate numeric (float) data type with a decimal point."

Because of this “approximate number” behavior, slight discrepancies (especially around .5 boundaries or values like 2.675) are expected and not considered bugs.

The only remaining problem is with numbers that exceed 17 significant digits: SQL Server truncates them, while our code sometimes rounds. I attempted a fix, but it caused regressions elsewhere, so I left it as is for now.

[kelepcera]

@Yvinayak07 Hello is there any update?
@tanyagupta17

[kelepcera]

Hi, I just wanted to kindly check if you’ve had a chance to look at the merge request I opened for the fix. It’s a very small change and you can check in your local env adding some break points. Thank you.
@Yvinayak07 @tanyagupta17

[tanyagupta17]

Hi, could you please add some test cases including functions, procedures and views calling this function STR.

[kelepcera]

Yes, sure thank you!

[kelepcera]

Hello again, it has already kind of test cases and I have just added more test cases to test new outputs and updated expected files.

[tanyagupta17]

Kindly you the term "-- TSQL:" instead

[kelepcera]

Thank you to inform me. I will fix them.

[kelepcera]

It was fixed.

[kelepcera]

Hello, is there any update? :) @tanyagupta17 @Yvinayak07 @rohit01010

[kelepcera]

Hi there,
I hope you’re doing well. I wanted to kindly check in regarding the review for the Babelfish Extension MR I submitted about two weeks ago.
Whenever you have a moment to take a look, I’d really appreciate it. Thanks in advance.
@tanyagupta17 @Yvinayak07 @rohit01010

[tanyagupta17]

Hi @kelepcera could you please fix the failing test cases in this PR. Thanks!
I checked the test failures, could you please re-run your PR, these failures are fixed in recent times.

[kelepcera]

Hi @kelepcera could you please fix the failing test cases in this PR. Thanks! I checked the test failures, could you please re-run your PR, these failures are fixed in recent times.

Hi @tanyagupta17 . I rebased my branch as a result of this the pipeline has been triggered. Every jobs has been finished successfully. And also thank you to take care of errors in my pipeline.

[kelepcera]

Hello @tanyagupta17 , is there any update about bugs?
@Yvinayak07 @rohit01010

[kelepcera]

Hello everybody, is there any update? It should be ready to be merged.
@tanscorpio7 @tanyagupta17 @Yvinayak07 @rohit01010

[rohit01010]

Similar to how its done in earlier return statements, for example following case

/* not enough space for the carried_over digit, return *** */
				/*
				 * the space limitation goes by the one set before the
				 * rounding & carried over
				 */
				/*
				 * STR(9999.998, 7, 2) returns "*******" but STR(10000.000, 7,
				 * 2) returns "10000.0"
				 */
				/*
				 * which means the max length constraint of integer part is
				 * still 4 after rounding
				 */
				memset(buf, '*', size - 1);
				return return_varchar_pointer(buf, size - 1);

can we use size-1 instead of computing the length again using strlen, as (int)strlen(buf) will anyways be size-1 ?

[rohit01010]

memset(float_char, '0', 1);

can we evaluate whether we need this or not ?

[rohit01010]

Thanks @kelepcera for your patience! I've completed my review and left some comments. Please check the comments when you get a chance.