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.

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

User defined function possible in generated_always_as column? #976

Closed
juandent opened this issue Mar 21, 2022 · 33 comments
Closed

User defined function possible in generated_always_as column? #976

juandent opened this issue Mar 21, 2022 · 33 comments
Labels

Comments

@juandent
Copy link
Contributor

juandent commented Mar 21, 2022

I don't think so, but just in case here it is:

the function:

using namespace std::chrono;

struct Age {
    int operator()(sys_days birth) const {
        auto diff = Today() - birth;
        return  duration_cast<years>(diff).count();
    }
  
    static const char* name() {
        return "AGE";
    }
};

and the make_storage() that won't compile:

    static auto storage = make_storage(dbFilePath,
        make_unique_index("name_unique.idx", &User::name ),
        make_table("user_table",
            make_column("id", &User::id, primary_key()),
            make_column("name", &User::name),
            make_column("born", &User::born),
            make_column("job", &User::job),
            make_column("blob", &User::blob),
            make_column("age", &User::age, generated_always_as(func<Age>(&User::born))),
            check(length(&User::name) > 2),
            check(c(&User::born) != 0),
            foreign_key(&User::job).references(&Job::id)),
        make_table("job", 
            make_column("id", &Job::id, primary_key(), autoincrement()),
            make_column("name", &Job::name, unique()),
            make_column("base_salary", &Job::base_salary)),
        make_table("user_view",
            make_column("name", &UserView::name)));

I get error:

1>C:\Components\klaus\include\sqlite_orm\sqlite_orm.h(14071,1): error C2664: 'std::chrono::sys_days sqlite_orm::row_extractor<std::chrono::sys_days,void>::extract(const char *)': cannot convert argument 1 from 'sqlite3_value *' to 'const char *'

@fnc12
Copy link
Owner

fnc12 commented Mar 21, 2022

can you use the same function outside of making a storage?

@fnc12 fnc12 added the question label Mar 21, 2022
@juandent
Copy link
Contributor Author

juandent commented Mar 21, 2022

no it does not compile in this line:

storage.create_scalar_function<Age>();

I get error:

error C2664: 'std::chrono::sys_days sqlite_orm::row_extractor<std::chrono::sys_days,void>::extract(const char *)': cannot convert argument 1 from 'sqlite3_value *' to 'const char *'

@juandent
Copy link
Contributor Author

this is the struct:

struct User {
    int64 id;
    std::string name;
    std::chrono::sys_days born;
    std::optional<int> job;
    std::vector<char > blob;
    int age;
};

@fnc12
Copy link
Owner

fnc12 commented Mar 21, 2022

probably you call create_scalar_function after calling sync_schema

@juandent
Copy link
Contributor Author

I think it has to do with use of sys_days for born column, since I added support for it that behaves like a string
if this is so, what could be done?

@juandent
Copy link
Contributor Author

juandent commented Mar 21, 2022

The error goes away when I add a method to struct row_extractor <std::chrono::sys_days> like this:

template<>
	struct row_extractor<std::chrono::sys_days> {
		std::chrono::sys_days extract(sqlite3_value* row_value)
		{
			return extract(row_value->u.zPType);
		}
		std::chrono::sys_days extract(const char* row_value) {
			auto sd = SysDaysFromString(row_value);
			if (sd != null_sys_day) {
				return sd;
			}
			else {
				throw std::runtime_error("incorrect date string (" + std::string(row_value) + ")");
			}
		}

		std::chrono::sys_days extract(sqlite3_stmt* stmt, int columnIndex) {
			auto str = sqlite3_column_text(stmt, columnIndex);
			return this->extract((const char*)str);
		}
	};

The problem is that sqlite3_value is opaque (it is defined inside sqlite3.c not in any header)

and what we need is to access the const char* so we can call extract(const char*) overload...

So?

@juandent
Copy link
Contributor Author

juandent commented Mar 21, 2022

Now i found that sqlite3_value has a union as first member:

