[MCOL-5437] columnstore fails to compile to due old cs->casedn_multiply use - Jira

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Blocker
Resolution: Fixed
Affects Version/s: None
Fix Version/s: 6.4.7- CS only
Component/s: MariaDB Server
Labels:
None

Description

https://buildbot.mariadb.org/#/builders/148/builds/19518/steps/5/logs/stdio
/home/buildbot/amd64-debian-10-deb-autobake/build/storage/columnstore/columnstore/utils/funcexp/func_lcase.cpp: In member function ‘virtual std::__cxx11::string funcexp::Func_lcase::getStrVal(rowgroup::Row&, funcexp::FunctionParm&, bool&, execplan::CalpontSystemCatalog::ColType&)’:
/home/buildbot/amd64-debian-10-deb-autobake/build/storage/columnstore/columnstore/utils/funcexp/func_lcase.cpp:56:33: error: invalid use of member function ‘uint charset_info_st::casedn_multiply() const’ (did you forget the ‘()’ ?)
uint64_t bufLen = inLen * cs->casedn_multiply;
~~~~^~~~~~~~~~~~~~~
make[4]: *** [storage/columnstore/columnstore/utils/funcexp/CMakeFiles/funcexp.dir/build.make:742: storage/columnstore/columnstore/utils/funcexp/CMakeFiles/funcexp.dir/func_lcase.cpp.o] Error 1

With ~~MDEV-30661~~ cs->casedn_multiply became a function rather than a structure member.

Attachments

Issue Links

is caused by

MDEV-30661 UPPER() returns an empty string for U+0251 in uca1400 collations for utf8

Closed

Activity

Ascending order - Click to sort in descending order

Daniel Black created issue - 2023-02-23 03:48

Daniel Black made changes - 2023-02-23 03:48

Field	Original Value	New Value
Link		This issue is caused by ~~MDEV-30661~~ [ ~~MDEV-30661~~ ]

Daniel Black made changes - 2023-02-23 04:02

Priority

Major [ 3 ]

Blocker [ 1 ]

Alexander Barkov made changes - 2023-02-23 19:17

Affects Version/s

6.4.7- CS only [ 28600 ]

Alexander Barkov made changes - 2023-02-23 19:17

Affects Version/s

6.4.7- CS only [ 28600 ]

Alexander Barkov added a comment - 2023-02-23 19:18

This patch should fix the problem:

diff --git a/utils/funcexp/func_lcase.cpp b/utils/funcexp/func_lcase.cpp

index 7de396e47..ec1df49c3 100644

--- a/utils/funcexp/func_lcase.cpp

+++ b/utils/funcexp/func_lcase.cpp

@@ -53,7 +53,7 @@ std::string Func_lcase::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& is

   CHARSET_INFO* cs = colType.getCharset();

   uint64_t inLen = tstr.length();

-  uint64_t bufLen = inLen * cs->casedn_multiply;

+  uint64_t bufLen = inLen * cs->casedn_multiply();

   char* outBuf = new char[bufLen];

   uint64_t outLen = cs->casedn(tstr.c_str(), inLen, outBuf, bufLen);

diff --git a/utils/funcexp/func_ucase.cpp b/utils/funcexp/func_ucase.cpp

index 4d78fb8b2..9ad1e22bc 100644

--- a/utils/funcexp/func_ucase.cpp

+++ b/utils/funcexp/func_ucase.cpp

@@ -62,7 +62,7 @@ std::string Func_ucase::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& is

   CHARSET_INFO* cs = colType.getCharset();

   uint64_t inLen = tstr.length();

-  uint64_t bufLen = inLen * cs->caseup_multiply;

+  uint64_t bufLen = inLen * cs->caseup_multiply();

   char* outBuf = new char[bufLen];

   uint64_t outLen = cs->caseup(tstr.c_str(), inLen, outBuf, bufLen);

