protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.11k stars 15.43k forks source link

proto2 bind to db is very useful but when I update to proto3 its not work well anymore #2480

Closed DexterPhone closed 7 years ago

DexterPhone commented 7 years ago

When I use proto2, I can bind protocol message with database tables, but when I using proto3 instead old code , it does not work correct.

message VipUser { optional int32 remain_amount = 1; optional string name = 2; optional int32 _id = 3; // primary key }

I can use this message to modify database: VipUser message; message.set_remain_amount(11); message.set_name("jack"); message.set__id(112); database::modify(message);

In modify function I can spell sql statment like this: std::string sqlstmt("update table set "); auto descriptor = message.GetDescriptor(); auto reflection = message.GetReflection(); for (auto i = 0; i < descriptor->field_count(); ++i) { auto field = descriptor->field(i); if(field == nullptr) continue; if(reflection->HasField(message, field)) {//not true code but almost like this sqlstmt.append(field->name()).append("=").append(get_filed_value(message, field)).append(" and "); } }

when proto3 when this happend message.set_remain_amount(0); 0 is default value,so when HasField called it returns false; then it can't change the column to 0,why proto3 change hasbit function, I don't think its a perfect change

xfxyjwf commented 7 years ago

Why not just remove the HasField() check?

DexterPhone commented 7 years ago

I use the is set function to find the bind item which column is want to make changes in the database. if its not working,what shall i do,change all data in database? class bind db table is very common needs in daily work,but the new proto3 version not support any more

xfxyjwf commented 7 years ago

The way to do this in proto3 is to use a FieldMask. See: https://github.com/google/protobuf/blob/master/src/google/protobuf/field_mask.proto

DexterPhone commented 7 years ago

thank you.I will try

DexterPhone commented 7 years ago

@xfxyjwf I'm not an expert in protobuf. I tried field_mask,but I don't think it's a perfect way to solve this problem. I want to change database table data, not another proto message Here is my code to bind database tables with proto message:

template <typename _Message>
class BaseTable
{
public:
    BaseTable();
    virtual ~BaseTable();

public:
    static int insert(_Message& message);
    static int update(const _Message& message, const string& condition);
    static int del(_Message* msg);
    static int del(const string& condition);
    static int query(const string& condition, vector<_Message>& items);
    static int query_one(_Message& item, string& condition="");

protected:
    static std::string m_table_name;
    static std::string m_table_create_sql;
    static vector<string> m_primary_key;
};

std::string make_value(const google::protobuf::Message& message, bool is_set_data)
{
    std::string set_value;
    bool is_first = true;
    if (!is_set_data)
    {
        set_value = " where ";
    }

    auto descriptor = message.GetDescriptor();
    auto reflection = message.GetReflection();
    for (auto i = 0; i < descriptor->field_count(); ++i)
    {
        auto field = descriptor->field(i);
        if(field == nullptr)
        {
            continue;
        }
        if(!reflection->HasField(message, field))
        {
            set_value.append(field->name()).append("=")
            .append(get_filed_value_str(field));
        }
    }

    return set_value;
}

template <typename _Message>
int BaseTable<_Message>::update(const _Message& message, string condition = "")
{
    int error_code = SQERROR_DB_TABLE_NOT_FOUND;
    auto& table_name = theTableName::instance().get_table_name(
    _Message::default_instance().GetTypeName());

    if (!table_name.empty())
    {
        std::string data_set = make_value(message, true);
        if(!condition.empty())
        {
            std::string sql_query = "update " + table_name + 
            " set " + data_set + " " + condition + ";";
            error_code = SQDBManager::instance().sql_execute(sql_query);
        }
        else
        {
            error_code = SQERROR_DB_BAD_CONDITION;
        }
    }
    return error_code;
}

We can define a simple table, and find how to use it:

message VipUser
{
    int32 _id           = 1;          // primary key
    int32 remain_amount = 2;
    string name         = 3;
}

typedef BaseTable<VipUser> Table_VipUser;
VipUser jack;
jack.set_remain_amount(100);
jack.set_name("jack");
Table_VipUser::insert(jack);
jack.set_remain_amount(0);
Table_VipUser::update(jack);// here has some code to make primary key condition

With proto2 it's working very happy, I can add new tables very fast, and less code, no bug happens. But if i change the code like this with proto3

message VipUser
{
    int32 _id           = 1;          // primary key
    int32 remain_amount = 2;
    string name         = 3;
    google.protobuf.FieldMask field_mask = 4; 
}

typedef BaseTable<VipUser> Table_VipUser;
VipUser jack;
jack.set_remain_amount(100);
jack.set_name("jack");
Table_VipUser::insert(jack);
jack.set_remain_amount(0);
jack.mutable_field_mask()->add_paths("remain_amount");
Table_VipUser::update(jack);// here has some code to make primary key condition

I also need to change the BaseTable::update sql stmt spell function, but its not the point, the point is this mask add may easy to be forgetten, because its a interface, you could not ask everyone know this, it will lead to grave consequences and hardly to find out. Now with proto2 it's working well, but I'm afraid that must be upgraded in the future.

DexterPhone commented 7 years ago

I set the field already but I also need to mask it, don't paint a snake with legs. T_T

xfxyjwf commented 7 years ago

The changed semantics in proto3 is required to make the protobuf implementation in some languages more performant and it's believed the majority of protobuf users don't actually care about has-states, so we are unlikely to change the proto3 semantic. There are other workarounds in proto3 to do what you want but none of them is as simple as proto2. If proto2 works for you better, you can stick with proto2. There is no plan to drop proto2 support and I don't think this will change in the next couple of years.