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

UNINDEXED column constraint #1279

Merged
merged 3 commits into from
May 5, 2024
Merged

UNINDEXED column constraint #1279

merged 3 commits into from
May 5, 2024

Conversation

fnc12
Copy link
Owner

@fnc12 fnc12 commented May 4, 2024

This PR adds UNINDEXED column constraint which is used in FTS virtual table.
To improve: add static assertion in table_t to ensure that its columns do not contain unindexed_t.

@fnc12 fnc12 requested a review from trueqbit May 4, 2024 16:35
ss << " " << type_printer<field_type_t<column_type>>().print();
ss << streaming_column_constraints(
call_as_template_base<column_constraints>(polyfill::identity{})(column),
column.is_not_null(),
context);
} else {
using constraints_tuple = typename column_type::constraints_type;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trueqbit probably this algorithm can be implemented way more beautiful. Do you have any idea?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed ;)

        template<class G, class S, class... Op>
        struct statement_serializer<column_t<G, S, Op...>, void> {
            using statement_type = column_t<G, S, Op...>;

            template<class Ctx>
            std::string operator()(const statement_type& column, const Ctx& context) const {
                std::stringstream ss;
                ss << streaming_identifier(column.name);
                if(!context.fts5_columns) {
                    ss << " " << type_printer<field_type_t<column_field<G, S>>>().print();
                }
                ss << streaming_column_constraints(
                    call_as_template_base<column_constraints>(polyfill::identity{})(column),
                    column.is_not_null(),
                    context);
                return ss.str();
            }
        };

And the accompanying streaming_column_constraints in serializing_util.h:

        template<class... Op, class Ctx>
        std::ostream& operator<<(std::ostream& ss,
                                 std::tuple<const streaming<stream_as::column_constraints>&,
                                            const column_constraints<Op...>&,
                                            const bool&,
                                            Ctx> tpl) {
            const auto& column = std::get<1>(tpl);
            const bool& isNotNull = std::get<2>(tpl);
            auto& context = std::get<3>(tpl);

            using constraints_tuple = decltype(column.constraints);
            if(!context.fts5_columns) {
                iterate_tuple(column.constraints, [&ss, &context](auto& constraint) {
                    ss << ' ' << serialize(constraint, context);
                });
                constexpr bool hasExplicitNullableConstraint =
                    mpl::invoke_t<mpl::disjunction<check_if_has_type<null_t>, check_if_has_type<not_null_t>>,
                                  constraints_tuple>::value;
                if SQLITE_ORM_CONSTEXPR_IF(!hasExplicitNullableConstraint) {
                    if(isNotNull) {
                        ss << " NOT NULL";
                    } else {
                        ss << " NULL";
                    }
                }
            } else {
                constexpr bool hasUnindexedOption =
                    mpl::invoke_t<check_if_has_type<unindexed_t>, constraints_tuple>::value;
                if SQLITE_ORM_CONSTEXPR_IF(hasUnindexedOption) {
                    ss << " UNINDEXED";
                }
            }

            return ss;
        }

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@@ -30,8 +30,6 @@ TEST_CASE("virtual table") {
Post{"SQLite Tutorial", "Help you learn SQLite quickly and effectively"},
};

// storage.remove_all<Post>();
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed old artifact


using namespace sqlite_orm;

TEST_CASE("statement_serializer table_t") {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test case is not related to PRs topic but I wanted to add this last time in the next PR so here we are

@@ -268,3 +268,184 @@ TEST_CASE("non-unique DBOs") {
make_column("time", &Record::setTime, &Record::time)));
db.sync_schema();
}