struct sqlite3_value {
  union MemValue {
    double r;           /* Real value used when MEM_Real is set in flags */
    i64 i;              /* Integer value used when MEM_Int is set in flags */
    int nZero;          /* Extra zero bytes when MEM_Zero and MEM_Blob set */
    const char *zPType; /* Pointer type when MEM_Term|MEM_Subtype|MEM_Null */
    FuncDef *pDef;      /* Used only when flags==MEM_Agg */
  } u;
  u16 flags;          /* Some combination of MEM_Null, MEM_Str, MEM_Dyn, etc. */
  u8  enc;            /* SQLITE_UTF8, SQLITE_UTF16BE, SQLITE_UTF16LE */
  u8  eSubtype;       /* Subtype for this value */
  int n;              /* Number of characters in string value, excluding '\0' */
  char *z;            /* String or BLOB value */
  /* ShallowCopy only needs to copy the information above */
  char *zMalloc;      /* Space to hold MEM_Str or MEM_Blob if szMalloc>0 */
  int szMalloc;       /* Size of the zMalloc allocation */
  u32 uTemp;          /* Transient storage for serial_type in OP_MakeRecord */
  sqlite3 *db;        /* The associated database connection */
  void (*xDel)(void*);/* Destructor for Mem.z - only valid if MEM_Dyn */
#ifdef SQLITE_DEBUG
  Mem *pScopyFrom;    /* This Mem is a shallow copy of pScopyFrom */
  u16 mScopyFlags;    /* flags value immediately after the shallow copy */
#endif
};

