Sqlite_orm: Allow deletion with a set of composite keys

[TwelveNights]

I'm currently having an issue where I have something like:

typedef struct {
    std::string serial_number;
    std::string device_id;
} Device;

Would there be a way for me to create a query similar to
DELETE FROM devices WHERE (serial_number, device_id) IN (VALUES ('abc', '123'), ('def', '456'))?

[fnc12]

how would you write it without sqlite_orm with raw SQL?

[fnc12]

@TwelveNights are you there?

[TwelveNights]

Hi @fnc12, I missed the notification in my inbox, sorry. The raw SQL is the example I showed above. I've ran this via the sqlite3 CLI

[fnc12]

oh man. I was sure that IN doesn't support two arguments. Thank you. I shall think how to perform it

[fnc12]

@TwelveNights where did you know about such type of usage of IN operator? I cannot find any information about IN with two arguments but it also worked for me when I ran it in my SQLite client

[TwelveNights]

I found some information. It seems like the left operand of IN can be a row value. 8. The IN and NOT IN operators also provides more details on the usage of IN. Also, it seems like VALUES is optional.

[fnc12]

thanks. I shall add it soon. I need to think about syntax first. If you have any ideas about how to make API for this please feel free to post it here. Thanks

[fnc12]

DELETE FROM devices WHERE (serial_number, device_id) IN (VALUES ('abc', '123'), ('def', '456'))

What if API would be like this:

storage.remove_all<Device>(where(in(std::make_tuple(&Device::serialNumber, &Device::deviceId), std::vector<std::tuple<std::string, std::string>>{std::make_tuple("abc", "123"), std::make_tuple("def", "456")})));

?

[TwelveNights]

That makes sense to me. Just out of curiousity; for std::make_tuple(&Device::serialNumber, &Device::deviceId), is there something that could be re-used from what already exists? Are there composite keys available in GROUP BY, for example?

[fnc12]

I don't understand. Please provide an example in SQL/C++

[TwelveNights]

Sorry, I was mistaken. I thought GROUP BY could be used like GROUP BY (serial_number, device_id). Please ignore what I said.

[fnc12]

@TwelveNights probably it is possible. Give it a try with SQLite client and sqlite_orm

[fnc12]

@TwelveNights https://github.com/fnc12/sqlite_orm/pull/534

[TwelveNights]

Forwarding the context down to each value looks pretty neat!

[fnc12]

merged. Please check it out.

[fnc12]

is the issue actual?

[TwelveNights]

is the issue actual?

Let me pull the changes.

[TwelveNights]

@fnc12, does it work if we pass a std::vector<std::tuple<...,>> as well? I noticed in your test you have values(...), but the vector fails with row value misused: SQL logic error.

[fnc12]

@TwelveNights how? Like this:

storage.remove_all<Device>(where(in(std::make_tuple(&Device::serialNumber, &Device::deviceId),
                                         values(myVector))));

?

[michelle-du]

@fnc12 Doing something like that gives me a compiler error.

error: ‘sqlite_orm::internal::statement_serializator<std::vector<std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >, void> serializator’ has incomplete type
             statement_serializator<T> serializator;

Also, would it be possible to directly pass in a select statement instead of values(myVector)

[fnc12]