Alexander Barkov added a comment - 2023-02-23 19:18 This patch should fix the problem: diff --git a/utils/funcexp/func_lcase.cpp b/utils/funcexp/func_lcase.cpp index 7de396e47..ec1df49c3 100644 --- a/utils/funcexp/func_lcase.cpp +++ b/utils/funcexp/func_lcase.cpp @@ -53,7 +53,7 @@ std::string Func_lcase::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool & is CHARSET_INFO* cs = colType.getCharset(); uint64_t inLen = tstr.length(); - uint64_t bufLen = inLen * cs->casedn_multiply; + uint64_t bufLen = inLen * cs->casedn_multiply(); char * outBuf = new char [bufLen]; uint64_t outLen = cs->casedn(tstr.c_str(), inLen, outBuf, bufLen); diff --git a/utils/funcexp/func_ucase.cpp b/utils/funcexp/func_ucase.cpp index 4d78fb8b2..9ad1e22bc 100644 --- a/utils/funcexp/func_ucase.cpp +++ b/utils/funcexp/func_ucase.cpp @@ -62,7 +62,7 @@ std::string Func_ucase::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool & is CHARSET_INFO* cs = colType.getCharset(); uint64_t inLen = tstr.length(); - uint64_t bufLen = inLen * cs->caseup_multiply; + uint64_t bufLen = inLen * cs->caseup_multiply(); char * outBuf = new char [bufLen]; uint64_t outLen = cs->caseup(tstr.c_str(), inLen, outBuf, bufLen);

Sergei Golubchik made changes - 2023-02-23 20:15

Assignee

Alexander Barkov [ bar ]

Sergei Golubchik added a comment - 2023-02-23 20:16

Ideally we need a solution that works for all server versions starting from 10.6

Sergei Golubchik added a comment - 2023-02-23 20:16 Ideally we need a solution that works for all server versions starting from 10.6

Todd Stoffel (Inactive) made changes - 2023-02-23 20:44

Rank

Ranked higher

Todd Stoffel (Inactive) made changes - 2023-02-23 20:45

Rank

Ranked lower

Daniel Black added a comment - 2023-02-23 20:50 - edited

Like:

#if MYSQL_VERSION_ID >= 101004

uint64_t bufLen = inLen * cs->casedn_multiply();

#else

uint64_t bufLen = inLen * cs->casedn_multiply;

#endif

(except neater)

Daniel Black added a comment - 2023-02-23 20:50 - edited Like: #if MYSQL_VERSION_ID >= 101004 uint64_t bufLen = inLen * cs->casedn_multiply(); #else uint64_t bufLen = inLen * cs->casedn_multiply; #endif (except neater)

Sergei Golubchik made changes - 2023-02-24 09:12

Assignee

Alexander Barkov [ bar ]

Roman [ drrtuy ]

Sergei Golubchik made changes - 2023-02-24 09:12

Fix Version/s

6.4.7- CS only [ 28600 ]

Roman made changes - 2023-02-24 14:30

Status

Open [ 1 ]

In Progress [ 3 ]

Andrew Hutchings (Inactive) added a comment - 2023-03-03 10:35

danblack That works for 10.10, but 10.11 and 11.0 are also broken in the same way. The `#if` might get quite complicated quite quickly.

Andrew Hutchings (Inactive) added a comment - 2023-03-03 10:35 danblack That works for 10.10, but 10.11 and 11.0 are also broken in the same way. The `#if` might get quite complicated quite quickly.

Alexander Barkov added a comment - 2023-03-03 11:22 - edited

TheLinuxJedi, danblack, something like this should probably work:

/* The API change happened in 10.10.4, 10.11.3, 11.0.2 */

#if ((MYSQL_VERSION_ID >= 101004 && MYSQL_VERSION_ID < 101099) || \

      (MYSQL_VERSION_ID >= 101103 && MYSQL_VERSION_ID < 101199) || \

      MYSQL_VERSION_ID >= 110002)

uint64_t bufLen = inLen * cs->casedn_multiply();

#else

uint64_t bufLen = inLen * cs->casedn_multiply;

#endif