so the address of the const char* in the union coincides with the address of the beginning of the sqlite3_value and I thought I would try something awful just in case it worked: (I don't propose this hack as a solution...I was just testing the waters)

		std::chrono::sys_days extract(sqlite3_value* row_value)
		{
			auto characters = reinterpret_cast<const char*>(row_value);
			return extract(characters);
		}

but alas, the pointed chars are a null string!! so it does not work!

Please some help!

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

Looks like the issues is not related to user defined functions at all. The issue is about binding std::chrono::sys_days as a column type. Please do what it takes to make std::chrono::sys_days work as a bindable type. Looks through examples related to bindings https://github.com/fnc12/sqlite_orm/blob/master/examples/enum_binding.cpp

@juandent
Copy link
Contributor Author

I followed that example to the dot .... I use s text_printer... Please take a look:

//  also we need transform functions to make string from enum..
inline std::string SysDaysToString(std::chrono::sys_days pt) {
	auto r = std::format("{:%F}", pt);
	return r;
}


constexpr auto null_sys_day = std::chrono::sys_days{ std::chrono::days{0} };


/**
 *  and enum from string. This function has nullable result cause
 *  string can be neither `male` nor `female`. Of course we
 *  won't allow this to happed but as developers we must have
 *  a scenario for this case.
 *  These functions are just helpers. They will be called from several places
 *  that's why I placed it separatedly. You can use any transformation type/form
 *  (for example BETTER_ENUM https://github.com/aantron/better-enums)
 */
inline std::chrono::sys_days SysDaysFromString(const std::string& s) {
	using namespace std::literals;
	using namespace std::chrono;

	std::stringstream ss{ s };
	std::chrono::sys_days tt;
	ss >> std::chrono::parse("%F"s, tt);
	if (!ss.fail())
	{
		return tt;
	}
	return null_sys_day;
}

/**
 *  This is where magic happens. To tell sqlite_orm how to act
 *  with SysDays we have to create a few service classes
 *  specializations (traits) in sqlite_orm namespace.
 */
namespace sqlite_orm {

	/**
	 *  First of all is a type_printer template class.
	 *  It is responsible for sqlite type string representation.
	 *  We want SysDays to be `TEXT` so let's just derive from
	 *  text_printer. Also there are other printers: real_printer and
	 *  integer_printer. We must use them if we want to map our type to `REAL` (double/float)
	 *  or `INTEGER` (int/long/short etc) respectively.
	 */
	template<>
	struct type_printer<std::chrono::sys_days> : public text_printer {};

	/**
	 *  This is a binder class. It is used to bind c++ values to sqlite queries.
	 *  Here we have to create sysday string representation and bind it as string.
	 *  Any statement_binder specialization must have `int bind(sqlite3_stmt*, int, const T&)` function
	 *  which returns bind result. Also you can call any of `sqlite3_bind_*` functions directly.
	 *  More here https://www.sqlite.org/c3ref/bind_blob.html
	 */
	template<>
	struct statement_binder<std::chrono::sys_days> {

		int bind(sqlite3_stmt* stmt, int index, const std::chrono::sys_days& value) {
			return statement_binder<std::string>().bind(stmt, index, SysDaysToString(value));

		}
	};

	/**
	 *  field_printer is used in `dump` and `where` functions. Here we have to create
	 *  a string from mapped object.
	 */
	template<>
	struct field_printer<std::chrono::sys_days> {
		std::string operator()(const std::chrono::sys_days& t) const {
			return SysDaysToString(t);
		}
	};

	/**
	 *  This is a reverse operation: here we have to specify a way to transform string received from
	 *  database to our sysdays object. Here we call `SysDaysFromString` and throw `std::runtime_error` if it returns
	 *  null_sys_day. Every `row_extractor` specialization must have `extract(const char*)` and `extract(sqlite3_stmt *stmt, int columnIndex)`
	 *  functions which return a mapped type value.
	 */
	template<>
	struct row_extractor<std::chrono::sys_days> {
#if 0 // THIS IS THE MiSSING FUNCTION
		std::chrono::sys_days extract(sqlite3_value* row_value)
		{
			auto characters = reinterpret_cast<const char*>(row_value);
			return extract(characters);
		}
#endif
		std::chrono::sys_days extract(const char* row_value) {
			auto sd = SysDaysFromString(row_value);
			if (sd != null_sys_day) {
				return sd;
			}
			else {
				throw std::runtime_error("incorrect date string (" + std::string(row_value) + ")");
			}
		}

		std::chrono::sys_days extract(sqlite3_stmt* stmt, int columnIndex) {
			auto str = sqlite3_column_text(stmt, columnIndex);
			return this->extract((const char*)str);
		}
	};
}

@juandent
Copy link
Contributor Author

I fixed the binding thans to @fnc12 ... now I have:

	template<>
	struct row_extractor<std::chrono::sys_days> {
#if 1
		std::chrono::sys_days extract(sqlite3_value* row_value)
		{
			auto characters = reinterpret_cast<const char*>(sqlite3_value_text(row_value));
			return extract(characters);
		}
                 // ...
}

and it works

@juandent
Copy link
Contributor Author

but the user defined function gives exception error at sync_schema()

[SQLite error:1] near "AGE": syntax error: SQL logic error

@juandent
Copy link
Contributor Author

juandent commented Mar 22, 2022

the necessary code to repro is:

using namespace std::chrono;

struct Age {
    int operator()(sys_days birth) const {
        const auto today = std::chrono::sys_days{ floor<std::chrono::days>(std::chrono::system_clock::now()) };
        auto diff = today - birth;
        return  duration_cast<years>(diff).count();
    }
  
    static const char* name() {
        return "AGE";
    }
};


    static auto storage = make_storage(dbFilePath,
        make_unique_index("name_unique.idx", &User::name ),
        make_table("user_table",
            make_column("id", &User::id, primary_key()),
            make_column("name", &User::name),
            make_column("born", &User::born),
            make_column("job", &User::job),
            make_column("blob", &User::blob),
            make_column("age", &User::age, generated_always_as(func<Age>(&User::born)) ),
            check(length(&User::name) > 2),
            check(c(&User::born) != 0),
            foreign_key(&User::job).references(&Job::id)),
        make_table("job", 
            make_column("id", &Job::id, primary_key(), autoincrement()),
            make_column("name", &Job::name, unique()),
            make_column("base_salary", &Job::base_salary)),
        make_table("user_view",
            make_column("name", &UserView::name)));


    storage.create_scalar_function<Age>();

    auto rows = storage.select(func<Age>(born));   // WORKS FINE HERE!!

        auto m = storage.sync_schema(true);              // THROW EXCEPTION HERE!!
}

@juandent
Copy link
Contributor Author

the persistent structs are:

struct User {
    int64 id;
    std::string name;
    std::chrono::sys_days born;
    std::optional<int> job;
    std::vector<char > blob;
    int age;
};

