Skip to content
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.

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

[CBRD-24597] fix coerce strict with char to numeric #4016

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

ctshim
Copy link
Contributor

@ctshim ctshim commented Dec 28, 2022

http://jira.cubrid.org/browse/CBRD-24597

  • When there is an index containing a numeric type and is performed in the prepare-execute method, it can be processed normally or incorrectly depending on the type of value to be bound.

Found in cases where there is an index containing a numeric type and it is done in a prepare-execute manner.
There was an error when the type of value to be bound was a string type such as char or varchar.

There are two places that use tp_atonumeric().
Called when converting character types to NUMERIC types in tp_value_coerc_strict() and tp_value_cast_internal().
First of all, you can see that there is a difference in the behavior after calling tp_atonumeric() in both functions.
tp_value_cast_internal() is calling tp_value_coercce() again.

tp_atonumeric() is replaced with a numeric form, but its precision, scale is not set to the same target that needs to be changed.
In other words, interpret the given string as it is.
Use tp_value_coercce() to change the value created in this way to the desired precision, scale again.

In tp_value_coercase_strict(), there is no process to call tp_value_coercase() to calibrate, so modify it for further processing.

drop if exists t2;
create table t2 (a numeric(10,3), b int);
insert into t2 values(1,1);
create index idx on t2 (a, b);
prepare s2 from 'select * from t2 where a <= ?';
execute s2 using '1';

@@ -6261,9 +6261,22 @@ tp_value_coerce_strict (const DB_VALUE * src, DB_VALUE * dest, const TP_DOMAIN *
case DB_TYPE_NCHAR:
case DB_TYPE_VARNCHAR:
{
if (tp_atonumeric (src, target) != NO_ERROR)
DB_VALUE temp;
Copy link
Member

Choose a reason for hiding this comment

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

Trivial:
DB_VALUE doesn't have a constructor. Only the domain of the DB_VALUE is initialized in tp_atonumeric.
It doesn't cause any issues for now. But whenever DB_VALUE is used, db_make_null() should be called just in case.

Suggested change
DB_VALUE temp;
DB_VALUE temp;
db_make_null (&temp);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initializing by calling db_value_domain_init() internally.
Also, it seems that this code will not affect you.
I'll keep this part as it is for now.

If we're going to modify it, we need to modify the same thing for tp_value_cast_internal() calling tp_atonumeric().
Thank you.

@ctshim ctshim merged commit bc4f35e into CUBRID:develop Jan 4, 2023
ctshim added a commit to ctshim/cubrid that referenced this pull request Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants