Skip to content

Fixed some defect on STR function #4104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kelepcera wants to merge 8 commits into
base: BABEL_5_X_DEV
Choose a base branch
from
+969 −97
Open

Fixed some defect on STR function #4104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3ab4856
Fixed some defect on STR function
Sep 18, 2025
7242c9d
Added regression test
Sep 24, 2025
aef42b2
Added more test cases
Nov 11, 2025
8d77a9e
Fix wrong names
Nov 11, 2025
342959d
Extended and updated test cases to cover new results
Nov 11, 2025
0ac6ce9
Reverted unnecessay test cases
Nov 11, 2025
84cfed7
Quick fix
Nov 11, 2025
bc346b3
Merge branch 'babelfish-for-postgresql:BABEL_5_X_DEV' into fix/str_fu…
kelepcera Dec 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 29 additions & 23 deletions contrib/babelfishpg_tsql/src/string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ tsql_varbinary_substr(PG_FUNCTION_ARGS)
/*
* Returns character data converted from numeric data. The character data
* is right-justified, with a specified length and decimal precision.
*/
*/
Datum
float_str(PG_FUNCTION_ARGS)
{
Expand All @@ -557,7 +557,7 @@ float_str(PG_FUNCTION_ARGS)
char *ptr;
int input_deci_digits;
int has_neg_sign = 0;
int input_deci_point = 0;
int input_deci_point = 0; /* NOTE: this is a flag (0/1), not an index */
int has_deci_point = 0;
int num_spaces = 0;
int int_digits = 0;
Expand Down Expand Up @@ -643,6 +643,8 @@ float_str(PG_FUNCTION_ARGS)
/* allocate buffer for putting result together */
buf = palloc(size);
memset(buf, 0, size);

/* account for sign and decimal point in available length */
if (has_neg_sign)
length--;
if (decimal > 0 && length > int_digits)
Expand Down Expand Up @@ -680,15 +682,15 @@ float_str(PG_FUNCTION_ARGS)

num_spaces = length - int_digits - deci_digits;

/* comp space for the decimal point */
/* if we planned a decimal point but deci_digits=0, remove point and reclaim 1 space */
if (has_deci_point && !deci_digits)
{
/*
* no enough space for decimal part, remove the decimal point and add
* one space. STR(1.234, 2, 1) returns " 1"
*/
num_spaces++;
has_deci_point--;
has_deci_point = 0;
}

/*
Expand Down Expand Up @@ -749,6 +751,7 @@ float_str(PG_FUNCTION_ARGS)
*/
memset(buf + num_spaces - 1, '-', 1);
num_spaces++;
++float_char; /* skip '-' in source */
}
memset(buf + num_spaces - 1, '1', 1);
memset(float_char, '0', 1);
Copy link
Copy Markdown
Contributor

@rohit01010 rohit01010 Jan 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

memset(float_char, '0', 1);

can we evaluate whether we need this or not ?

Expand All @@ -774,7 +777,6 @@ float_str(PG_FUNCTION_ARGS)
}
}


/* copy the actual number to the buffer after preceding spaces */
strncpy(buf + num_spaces, float_char, size - 1 - num_spaces);

Expand All @@ -801,47 +803,50 @@ float_str(PG_FUNCTION_ARGS)
}
}

return return_varchar_pointer(buf, size);
/* return VARCHAR with actual content length (no NUL) */
return return_varchar_pointer(buf, (int)strlen(buf));
Comment on lines +806 to +807
Copy link
Copy Markdown
Contributor

@rohit01010 rohit01010 Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

}

/*
* Find the rounding position of the float_char input using the constraints
* returns the rounding position
*/
*/
static int
find_round_pos(char *float_char, int has_neg_sign, int int_digits, int deci_digits, int input_deci_digits, int input_deci_point, int deci_sig)
{
int round_pos = 0;
int curr_digit;
int look_idx;

if (int_digits + input_deci_digits > 17)
{
/*
* exceeds the max precision, need to round to 17th digit(excluding -
* and .)
*/
/* Case 1: 18th significant digit is in integer part */
if (int_digits > 17)
{
/* round in int part */
/* STR(12345678901234567890, 20) returns "1234567890123456800" */
curr_digit = float_char[17 + has_neg_sign] - '0';
/* look at the 18th significant digit (skip '-') */
look_idx = 17 + has_neg_sign;
curr_digit = float_char[look_idx] - '0';

if (curr_digit >= 5)
round_pos = 16 + has_neg_sign;
round_pos = look_idx - 1; /* round the 17th significant digit */
}
else
{
/* round in decimal part */

/*
* STR(1234567890.1234567890, 22, 20) returns
* "1234567890.12345670000"
* Case 2: 18th significant digit is in decimal part.
* We still "walk" 17 significant digits from start (skipping '-'
* and not counting '.'), so index is:
* has_neg_sign + input_deci_point + 17
* (int_digits cancels out: int_digits + (17 - int_digits) = 17)
*/
curr_digit = float_char[17 + has_neg_sign + input_deci_point] - '0';
look_idx = 17 + has_neg_sign + input_deci_point;
curr_digit = float_char[look_idx] - '0';

if (curr_digit >= 5)
round_pos = 16 + has_neg_sign + input_deci_point;
round_pos = look_idx - 1; /* round the 17th significant digit */
}
}
/* Otherwise, if we have more input decimals than we will print, round to last printed decimal */
else if (deci_digits && input_deci_digits > deci_sig)
{
/* input decimal digits > needed, round to last output decimal digit */
Expand All @@ -850,6 +855,7 @@ find_round_pos(char *float_char, int has_neg_sign, int int_digits, int deci_digi
if (curr_digit >= 5)
round_pos = has_neg_sign + int_digits + input_deci_point + deci_sig - 1;
}
/* If no decimal will be printed but input had decimals, round to integer */
else if (!deci_sig && input_deci_digits)
{
/* int part == length and has deci digit input, round to integer */
Expand All @@ -865,14 +871,14 @@ find_round_pos(char *float_char, int has_neg_sign, int int_digits, int deci_digi

/*
* Inplace round the float_char to the digit at round_pos, returns the final carried over digit
*/
*/
static int
round_float_char(char *float_char, int round_pos, int has_neg_sign)
{
int curr_digit;
int carry = 1;

while (round_pos > (0 + has_neg_sign) && carry)
while (round_pos >= (0 + has_neg_sign) && carry)
{
if (float_char[round_pos] == '.')
{
Expand Down
Loading
Loading