struct UserView {
    std::string name;
};

struct Job {
    int id;
    std::string name;
    double base_salary;
};

@juandent
Copy link
Contributor Author

The whole binding file is:

#include <chrono>

#include <sqlite_orm/sqlite_orm.h>
#include <sstream>
#include <regex>
#include <format>

inline std::chrono::sys_days Today()
{
	const auto today = std::chrono::sys_days{ floor<std::chrono::days>(std::chrono::system_clock::now()) };
	return today;
}

/**
 *  This is the date we want to map to our sqlite db.
  *  Let's make it `TEXT`
  */

//  also we need transform functions to make string from enum..
inline std::string SysDaysToString(std::chrono::sys_days pt) {
	auto r = std::format("{:%F}", pt);
	return r;
}

constexpr auto null_sys_day = std::chrono::sys_days{ std::chrono::days{0} };


/**
 *  and sys_days from string. This function has nullable result cause
 *  string can be neither `male` nor `female`. i have taken null_sys_day to
 *  correspond to null. Don't know if should use std::optional as return case??
 *  Of course we won't allow this to happen but as developers we must have
 *  a scenario for this case.
 *  These functions are just helpers. They will be called from several places
 *  that's why I placed it separatedly. You can use any transformation type/form
 *  (for example BETTER_ENUM https://github.com/aantron/better-enums)
 */
inline std::chrono::sys_days SysDaysFromString(const std::string& s) {
	using namespace std::literals;
	using namespace std::chrono;

	std::stringstream ss{ s };
	std::chrono::sys_days tt;
	ss >> std::chrono::parse("%F"s, tt);
	if (!ss.fail())
	{
		return tt;
	}
	return null_sys_day;     //// OR use std::optional<std::chrono::sys_days> and std::nullopt?
}

/**
 *  This is where magic happens. To tell sqlite_orm how to act
 *  with SysDays we have to create a few service classes
 *  specializations (traits) in sqlite_orm namespace.
 */
namespace sqlite_orm {

	/**
	 *  First of all is a type_printer template class.
	 *  It is responsible for sqlite type string representation.
	 *  We want SysDays to be `TEXT` so let's just derive from
	 *  text_printer. Also there are other printers: real_printer and
	 *  integer_printer. We must use them if we want to map our type to `REAL` (double/float)
	 *  or `INTEGER` (int/long/short etc) respectively.
	 */
	template<>
	struct type_printer<std::chrono::sys_days> : public text_printer {};

	/**
	 *  This is a binder class. It is used to bind c++ values to sqlite queries.
	 *  Here we have to create sysday string representation and bind it as string.
	 *  Any statement_binder specialization must have `int bind(sqlite3_stmt*, int, const T&)` function
	 *  which returns bind result. Also you can call any of `sqlite3_bind_*` functions directly.
	 *  More here https://www.sqlite.org/c3ref/bind_blob.html
	 */
	template<>
	struct statement_binder<std::chrono::sys_days> {

		int bind(sqlite3_stmt* stmt, int index, const std::chrono::sys_days& value) {
			return statement_binder<std::string>().bind(stmt, index, SysDaysToString(value));

		}
	};

	/**
	 *  field_printer is used in `dump` and `where` functions. Here we have to create
	 *  a string from mapped object.
	 */
	template<>
	struct field_printer<std::chrono::sys_days> {
		std::string operator()(const std::chrono::sys_days& t) const {
			return SysDaysToString(t);
		}
	};

	/**
	 *  This is a reverse operation: here we have to specify a way to transform string received from
	 *  database to our sysdays object. Here we call `SysDaysFromString` and throw `std::runtime_error` if it returns
	 *  null_sys_day. Every `row_extractor` specialization must have `extract(const char*)` and `extract(sqlite3_stmt *stmt, int columnIndex)`
	 *  functions which return a mapped type value.
	 */
	template<>
	struct row_extractor<std::chrono::sys_days> {
		std::chrono::sys_days extract(sqlite3_value* row_value)
		{
			auto characters = reinterpret_cast<const char*>(sqlite3_value_text(row_value));
			return extract(characters);
		}
		std::chrono::sys_days extract(const char* row_value) {
			auto sd = SysDaysFromString(row_value);
			if (sd != null_sys_day) {
				return sd;
			}
			else {
				throw std::runtime_error("incorrect date string (" + std::string(row_value) + ")");
			}
		}

		std::chrono::sys_days extract(sqlite3_stmt* stmt, int columnIndex) {
			auto str = sqlite3_column_text(stmt, columnIndex);
			return this->extract((const char*)str);
		}
	};
}

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

@juandent how would you write it using raw SQL?

@juandent
Copy link
Contributor Author

CREATE TABLE USERS (
   Born TEXT NOT NULL,
   Age TEXT GENERATED ALWAYS AS (AGE(Born))
//...
)

i think something like that

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

are you that it must work with raw SQLite? generated column must work only with deterministic functions. E.g. you cannot use RANDOM in it:
Снимок экрана 2022-03-22 в 23 46 06

@juandent
Copy link
Contributor Author

but I am not using random in user defined function AGE

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

ok. It is a question to SQLite team actually. But we also can test whether it works

@juandent
Copy link
Contributor Author

juandent commented Mar 22, 2022

but how can test it in a sql client if we can't get sync_schema to run and create the table with the function AGE?
the function AGE contains C++ code... we cannot create it in a sql client, it would not be the same...

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

I made a shorter version of your code to repro it:

    struct User {
        int id = 0;
        time_t birthdate = 0;
        int age = 0;
    };
    
    struct AgeFunction {
        int operator()(time_t birthdate) const {
            constexpr auto secondsPerYear = 60 * 60 * 24 * 365;
            return int((::time(nullptr) - birthdate) / secondsPerYear);
        }
        
        static std::string_view name() {
            return "AGE";
        }
    };
    
    auto storage = make_storage({},
                                make_table("users",
                                           make_column("id", &User::id, primary_key()),
                                           make_column("birthdate", &User::birthdate),
                                           make_column("age", &User::age, as(func<AgeFunction>(&User::birthdate)))));
    storage.create_scalar_function<AgeFunction>();
    storage.sync_schema();

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

  1. there is a bug with GENERATED ALWAYS serialization. I'll fix it.
  2. one cannot use user defined function inside generated column cause it requires only deterministic functions cause after fixing the serialization bug I get non-deterministic functions prohibited in generated

@fnc12 fnc12 added bug and removed question labels Mar 22, 2022
@juandent
Copy link
Contributor Author

sh*t!

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

you can write an expression which evaluates age right inside GENERATED ALWAYS constraint. I don't see any critical problem here

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

serialization fix is here #978

@juandent
Copy link
Contributor Author

juandent commented Mar 22, 2022

not sure if we could... we would need to write the equivalent of:

        const auto today = std::chrono::sys_days{ floor<std::chrono::days>(std::chrono::system_clock::now()) };
        auto diff = today - birth;
        return  duration_cast<years>(diff).count();

inside the generated_always_as!

@juandent
Copy link
Contributor Author

so far I have:

make_column("age", &User::age, generated_always_as(
                (sys_days{ floor<days>(system_clock::now()) } - c(&User::born)))),
// .....

which compiles but it does not mean anything

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

replace system_clock::now() with correct date time function from SQLite https://www.sqlite.org/lang_datefunc.html

@juandent
Copy link
Contributor Author

will look into that .... I was trying to stay within the new way of dealing with dates/times: the chrono library

@fnc12
Copy link
Owner

fnc12 commented Mar 22, 2022

you can do it but avoiding using it inside generated_always_as

@juandent
Copy link
Contributor Author

I think we agreed that we could not place user defined functions in generated_always_as column, because sqlite cannot see the code and it is interpreted as non-deterministic. If I am correct please confirm so as to close this issue...

@fnc12
Copy link
Owner

fnc12 commented Mar 26, 2022

Almost. We could not place user defined functions in generated_always_as expression cause one can place only deterministic functions inside GENERATED ALWAYS AS expression in SQLite but user defined functions are non-deterministic

@juandent
Copy link
Contributor Author

ok, yes very clear!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants