[MCOL-5437] columnstore fails to compile to due old cs->casedn_multiply use Created: 2023-02-23  Updated: 2023-05-08  Resolved: 2023-04-11

Status: Closed
Project: MariaDB ColumnStore
Component/s: MariaDB Server
Affects Version/s: None
Fix Version/s: 6.4.7- CS only

Type: Bug Priority: Blocker
Reporter: Daniel Black Assignee: Roman
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Problem/Incident
is caused by MDEV-30661 UPPER() returns an empty string for U... Closed

 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.

 Comments   
Comment by Alexander Barkov [ 2023-02-23 ]

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);

Comment by Sergei Golubchik [ 2023-02-23 ]

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

Comment by Daniel Black [ 2023-02-23 ]

Like:

#if MYSQL_VERSION_ID >= 101004
 
uint64_t bufLen = inLen * cs->casedn_multiply();
 
#else
 
uint64_t bufLen = inLen * cs->casedn_multiply;
 
#endif

(except neater)

Comment by Andrew Hutchings [ 2023-03-03 ]

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.

Comment by Alexander Barkov [ 2023-03-03 ]

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

Comment by Alexander Barkov [ 2023-03-03 ]

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.

Comment by Daniel Black [ 2023-03-03 ]

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.

Comment by Roman [ 2023-03-23 ]

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

Comment by Daniel Black [ 2023-03-23 ]

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

Thanks for the fix.

Generated at Thu Feb 08 02:57:53 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.