@michelle-du inserting vector inside values call is not supported right now. Right now only tuple is available cause it is compile time container but vector is not. I can add vector support but this kind of queries will not support get function for obtaining prepared statementsfields just like it is done withinanddynamic_order_by`.

How do you expect to pass in a select statement instead of values(myVector)? Please show me raw SQL query - it will help me to understand you and to make an improvement. Thanks

[TwelveNights]

The use-case for this delete is that the number of values is dynamic (e.g. it could be VALUES ('123', 'abc'), ('456', 'def') or VALUES ('123', 'abc'))

[fnc12]

@TwelveNights ok I shall add vector support as well like this:

storage.remove_all<Device>(where(in(std::make_tuple(&Device::serialNumber, &Device::deviceId),
                                         values(myVector))));

[michelle-du]

@fnc12 Something like

DELETE FROM device
WHERE (serial_number, device_id) IN 
(SELECT serial_number, device_id 
FROM manufacturer
WHERE manufacturer_id = 'abc')

[fnc12]

@michelle-du try it as is - it must work:

storage.remove_all<Device>(where(in(std::make_tuple(...), 
    select(columns(&Device::serialNumber, &Device::deviceId), 
    where(is_equal(&Device::manufacturerId, "abc"))))));

[fnc12]

So I've added vector support. Please check out the PR https://github.com/fnc12/sqlite_orm/pull/555

[fnc12]

merged into dev branch. Please check it out

[fnc12]

is the issue actual?

[fnc12]

closing cause it is fixed. If you still have any issue please feel free to post it and reopen the issue. Thanks

[FunDoor]

ms vc141 compile error: 'tupleSize' cannot be implicitly captured because no default capture mode has been specified
I just put the 'tupleSize' into capture list and compile success.
But remove_all still has no effect for deletion.

[fnc12]

@FunDoor what line? Can you please show where did you put tupleSize exactly?

[FunDoor]

@fnc12 line 11117

const auto tupleSize = int(std::tuple_size<statement_type>::value);
iterate_tuple(statement, [&context, &index, &ss](auto &value) {
    ss << serialize(value, context);
    if(index < tupleSize - 1) {
        ss << ", ";
    }
    ++index;
});

[fnc12]

hm looks weird cause it works on CI. Probably the reason is that this constant is optimized to constexpr constant which is accessible everywhere without a capture. I shall make a PR with fix. Thank you

[fnc12]

when I add tupleSize inside capture I get an error with clang in Xcode Lambda capture 'tupleSize' is not required to be captured for this use. What if you remove it from capture but add constexpr to tupleSize?

[FunDoor]

@fnc12 still report same error after remove it from capture but add constexpr to tupleSize

[fnc12]

it is weird cause all checks are passed on appveyor which runs on windows with different vc++ compilers. What is you visual studio version and c++ standard?

[FunDoor]

vs 2017,I have upgrade to newest version 15.9.26,
whatever choose c++14 c++17 and c++ lastest standard, it report the same error
problaly it is a compiler bug.

[FunDoor]

I have check out the dev branch, and notice the delete_with_two_fields.cpp didn't add into cmakelists.txt.

[FunDoor]

another problem.
I change test case for delete_with_two_fields.cpp,
it test failed.

#include <sqlite_orm/sqlite_orm.h>
#include <catch2/catch.hpp>

using namespace sqlite_orm;

TEST_CASE("delete with two fields") {
    struct Device {
        std::string serialNumber;
        std::string deviceId;
    };
    auto storage = make_storage({},
                                make_table("devices",
                                           make_column("serial_number", &Device::serialNumber),
                                           make_column("device_id", &Device::deviceId)));
    storage.sync_schema();

    Device device_abc{ "abc", "123" };
    Device device_def{ "def", "456" };

    storage.replace(device_abc);
    storage.replace(device_def);

    storage.remove_all<Device>(where(in(std::make_tuple(&Device::serialNumber, &Device::deviceId),
                                        values(std::make_tuple("abc", "123"), std::make_tuple("def", "456")))));

    storage.remove_all<Device>(
        where(in(std::make_tuple(&Device::serialNumber, &Device::deviceId),
                 values(std::vector<std::tuple<std::string, std::string>>{std::make_tuple("abc", "123"),
                                                                          std::make_tuple("def", "456")}))));
    int count_devices = storage.count<Device>();
    REQUIRE(count_devices == 0);
}

[fnc12]

I have check out the dev branch, and notice the delete_with_two_fields.cpp didn't add into cmakelists.txt.

My bad. I shall fix it.

I change test case for delete_with_two_fields.cpp,

what exactly did you change?

[fnc12]

I have check out the dev branch, and notice the delete_with_two_fields.cpp didn't add into cmakelists.txt.

https://github.com/fnc12/sqlite_orm/pull/580

[FunDoor]

insert 2 devices into table
use remove_all with a set of composite key to delete them
count the devices
test count == 0 failed.

[fnc12]

you know adding delete_with_two_fields.cpp to cmake fails on windows only https://ci.appveyor.com/project/fnc12/sqlite-orm/builds/34682525 . Looks like it needs to be fixed.

[fnc12]

I changed value to type so it must work on windows and linux/mac. https://github.com/fnc12/sqlite_orm/pull/580 Next I shall fix unit test

[fnc12]

fixed in https://github.com/fnc12/sqlite_orm/pull/580.

[fnc12]

unit tests fix https://github.com/fnc12/sqlite_orm/pull/581

[fnc12]

@FunDoor fixed. Please check it out in dev branch

[FunDoor]

it works, thanks a lot.