TEST_CASE("insert") {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved all remaining tests from tests2.cpp here cause they are related to storage_t

using statement_type = unindexed_t;

template<class Ctx>
std::string operator()(const statement_type& c, const Ctx& context) const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should return serialize_result_type.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

using context_t = internal::serializer_context<db_objects_t>;
context_t context{dbObjects};
value = internal::serialize(table, context);
expected = "CREATE TABLE \"users\" (\"id\" INTEGER NOT NULL, \"name\" TEXT NOT NULL)";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use raw string literals, please!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

using context_t = internal::serializer_context<db_objects_t>;
context_t context{dbObjects};
value = internal::serialize(table, context);
expected = "CREATE TABLE \"users\" (\"id\" INTEGER NOT NULL, \"name\" TEXT NOT NULL) WITHOUT ROWID";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use raw string literals, please!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

SECTION("simple") {
auto node = using_fts5(make_column("title", &Post::title), make_column("body", &Post::body));
value = serialize(node, context);
expected = "USING FTS5(\"title\", \"body\")";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use raw string literals, please!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

SECTION("unindexed") {
auto node = using_fts5(make_column("title", &Post::title), make_column("body", &Post::body, unindexed()));
value = serialize(node, context);
expected = "USING FTS5(\"title\", \"body\" UNINDEXED)";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use raw string literals, please!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -41,7 +41,6 @@ add_executable(unit_tests
xdestroy_handling.cpp
sync_schema_tests.cpp
tests.cpp
tests2.cpp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. I can tell you why all ungrouped tests were divided up to several files. Cause Windows CI couldn't handle such a huge .cpp file to compile 🤦

@@ -8,7 +8,7 @@ namespace sqlite_orm {
bool replace_bindable_with_question = false;
bool skip_table_name = true;
bool use_parentheses = true;
bool skip_types_and_constraints = false;
bool skip_types_and_constraints_except_unindexed = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename this flag into something like fts5_columns? Also see below when checking it.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

ss << " " << type_printer<field_type_t<column_type>>().print();
ss << streaming_column_constraints(
call_as_template_base<column_constraints>(polyfill::identity{})(column),
column.is_not_null(),
context);
} else {
using constraints_tuple = typename column_type::constraints_type;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed ;)

        template<class G, class S, class... Op>
        struct statement_serializer<column_t<G, S, Op...>, void> {
            using statement_type = column_t<G, S, Op...>;

            template<class Ctx>
            std::string operator()(const statement_type& column, const Ctx& context) const {
                std::stringstream ss;
                ss << streaming_identifier(column.name);
                if(!context.fts5_columns) {
                    ss << " " << type_printer<field_type_t<column_field<G, S>>>().print();
                }
                ss << streaming_column_constraints(
                    call_as_template_base<column_constraints>(polyfill::identity{})(column),
                    column.is_not_null(),
                    context);
                return ss.str();
            }
        };

And the accompanying streaming_column_constraints in serializing_util.h:

        template<class... Op, class Ctx>
        std::ostream& operator<<(std::ostream& ss,
                                 std::tuple<const streaming<stream_as::column_constraints>&,
                                            const column_constraints<Op...>&,
                                            const bool&,
                                            Ctx> tpl) {
            const auto& column = std::get<1>(tpl);
            const bool& isNotNull = std::get<2>(tpl);
            auto& context = std::get<3>(tpl);

            using constraints_tuple = decltype(column.constraints);
            if(!context.fts5_columns) {
                iterate_tuple(column.constraints, [&ss, &context](auto& constraint) {
                    ss << ' ' << serialize(constraint, context);
                });
                constexpr bool hasExplicitNullableConstraint =
                    mpl::invoke_t<mpl::disjunction<check_if_has_type<null_t>, check_if_has_type<not_null_t>>,
                                  constraints_tuple>::value;
                if SQLITE_ORM_CONSTEXPR_IF(!hasExplicitNullableConstraint) {
                    if(isNotNull) {
                        ss << " NOT NULL";
                    } else {
                        ss << " NULL";
                    }
                }
            } else {
                constexpr bool hasUnindexedOption =
                    mpl::invoke_t<check_if_has_type<unindexed_t>, constraints_tuple>::value;
                if SQLITE_ORM_CONSTEXPR_IF(hasUnindexedOption) {
                    ss << " UNINDEXED";
                }
            }

            return ss;
        }

@fnc12 fnc12 requested a review from trueqbit May 5, 2024 06:30
@fnc12 fnc12 merged commit b8b5bfb into dev May 5, 2024
3 checks passed
@trueqbit trueqbit deleted the feature/unindexed branch May 5, 2024 19:28
@trueqbit trueqbit linked an issue May 8, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FTS5 Options: Unindexed columns, prefix indexes, tokenizers & content
2 participants