From 2eca3ee6566f01b8d5293237a3475b2011b36a05 Mon Sep 17 00:00:00 2001 From: Sergey Zefirov <72864488+mariadb-SergeyZefirov@users.noreply.github.com> Date: Tue, 21 Nov 2023 23:46:03 +0300 Subject: [PATCH] fix(funcexp): MCOL-5607: JSON function use crashes query execution (#3028) JSON functions were implemented violating an assumption of their pureness, as they should not have any state. This concrete patch fixes implementation of JSON_VALUE function. --- .../basic/r/MCOL-5607-json-value-fix.result | 34 ++++++++++++++++ .../basic/t/MCOL-5607-json-value-fix.test | 32 +++++++++++++++ utils/funcexp/func_json_value.cpp | 40 +++++++++++-------- utils/funcexp/functor.h | 5 +++ utils/funcexp/functor_json.h | 8 +--- 5 files changed, 97 insertions(+), 22 deletions(-) create mode 100644 mysql-test/columnstore/basic/r/MCOL-5607-json-value-fix.result create mode 100644 mysql-test/columnstore/basic/t/MCOL-5607-json-value-fix.test diff --git a/mysql-test/columnstore/basic/r/MCOL-5607-json-value-fix.result b/mysql-test/columnstore/basic/r/MCOL-5607-json-value-fix.result new file mode 100644 index 000000000..2d8b58b81 --- /dev/null +++ b/mysql-test/columnstore/basic/r/MCOL-5607-json-value-fix.result @@ -0,0 +1,34 @@ +DROP DATABASE IF EXISTS MCOL5607; +CREATE DATABASE MCOL5607; +USE MCOL5607; +CREATE TABLE zu (hu TEXT) ENGINE = COLUMNSTORE; +INSERT INTO zu(hu) VALUES ('{}'), (NULL), ('{ "": "huh", "10001" : "10001", "10002" : "10001", "10003" : "10001", "10004" : "10001", "10005" : "10001", "10006" : "10001", "10007" : "10001", "10008" : "10001", "10009" : "10001", "10010" : "10001", "10011" : "10001", "10012" : "10001", "10013" : "10001", "10014" : "10001", "10015" : "10001", "10016" : "10001", "10017" : "10001", "10018" : "10001", "10019" : "10001", "10020" : "10001", "buga" : ""}'); +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +SELECT COUNT(*) FROM zu; +COUNT(*) +3145728 +SELECT COUNT(*) FROM zu wHERE +JSON_VALUE(hu, '$.buga') IS NOT NULL +AND JSON_VALUE(hu, '$.zhuga') IS NULL; +COUNT(*) +1048576 +DROP DATABASE MCOL5607; diff --git a/mysql-test/columnstore/basic/t/MCOL-5607-json-value-fix.test b/mysql-test/columnstore/basic/t/MCOL-5607-json-value-fix.test new file mode 100644 index 000000000..a344e4cee --- /dev/null +++ b/mysql-test/columnstore/basic/t/MCOL-5607-json-value-fix.test @@ -0,0 +1,32 @@ +--disable_warnings +DROP DATABASE IF EXISTS MCOL5607; +--enable_warnings +CREATE DATABASE MCOL5607; +USE MCOL5607; +CREATE TABLE zu (hu TEXT) ENGINE = COLUMNSTORE; +INSERT INTO zu(hu) VALUES ('{}'), (NULL), ('{ "": "huh", "10001" : "10001", "10002" : "10001", "10003" : "10001", "10004" : "10001", "10005" : "10001", "10006" : "10001", "10007" : "10001", "10008" : "10001", "10009" : "10001", "10010" : "10001", "10011" : "10001", "10012" : "10001", "10013" : "10001", "10014" : "10001", "10015" : "10001", "10016" : "10001", "10017" : "10001", "10018" : "10001", "10019" : "10001", "10020" : "10001", "buga" : ""}'); +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +INSERT INTO zu(hu) SELECT hu FROM zu; +SELECT COUNT(*) FROM zu; +SELECT COUNT(*) FROM zu wHERE + JSON_VALUE(hu, '$.buga') IS NOT NULL +AND JSON_VALUE(hu, '$.zhuga') IS NULL; +DROP DATABASE MCOL5607; diff --git a/utils/funcexp/func_json_value.cpp b/utils/funcexp/func_json_value.cpp index 715e750b4..37417b254 100644 --- a/utils/funcexp/func_json_value.cpp +++ b/utils/funcexp/func_json_value.cpp @@ -65,27 +65,17 @@ bool JSONPathWrapper::extract(std::string& ret, rowgroup::Row& row, execplan::SP const string& js = funcParamJS->data()->getStrVal(row, isNullJS); const string_view jsp = funcParamPath->data()->getStrVal(row, isNullPath); + if (isNullJS || isNullPath) return true; int error = 0; - if (!parsed) - { - if (!constant) - { - ConstantColumn* constCol = dynamic_cast(funcParamPath->data()); - constant = (constCol != nullptr); - } + if (json_path_setup(&p, getCharset(funcParamPath), (const uchar*)jsp.data(), + (const uchar*)jsp.data() + jsp.size())) + return true; - if (isNullPath || json_path_setup(&p, getCharset(funcParamPath), (const uchar*)jsp.data(), - (const uchar*)jsp.data() + jsp.size())) - return true; - - parsed = constant; - } - - JSONEgWrapper je(js, getCharset(funcParamJS)); + JSONEgWrapper je(getCharset(funcParamJS), (const uchar*)js.data(), (const uchar*)js.data() + js.size()); currStep = p.steps; @@ -112,11 +102,29 @@ CalpontSystemCatalog::ColType Func_json_value::operationType(FunctionParm& fp, return fp[0]->data()->resultType(); } +class JSONPathWrapperValue : public JSONPathWrapper +{ + public: + JSONPathWrapperValue() + { + } + virtual ~JSONPathWrapperValue() + { + } + + bool checkAndGetValue(JSONEgWrapper* je, string& res, int* error) override + { + return je->checkAndGetScalar(res, error); + } + +}; + string Func_json_value::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& isNull, execplan::CalpontSystemCatalog::ColType& type) { string ret; - isNull = JSONPathWrapper::extract(ret, row, fp[0], fp[1]); + JSONPathWrapperValue pw; + isNull = pw.extract(ret, row, fp[0], fp[1]); return isNull ? "" : ret; } } // namespace funcexp diff --git a/utils/funcexp/functor.h b/utils/funcexp/functor.h index 1cc73879f..b09ce16a6 100644 --- a/utils/funcexp/functor.h +++ b/utils/funcexp/functor.h @@ -54,6 +54,11 @@ namespace funcexp typedef std::vector FunctionParm; /** @brief Func class + * + * @desc IMPORTANT: functions are pure transformers, they should + * not have state shared between invocations. This is so because + * functions' objects are, essentially, singletons and the same + * objects will be used in diffeent threads. */ class Func { diff --git a/utils/funcexp/functor_json.h b/utils/funcexp/functor_json.h index a8a3ad92d..8ef62e66a 100644 --- a/utils/funcexp/functor_json.h +++ b/utils/funcexp/functor_json.h @@ -56,6 +56,7 @@ class JSONPathWrapper : public JSONPath { } virtual bool checkAndGetValue(JSONEgWrapper* je, std::string& ret, int* error) = 0; + public: bool extract(std::string& ret, rowgroup::Row& row, execplan::SPTP& funcParmJS, execplan::SPTP& funcParmPath); }; @@ -381,7 +382,7 @@ class Func_json_merge_patch : public Func_Str /** @brief Func_json_value class */ -class Func_json_value : public Func_Str, public JSONPathWrapper +class Func_json_value : public Func_Str { public: Func_json_value() : Func_Str("json_value") @@ -391,11 +392,6 @@ class Func_json_value : public Func_Str, public JSONPathWrapper { } - bool checkAndGetValue(JSONEgWrapper* je, string& res, int* error) override - { - return je->checkAndGetScalar(res, error); - } - execplan::CalpontSystemCatalog::ColType operationType( FunctionParm& fp, execplan::CalpontSystemCatalog::ColType& resultType) override;