Alexander Barkov added a comment - 2023-03-03 11:22 - edited TheLinuxJedi , danblack , something like this should probably work: /* The API change happened in 10.10.4, 10.11.3, 11.0.2 */ #if ((MYSQL_VERSION_ID >= 101004 && MYSQL_VERSION_ID < 101099) || \ (MYSQL_VERSION_ID >= 101103 && MYSQL_VERSION_ID < 101199) || \ MYSQL_VERSION_ID >= 110002) uint64_t bufLen = inLen * cs->casedn_multiply(); #else uint64_t bufLen = inLen * cs->casedn_multiply; #endif

Alexander Barkov added a comment - 2023-03-03 11:28 - edited

Another option (to avoid this cumbersome #if duplication) would be to move this code into a shared header file:

/* The API change happened in 10.10.4, 10.11.3, 11.0.2 */

#if ((MYSQL_VERSION_ID >= 101004 && MYSQL_VERSION_ID < 101099) || \

      (MYSQL_VERSION_ID >= 101103 && MYSQL_VERSION_ID < 101199) || \

      MYSQL_VERSION_ID >= 110002)

#define CS_CASEDN_MULTIPLY(cs) ((cs)->casedn_multiply())

#define CS_CASEUP_MULTIPLY(cs) ((cs)->caseup_multiply())

#else

#define CS_CASEDN_MULTIPLY(cs) ((cs)->casedn_multiply)

#define CS_CASEUP_MULTIPLY(cs) ((cs)->caseup_multiply)

#endif

and include it from func_lcase.cpp and func_ucase.cpp.

Alexander Barkov added a comment - 2023-03-03 11:28 - edited Another option (to avoid this cumbersome #if duplication) would be to move this code into a shared header file: /* The API change happened in 10.10.4, 10.11.3, 11.0.2 */ #if ((MYSQL_VERSION_ID >= 101004 && MYSQL_VERSION_ID < 101099) || \ (MYSQL_VERSION_ID >= 101103 && MYSQL_VERSION_ID < 101199) || \ MYSQL_VERSION_ID >= 110002) #define CS_CASEDN_MULTIPLY(cs) ((cs)->casedn_multiply()) #define CS_CASEUP_MULTIPLY(cs) ((cs)->caseup_multiply()) #else #define CS_CASEDN_MULTIPLY(cs) ((cs)->casedn_multiply) #define CS_CASEUP_MULTIPLY(cs) ((cs)->caseup_multiply) #endif and include it from func_lcase.cpp and func_ucase.cpp .

Daniel Black added a comment - 2023-03-03 23:25

There's no point over complicating it. The git submodule ties the columnstore to the server version. So if you when back to 11.0.1, the git submodule would go to a previous columnstore version that didn't have this change.

Daniel Black added a comment - 2023-03-03 23:25 There's no point over complicating it. The git submodule ties the columnstore to the server version. So if you when back to 11.0.1, the git submodule would go to a previous columnstore version that didn't have this change.

Roman added a comment - 2023-03-23 17:53

danblack TheLinuxJedi AFAIK the issue has been resolved. Am I right?

Roman added a comment - 2023-03-23 17:53 danblack TheLinuxJedi AFAIK the issue has been resolved. Am I right?

Daniel Black added a comment - 2023-03-23 21:36 - edited

Yes. and serg committed the submodule update in the server with commit 5f33351f48557fa09d5ebcf9864c33bfa296cd8e.

Thanks for the fix.

Daniel Black added a comment - 2023-03-23 21:36 - edited Yes. and serg committed the submodule update in the server with commit 5f33351f48557fa09d5ebcf9864c33bfa296cd8e . Thanks for the fix.

Todd Stoffel (Inactive) made changes - 2023-04-05 04:34

Rank

Ranked higher

Roman made changes - 2023-04-11 21:19

Resolution		Fixed [ 1 ]
Status	In Progress [ 3 ]	Closed [ 6 ]

People

Assignee:: Roman

Reporter:: Daniel Black

Votes:: 0 Vote for this issue

Watchers:: 5 Start watching this issue

Dates

Created:: 2023-02-23 03:48

Updated:: 2023-05-08 18:57

Resolved:: 2023-04-11 21:19

Git Integration

Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.

MariaDB ColumnStore

Details

Description

Attachments

Issue Links

Activity

People

Dates

Git Integration