MCOL-4814 Add a cmake build option to enable LZ4 compression.

[denis0x0D]

This patch adds an option for cmake flags to enable lz4 compression.
To enable LZ4 compression add "-DWITH_LZ4=1" to cmake flags.
By default CS without LZ4.

[vuvova]

• always MESSAGE_ONCE() please
• mcsconfig.h.in is better than ADD_DEFINITION

[mariadb-DenisKhalikov]

always MESSAGE_ONCE() please

thanks fixed.

mcsconfig.h.in is better than ADD_DEFINITION

Probably I didn't get what you mean, do you want to update cmake configure to search for lz4 and define "HAVE_LZ4" by this? As far as I understood InnoDB also use "ADD_DEFINITION"
Thanks.

[vuvova]

There are two ways to pass a definition from cmake to the compiler. You can use ADD_DEFINITION or you can use config.h template. ColumnStore already has it, mcsconfig.h.in, search for "ZLIB" there, for example. It has hundreds of definitions, putting them all into the command line with ADD_DEFINITION would create a really really long command line.

ADD_DEFINITION should be used when you have only few definitions and don't want to bother creating a config.h template — this is InnoDB case, InnoDB doesn't use config.h template. Or when you need something to be defined first, before the very first #include. Otherwise put your definitions in the already existing mcsconfig.h.in

[mariadb-DenisKhalikov]

@vuvova thanks for explanation. But in our case we can have situation when we have lz4 installed and we don't want to build with lZ4 (-DWITH_COLUMNSTORE_LZ4=OFF)?
So, the mcsconfig.h is cached and defined HAVE_LZ4 even if I try to build CS without LZ4 on the next build

+SET(LINK_WITH_LZ4 OFF)
+IF (WITH_COLUMNSTORE_LZ4 STREQUAL "ON" OR WITH_COLUMNSTORE_LZ4 STREQUAL "AUTO")
+    FIND_PACKAGE(LZ4)
+    IF (NOT LZ4_FOUND)
+        IF (WITH_COLUMNSTORE_LZ4 STREQUAL "AUTO")
+            MESSAGE_ONCE(STATUS "LZ4 not found, building without LZ4")
+        ELSE()
+            MESSAGE_ONCE(FATAL_ERROR "LZ4 not found.")
+        ENDIF()
+    ELSE()
+        MESSAGE_ONCE(STATUS "Building with LZ4")
+        CHECK_INCLUDE_FILE_CXX(lz4.h HAVE_LZ4)
+        SET(LINK_WITH_LZ4 ON)
+    ENDIF()
+ELSE()
+    MESSAGE_ONCE(STATUS "Building without LZ4")
+ENDIF()

[vuvova]

No, it's the same thing. cmake will regenerate mcsconfig.h every time, so it'll show the latest settings

[mariadb-DenisKhalikov]

Sorry, but I cannot reproduce your suggestion locally,
If I add to mcsconfig.h.in
#cmakedefine HAVE_LZ4
it puts
#define HAVE_LZ4
no matter what I add to cmake flag, OFF, ON, AUTO.
So, I cannot have a working build locally with OFF and LZ4 installed

[vuvova]

HAVE_LZ4 needs to be persistent. Not

SET(LINK_WITH_LZ4 TRUE)

but

SET(LINK_WITH_LZ4 TRUE CACHE INTERNAL "")

and yes, I've tried it before suggesting :)

[mariadb-DenisKhalikov]

@vuvova thanks, finally I found the bug, sorry for the stupid questions.

[vuvova]

you need CACHE here too. Because you set the cached value to TRUE but never reset it to FALSE even if the user decides to recompile without lz4.

Also, I don't really understand why you need two flags with exactly the same value, but whatever.

[mariadb-DenisKhalikov]

fixed.

[vuvova]

looks good to me