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.

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

Fix sql failed when using replace function #9524

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft

Fix sql failed when using replace function #9524

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6b1ef3b
debug
EricZequan Oct 14, 2024
42be4d8
add 2 function to complete replace for constant
EricZequan Oct 14, 2024
f163a32
remove chinese note
EricZequan Oct 14, 2024
92e8833
fix format
EricZequan Oct 14, 2024
fd4d75e
shorten code and add unit-test
EricZequan Oct 14, 2024
ec18d50
add unit test
EricZequan Oct 14, 2024
3284676
fix unit test fail
EricZequan Oct 14, 2024
b09195b
fix ci
EricZequan Oct 14, 2024
ec6c7d0
fix ci
EricZequan Oct 15, 2024
6408ebd
fix comment
EricZequan Oct 15, 2024
b80338a
add function to solve constant case
EricZequan Oct 15, 2024
2e83159
Merge remote-tracking branch 'upstream/master' into fix-issue-9522
EricZequan Oct 15, 2024
5fc3f5e
fix comment
EricZequan Oct 15, 2024
464d990
fix ci
EricZequan Oct 16, 2024
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
83 changes: 79 additions & 4 deletions dbms/src/Functions/FunctionsStringReplace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,17 @@ class FunctionStringReplace : public IFunction
auto needle = c1_const->getValue<String>();
auto replacement = c2_const->getValue<String>();

if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
if (const auto * col_const = checkAndGetColumnConst<ColumnString>(column_src.get()))
{
std::string result_value;
const auto * src_const = typeid_cast<const ColumnConst *>(column_src.get());
auto src = src_const->getValue<String>();
Impl::constant(src, needle, replacement, pos, occ, match_type, collator, result_value);
auto col_res = ColumnString::create();
col_res->insert(result_value);
column_result.column = std::move(col_res);
}
else if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
{
auto col_res = ColumnString::create();
Impl::vector(
Expand Down Expand Up @@ -232,7 +242,29 @@ class FunctionStringReplace : public IFunction
const auto * col_replacement_const = typeid_cast<const ColumnConst *>(column_replacement.get());
auto replacement = col_replacement_const->getValue<String>();

if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
if (const auto * col_const = checkAndGetColumnConst<ColumnString>(column_src.get()))
{
// using the data directly as a reference.
const auto & const_data = col_const->getDataColumn();
const auto * col = typeid_cast<const ColumnString *>(&const_data);
Copy link
Contributor

@gengliqi gengliqi Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's wrong. The length of this column is just 1. You will find some errors if you correctly add a test for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right. Now I add vectorConstSrcAndReplace、vectorConstSrcAndNeedle、vectorConstSrc to handle the corresponding case to ensure the same number of rows.


auto col_res = ColumnString::create();
Impl::vectorNonConstNeedle(
col->getChars(),
col->getOffsets(),
col_needle->getChars(),
col_needle->getOffsets(),
replacement,
pos,
occ,
match_type,
collator,
col_res->getChars(),
col_res->getOffsets());

column_result.column = std::move(col_res);
}
else if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
{
auto col_res = ColumnString::create();
Impl::vectorNonConstNeedle(
Expand Down Expand Up @@ -292,7 +324,28 @@ class FunctionStringReplace : public IFunction
auto needle = col_needle_const->getValue<String>();
const auto * col_replacement = typeid_cast<const ColumnString *>(column_replacement.get());

if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
if (const auto * col_const = checkAndGetColumnConst<ColumnString>(column_src.get()))
{
// using the data directly as a reference.
const auto & const_data = col_const->getDataColumn();
const auto * col = typeid_cast<const ColumnString *>(&const_data);

auto col_res = ColumnString::create();
Impl::vectorNonConstReplacement(
col->getChars(),
col->getOffsets(),
needle,
col_replacement->getChars(),
col_replacement->getOffsets(),
pos,
occ,
match_type,
collator,
col_res->getChars(),
col_res->getOffsets());
column_result.column = std::move(col_res);
}
else if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
{
auto col_res = ColumnString::create();
Impl::vectorNonConstReplacement(
Expand Down Expand Up @@ -351,7 +404,29 @@ class FunctionStringReplace : public IFunction
const auto * col_needle = typeid_cast<const ColumnString *>(column_needle.get());
const auto * col_replacement = typeid_cast<const ColumnString *>(column_replacement.get());

if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
if (const auto * col_const = checkAndGetColumnConst<ColumnString>(column_src.get()))
{
// using the data directly as a reference.
const auto & const_data = col_const->getDataColumn();
const auto * col = typeid_cast<const ColumnString *>(&const_data);

auto col_res = ColumnString::create();
Impl::vectorNonConstNeedleReplacement(
col->getChars(),
col->getOffsets(),
col_needle->getChars(),
col_needle->getOffsets(),
col_replacement->getChars(),
col_replacement->getOffsets(),
pos,
occ,
match_type,
collator,
col_res->getChars(),
col_res->getOffsets());
column_result.column = std::move(col_res);
}
else if (const auto * col = checkAndGetColumn<ColumnString>(column_src.get()))
{
auto col_res = ColumnString::create();
Impl::vectorNonConstNeedleReplacement(
Expand Down
32 changes: 32 additions & 0 deletions dbms/src/Functions/tests/gtest_strings_replace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,38 @@ try
toVec({" hello ", " h e llo", "hello ", " ", "hello, world"}),
toVec({" ", "h", "", "h", ","}),
toVec({"", "x", "xx", " ", ","})));

/// const src replacement
ASSERT_COLUMN_EQ(
toVec({"Good Night", "Bad Afternoon", "Good Afterwhile"}),
executeFunction(
"replaceAll",
toConst({"Good Afternoon"}),
toVec({"Afternoon", "Good", "noon"}),
toVec({"Night", "Bad", "while"})));

/// const src and needle replacement
ASSERT_COLUMN_EQ(
toVec({"Good Night", "Good Bad", "Good while"}),
executeFunction(
"replaceAll",
toConst({"Good Afternoon"}),
toConst({"Afternoon"}),
toVec({"Night", "Bad", "while"})));

/// const src and replace replacement
ASSERT_COLUMN_EQ(
toVec({"Good Night", "Night Afternoon", "Good AfterNight"}),
executeFunction(
"replaceAll",
toConst({"Good Afternoon"}),
toVec({"Afternoon", "Good", "noon"}),
toConst({"Night"})));

/// const src and replace replacement
ASSERT_COLUMN_EQ(
toVec({"Good Night"}),
executeFunction("replaceAll", toConst({"Good Afternoon"}), toConst({"Afternoon"}), toConst({"Night"})));
}
CATCH

Expand Down