From dc46b8033f1d515713fc8ea2d0da5670fb31e2eb Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Tue, 3 Sep 2024 11:53:32 -0700 Subject: [PATCH 01/11] add error checking for operate(); wip --- src/main/client/operate.c | 27 +++++++++++++++++++-------- test/new_tests/test_operate.py | 4 ++++ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index dbc2513e8..73fe4e919 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -836,7 +836,6 @@ static PyObject *AerospikeClient_Operate_Invoke(AerospikeClient *self, PyObject *py_meta, PyObject *py_policy) { - int i = 0; long operation; long return_type = -1; bool operation_succeeded = false; @@ -853,6 +852,10 @@ static PyObject *AerospikeClient_Operate_Invoke(AerospikeClient *self, as_operations ops; Py_ssize_t size = PyList_Size(py_list); + if (PyErr_Occurred()) { + return NULL; + } + as_operations_inita(&ops, size); if (py_policy) { @@ -872,14 +875,19 @@ static PyObject *AerospikeClient_Operate_Invoke(AerospikeClient *self, goto CLEANUP; } - for (i = 0; i < size; i++) { + for (Py_ssize_t i = 0; i < size; i++) { PyObject *py_val = PyList_GetItem(py_list, i); + if (py_val == NULL) { + return NULL; + } - if (PyDict_Check(py_val)) { - if (add_op(self, err, py_val, unicodeStrVector, &static_pool, &ops, - &operation, &return_type) != AEROSPIKE_OK) { - goto CLEANUP; - } + if (!PyDict_Check(py_val)) { + return NULL; + } + + if (add_op(self, err, py_val, unicodeStrVector, &static_pool, &ops, + &operation, &return_type) != AEROSPIKE_OK) { + goto CLEANUP; } } if (err->code != AEROSPIKE_OK) { @@ -898,7 +906,10 @@ static PyObject *AerospikeClient_Operate_Invoke(AerospikeClient *self, operation_succeeded = true; if (rec) { - record_to_pyobject(self, err, rec, key, &py_rec); + as_status retval = record_to_pyobject(self, err, rec, key, &py_rec); + if (!retval) { + goto CLEANUP; + } } CLEANUP: diff --git a/test/new_tests/test_operate.py b/test/new_tests/test_operate.py index 02c04b621..a37640554 100644 --- a/test/new_tests/test_operate.py +++ b/test/new_tests/test_operate.py @@ -851,6 +851,10 @@ def test_pos_operate_with_list_remove_operations(self, list, bin, expected): assert bins[bin] == expected + def test_operate_empty_list(self): + key = ("test", "demo", "list_key") + self.as_connection.operate(key, []) + def test_pos_operate_with_list_size(self): """ Invoke operate() with list_size operation From 43bcd50200e1e68425fc75a460a3b144996d4fd0 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 5 Sep 2024 09:54:06 -0700 Subject: [PATCH 02/11] wip --- src/main/client/operate.c | 118 ++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 69 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index 73fe4e919..8ac72f9dd 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -42,10 +42,9 @@ #include #include -static as_status get_operation(as_error *err, PyObject *op_dict, - long *operation_ptr); +static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, + long *operation_ptr); -static inline bool isListOp(int op); static inline bool isNewMapOp(int op); static inline bool isBitOp(int op); static inline bool isHllOp(int op); @@ -210,27 +209,6 @@ int check_type(AerospikeClient *self, PyObject *py_value, int op, as_error *err) return 0; } -static inline bool isListOp(int op) -{ - return ( - op == OP_LIST_APPEND || op == OP_LIST_APPEND_ITEMS || - op == OP_LIST_INSERT || op == OP_LIST_INSERT_ITEMS || - op == OP_LIST_POP || op == OP_LIST_POP_RANGE || op == OP_LIST_REMOVE || - op == OP_LIST_REMOVE_RANGE || op == OP_LIST_CLEAR || - op == OP_LIST_SET || op == OP_LIST_GET || op == OP_LIST_GET_RANGE || - op == OP_LIST_TRIM || op == OP_LIST_SIZE || op == OP_LIST_INCREMENT || - op == OP_LIST_GET_BY_INDEX || op == OP_LIST_GET_BY_INDEX_RANGE || - op == OP_LIST_GET_BY_RANK || op == OP_LIST_GET_BY_RANK_RANGE || - op == OP_LIST_GET_BY_VALUE || op == OP_LIST_GET_BY_VALUE_LIST || - op == OP_LIST_GET_BY_VALUE_RANGE || op == OP_LIST_REMOVE_BY_INDEX || - op == OP_LIST_REMOVE_BY_INDEX_RANGE || op == OP_LIST_REMOVE_BY_RANK || - op == OP_LIST_REMOVE_BY_RANK_RANGE || op == OP_LIST_REMOVE_BY_VALUE || - op == OP_LIST_REMOVE_BY_VALUE_LIST || - op == OP_LIST_REMOVE_BY_VALUE_RANGE || op == OP_LIST_SET_ORDER || - op == OP_LIST_SORT || op == OP_LIST_REMOVE_BY_VALUE_RANK_RANGE_REL || - op == OP_LIST_GET_BY_VALUE_RANK_RANGE_REL || op == OP_LIST_CREATE); -} - static inline bool isNewMapOp(int op) { return (op == OP_MAP_REMOVE_BY_KEY_INDEX_RANGE_REL || @@ -316,7 +294,7 @@ bool opRequiresKey(int op) op == OP_MAP_GET_BY_KEY_RANGE); } -as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, +as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_vector *unicodeStrVector, as_static_pool *static_pool, as_operations *ops, long *op, long *ret_type) { @@ -332,7 +310,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, long ttl = 0; double double_offset = 0.0; int index = 0; - long operation = 0; + long op_code = 0; uint64_t return_type = AS_MAP_RETURN_NONE; PyObject *py_ustr = NULL; PyObject *py_ustr1 = NULL; @@ -341,7 +319,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, as_map_policy map_policy; as_map_policy_init(&map_policy); - PyObject *key_op = NULL, *value = NULL; + PyObject *py_dict_key = NULL, *value = NULL; PyObject *py_value = NULL; PyObject *py_key = NULL; PyObject *py_index = NULL; @@ -354,45 +332,49 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, Py_ssize_t pos = 0; - if (get_operation(err, py_val, &operation) != AEROSPIKE_OK) { + if (get_op_code_from_py_op_dict(err, py_op_dict, &op_code) != + AEROSPIKE_OK) { return err->code; } /* Handle the list operations with a helper in the cdt_list_operate.c file */ - if (isListOp(operation)) { + if (op_code >= OP_LIST_APPEND && op_code <= OP_LIST_CREATE) { return add_new_list_op( - self, err, py_val, unicodeStrVector, static_pool, ops, operation, + self, err, py_op_dict, unicodeStrVector, static_pool, ops, op_code, ret_type, SERIALIZER_PYTHON); //This hardcoding matches current behavior } - if (isNewMapOp(operation)) { - return add_new_map_op(self, err, py_val, unicodeStrVector, static_pool, - ops, operation, ret_type, SERIALIZER_PYTHON); + if (op_code >= OP_MAP_SET_POLICY && op <= OP_MAP_CREATE) { + return add_new_map_op(self, err, py_op_dict, unicodeStrVector, + static_pool, ops, op_code, ret_type, + SERIALIZER_PYTHON); } - if (isBitOp(operation)) { - return add_new_bit_op(self, err, py_val, unicodeStrVector, static_pool, - ops, operation, ret_type, SERIALIZER_PYTHON); + if (isBitOp(op_code)) { + return add_new_bit_op(self, err, py_op_dict, unicodeStrVector, + static_pool, ops, op_code, ret_type, + SERIALIZER_PYTHON); } - if (isHllOp(operation)) { - return add_new_hll_op(self, err, py_val, unicodeStrVector, static_pool, - ops, operation, ret_type, SERIALIZER_PYTHON); + if (isHllOp(op_code)) { + return add_new_hll_op(self, err, py_op_dict, unicodeStrVector, + static_pool, ops, op_code, ret_type, + SERIALIZER_PYTHON); } - if (isExprOp(operation)) { - return add_new_expr_op(self, err, py_val, unicodeStrVector, ops, - operation, SERIALIZER_PYTHON); + if (isExprOp(op_code)) { + return add_new_expr_op(self, err, py_op_dict, unicodeStrVector, ops, + op_code, SERIALIZER_PYTHON); } - while (PyDict_Next(py_val, &pos, &key_op, &value)) { - if (!PyUnicode_Check(key_op)) { + while (PyDict_Next(py_op_dict, &pos, &py_dict_key, &value)) { + if (!PyUnicode_Check(py_dict_key)) { return as_error_update(err, AEROSPIKE_ERR_CLIENT, "An operation key must be a string."); } else { - char *name = (char *)PyUnicode_AsUTF8(key_op); + char *name = (char *)PyUnicode_AsUTF8(py_dict_key); if (!strcmp(name, "op")) { continue; } @@ -440,7 +422,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, } } - *op = operation; + *op = op_code; if (py_bin) { if (PyUnicode_Check(py_bin)) { @@ -470,25 +452,24 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, } } } - else if (operation != AS_OPERATOR_TOUCH && - operation != AS_OPERATOR_DELETE) { + else if (op_code != AS_OPERATOR_TOUCH && op_code != AS_OPERATOR_DELETE) { as_error_update(err, AEROSPIKE_ERR_PARAM, "Bin is not given"); goto CLEANUP; } if (py_value) { if (self->strict_types) { - if (check_type(self, py_value, operation, err)) { + if (check_type(self, py_value, op_code, err)) { goto CLEANUP; } } } - else if (opRequiresValue(operation)) { + else if (opRequiresValue(op_code)) { return as_error_update(err, AEROSPIKE_ERR_PARAM, "Value should be given"); } - if (!py_key && opRequiresKey(operation)) { + if (!py_key && opRequiresKey(op_code)) { return as_error_update(err, AEROSPIKE_ERR_PARAM, "Operation requires key parameter"); } @@ -499,12 +480,12 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, goto CLEANUP; } } - else if (opRequiresMapPolicy(operation)) { + else if (opRequiresMapPolicy(op_code)) { return as_error_update(err, AEROSPIKE_ERR_PARAM, "Operation requires map_policy parameter"); } - if (!py_range && opRequiresRange(operation)) { + if (!py_range && opRequiresRange(op_code)) { return as_error_update(err, AEROSPIKE_ERR_PARAM, "Range should be given"); } @@ -518,7 +499,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, } /* Add the inverted flag to the return type if it's present */ - if (invertIfSpecified(err, py_val, &return_type) != AEROSPIKE_OK) { + if (invertIfSpecified(err, py_op_dict, &return_type) != AEROSPIKE_OK) { goto CLEANUP; } @@ -529,7 +510,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, *ret_type = return_type; if (py_index) { - if (self->strict_types && !opRequiresIndex(operation)) { + if (self->strict_types && !opRequiresIndex(op_code)) { return as_error_update(err, AEROSPIKE_ERR_PARAM, "Operation does not need an index value"); } @@ -541,12 +522,12 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_val, "Index should be an integer"); } } - else if (opRequiresIndex(operation)) { + else if (opRequiresIndex(op_code)) { return as_error_update(err, AEROSPIKE_ERR_PARAM, "Operation needs an index value"); } - switch (operation) { + switch (op_code) { case AS_OPERATOR_APPEND: if (PyUnicode_Check(py_value)) { py_ustr1 = PyUnicode_AsUTF8String(py_value); @@ -876,24 +857,23 @@ static PyObject *AerospikeClient_Operate_Invoke(AerospikeClient *self, } for (Py_ssize_t i = 0; i < size; i++) { - PyObject *py_val = PyList_GetItem(py_list, i); - if (py_val == NULL) { - return NULL; + PyObject *py_op_dict = PyList_GetItem(py_list, i); + if (py_op_dict == NULL) { + goto CLEANUP; } - if (!PyDict_Check(py_val)) { - return NULL; + if (!PyDict_Check(py_op_dict)) { + as_error_update( + err, AEROSPIKE_ERR_PARAM, + "Operation at list index %d is not a Python dictionary", i); + goto CLEANUP; } - if (add_op(self, err, py_val, unicodeStrVector, &static_pool, &ops, + if (add_op(self, err, py_op_dict, unicodeStrVector, &static_pool, &ops, &operation, &return_type) != AEROSPIKE_OK) { goto CLEANUP; } } - if (err->code != AEROSPIKE_OK) { - as_error_update(err, err->code, NULL); - goto CLEANUP; - } Py_BEGIN_ALLOW_THREADS aerospike_key_operate(self->as, err, operate_policy_p, key, &ops, &rec); @@ -1420,8 +1400,8 @@ PyObject *AerospikeClient_Touch(AerospikeClient *self, PyObject *args, return PyLong_FromLong(0); } -static as_status get_operation(as_error *err, PyObject *op_dict, - long *operation_ptr) +static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, + long *operation_ptr) { PyObject *py_operation = PyDict_GetItemString(op_dict, PY_OPERATION_KEY); if (!py_operation) { From 078f626679cbc78682b247ec96eee9cfa6ea8eb0 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 5 Sep 2024 10:24:17 -0700 Subject: [PATCH 03/11] wip --- src/main/client/operate.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index 8ac72f9dd..6ba4ab12f 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -294,9 +294,22 @@ bool opRequiresKey(int op) op == OP_MAP_GET_BY_KEY_RANGE); } -as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, - as_vector *unicodeStrVector, as_static_pool *static_pool, - as_operations *ops, long *op, long *ret_type) +struct aerospike_helper_codes_to_add_op_methods { + // Internal code only used by aerospike_helpers + // Represents the operation to add in the C client + unsigned int code; + void *add_op_method; +}; + +struct aerospike_helper_codes_to_add_op_methods map[] = + { + {OP_LIST_APPEND, as_operations_list_append}, +} + +as_status +add_op(AerospikeClient * self, as_error *err, PyObject *py_op_dict, + as_vector *unicodeStrVector, as_static_pool *static_pool, + as_operations *ops, long *op, long *ret_type) { as_val *put_val = NULL; as_val *put_key = NULL; @@ -337,6 +350,11 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, return err->code; } + // TODO: get bin if it exists. Don't fail out if it doesn't + if (get_bin(err, py_op_dict, unicodeStrVector, &bin) != AEROSPIKE_OK) { + return err->code; + } + /* Handle the list operations with a helper in the cdt_list_operate.c file */ if (op_code >= OP_LIST_APPEND && op_code <= OP_LIST_CREATE) { return add_new_list_op( @@ -1401,7 +1419,7 @@ PyObject *AerospikeClient_Touch(AerospikeClient *self, PyObject *args, } static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, - long *operation_ptr) + long *op_code_ref) { PyObject *py_operation = PyDict_GetItemString(op_dict, PY_OPERATION_KEY); if (!py_operation) { @@ -1413,10 +1431,9 @@ static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, "Operation must be an integer"); } - *operation_ptr = PyLong_AsLong(py_operation); + long op_code = PyLong_AsLong(py_operation); if (PyErr_Occurred()) { - if (*operation_ptr == -1 && - PyErr_ExceptionMatches(PyExc_OverflowError)) { + if (op_code == -1 && PyErr_ExceptionMatches(PyExc_OverflowError)) { return as_error_update(err, AEROSPIKE_ERR_PARAM, "Operation code too large"); } @@ -1425,6 +1442,8 @@ static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, "Invalid operation"); } } + *op_code_ref = op_code; + return AEROSPIKE_OK; } From fed8b56f0007d7e66a3d8759cd120c7cd7be1a72 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 5 Sep 2024 11:54:57 -0700 Subject: [PATCH 04/11] wip --- src/main/client/operate.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index 6ba4ab12f..3ab6b2d1b 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -294,22 +294,21 @@ bool opRequiresKey(int op) op == OP_MAP_GET_BY_KEY_RANGE); } -struct aerospike_helper_codes_to_add_op_methods { - // Internal code only used by aerospike_helpers - // Represents the operation to add in the C client - unsigned int code; - void *add_op_method; -}; - -struct aerospike_helper_codes_to_add_op_methods map[] = - { - {OP_LIST_APPEND, as_operations_list_append}, -} - -as_status -add_op(AerospikeClient * self, as_error *err, PyObject *py_op_dict, - as_vector *unicodeStrVector, as_static_pool *static_pool, - as_operations *ops, long *op, long *ret_type) +// struct aerospike_helper_codes_to_add_op_methods { +// // Internal code only used by aerospike_helpers +// // Represents the operation to add in the C client +// unsigned int code; +// void *add_op_method; +// }; + +// struct aerospike_helper_codes_to_add_op_methods map[] = +// { +// {OP_LIST_APPEND, as_operations_list_append}, +// }; + +as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, + as_vector *unicodeStrVector, as_static_pool *static_pool, + as_operations *ops, long *op, long *ret_type) { as_val *put_val = NULL; as_val *put_key = NULL; @@ -355,6 +354,13 @@ add_op(AerospikeClient * self, as_error *err, PyObject *py_op_dict, return err->code; } + // TODO: no way to define an array of function pointers with differing arguments + switch (op_code) { + case OP_LIST_APPEND: + // as_operations_list_append(); + break; + } + /* Handle the list operations with a helper in the cdt_list_operate.c file */ if (op_code >= OP_LIST_APPEND && op_code <= OP_LIST_CREATE) { return add_new_list_op( From 073bf7c6b34c87d4baa00fdb3177147130282494 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:42:44 -0700 Subject: [PATCH 05/11] move map policy to where it's first used --- src/main/client/operate.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index 3ab6b2d1b..e658e7328 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -328,9 +328,6 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, PyObject *py_ustr1 = NULL; PyObject *py_bin = NULL; - as_map_policy map_policy; - as_map_policy_init(&map_policy); - PyObject *py_dict_key = NULL, *value = NULL; PyObject *py_value = NULL; PyObject *py_key = NULL; @@ -498,6 +495,8 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, "Operation requires key parameter"); } + as_map_policy map_policy; + as_map_policy_init(&map_policy); if (py_map_policy) { if (pyobject_to_map_policy(err, py_map_policy, &map_policy) != AEROSPIKE_OK) { From 3ebceb5d0beadaed624c3fb0501a5922e52f463e Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:56:58 -0700 Subject: [PATCH 06/11] WIP --- src/main/client/cdt_list_operate.c | 1 + src/main/client/operate.c | 52 ++++++++++++++++-------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/main/client/cdt_list_operate.c b/src/main/client/cdt_list_operate.c index 723927d2f..03935cfe5 100644 --- a/src/main/client/cdt_list_operate.c +++ b/src/main/client/cdt_list_operate.c @@ -1198,6 +1198,7 @@ static as_status add_op_list_create(AerospikeClient *self, as_error *err, } } +// ops must point to a stack-allocated as_operations variable static as_status add_op_list_append(AerospikeClient *self, as_error *err, char *bin, PyObject *op_dict, as_operations *ops, diff --git a/src/main/client/operate.c b/src/main/client/operate.c index e658e7328..d97648e05 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -42,8 +42,8 @@ #include #include -static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, - long *operation_ptr); +static void get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, + long *op_code_ref); static inline bool isNewMapOp(int op); static inline bool isBitOp(int op); @@ -306,9 +306,10 @@ bool opRequiresKey(int op) // {OP_LIST_APPEND, as_operations_list_append}, // }; -as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, - as_vector *unicodeStrVector, as_static_pool *static_pool, - as_operations *ops, long *op, long *ret_type) +// +void add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, + as_vector *unicodeStrVector, as_static_pool *static_pool, + as_operations *ops, long *op, long *ret_type) { as_val *put_val = NULL; as_val *put_key = NULL; @@ -341,8 +342,8 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, Py_ssize_t pos = 0; - if (get_op_code_from_py_op_dict(err, py_op_dict, &op_code) != - AEROSPIKE_OK) { + get_op_code_from_py_op_dict(err, py_op_dict, &op_code); + if (PyErr_Occurred() || err->code == AEROSPIKE_OK) { return err->code; } @@ -351,10 +352,11 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, return err->code; } - // TODO: no way to define an array of function pointers with differing arguments + // No way to define an array of function pointers with differing arguments switch (op_code) { case OP_LIST_APPEND: - // as_operations_list_append(); + as_operations_list_append(ops, bin, (ctx_in_use ? &ctx : NULL), + (policy_in_use ? &list_policy : NULL), val); break; } @@ -1423,33 +1425,33 @@ PyObject *AerospikeClient_Touch(AerospikeClient *self, PyObject *args, return PyLong_FromLong(0); } -static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, - long *op_code_ref) +// error indicator must be checked after this call +static void get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, + long *op_code_ref) { PyObject *py_operation = PyDict_GetItemString(op_dict, PY_OPERATION_KEY); if (!py_operation) { - return as_error_update(err, AEROSPIKE_ERR_PARAM, - "Operation must contain an \"op\" entry"); + as_error_update(err, AEROSPIKE_ERR_PARAM, + "Operation must contain an \"op\" entry"); + goto error; } if (!PyLong_Check(py_operation)) { - return as_error_update(err, AEROSPIKE_ERR_PARAM, - "Operation must be an integer"); + as_error_update(err, AEROSPIKE_ERR_PARAM, + "Operation must be an integer"); + goto error; } long op_code = PyLong_AsLong(py_operation); - if (PyErr_Occurred()) { - if (op_code == -1 && PyErr_ExceptionMatches(PyExc_OverflowError)) { - return as_error_update(err, AEROSPIKE_ERR_PARAM, - "Operation code too large"); - } - else { - return as_error_update(err, AEROSPIKE_ERR_PARAM, - "Invalid operation"); - } + if (op_code == -1 && PyErr_Occurred()) { + PyErr_Clear(); + as_error_update(err, AEROSPIKE_ERR_PARAM, + "Operation must be an integer"); + goto error; } *op_code_ref = op_code; - return AEROSPIKE_OK; +error: + return; } static as_status invertIfSpecified(as_error *err, PyObject *op_dict, From f30649198b0bca4ded6d22780eb2b129d70d0b0a Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Wed, 9 Oct 2024 14:18:28 -0700 Subject: [PATCH 07/11] failing --- src/main/client/operate.c | 119 +++++++++++++------------------------- 1 file changed, 40 insertions(+), 79 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index d97648e05..a3dcf755a 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -306,10 +306,9 @@ bool opRequiresKey(int op) // {OP_LIST_APPEND, as_operations_list_append}, // }; -// -void add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, - as_vector *unicodeStrVector, as_static_pool *static_pool, - as_operations *ops, long *op, long *ret_type) +as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, + as_vector *unicodeStrVector, as_static_pool *static_pool, + as_operations *ops, long *op, long *ret_type) { as_val *put_val = NULL; as_val *put_key = NULL; @@ -329,7 +328,7 @@ void add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, PyObject *py_ustr1 = NULL; PyObject *py_bin = NULL; - PyObject *py_dict_key = NULL, *value = NULL; + PyObject *py_dict_key = NULL, *py_dict_value = NULL; PyObject *py_value = NULL; PyObject *py_key = NULL; PyObject *py_index = NULL; @@ -342,14 +341,28 @@ void add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, Py_ssize_t pos = 0; - get_op_code_from_py_op_dict(err, py_op_dict, &op_code); - if (PyErr_Occurred() || err->code == AEROSPIKE_OK) { + // Get required op dictionary entries + if (get_op_code_from_py_op_dict(err, py_op_dict, &op_code)) { return err->code; } - // TODO: get bin if it exists. Don't fail out if it doesn't - if (get_bin(err, py_op_dict, unicodeStrVector, &bin) != AEROSPIKE_OK) { - return err->code; + // Validate op dictionary using hashset? + while (PyDict_Next(py_op_dict, &pos, &py_dict_key, &py_dict_value)) { + if (!PyUnicode_Check(py_dict_key)) { + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "An operation key must be a string."); + } + else if (strcmp(dict_key, "ctx") == 0) { + CONVERT_PY_CTX_TO_AS_CTX(); + ctx_ref = (ctx_in_use ? &ctx : NULL); + } + else { + as_error_update( + err, AEROSPIKE_ERR_PARAM, + "Operation can contain only op, bin, index, key, val, " + "return_type and map_policy keys"); + goto CLEANUP; + } } // No way to define an array of function pointers with differing arguments @@ -391,60 +404,6 @@ void add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, op_code, SERIALIZER_PYTHON); } - while (PyDict_Next(py_op_dict, &pos, &py_dict_key, &value)) { - if (!PyUnicode_Check(py_dict_key)) { - return as_error_update(err, AEROSPIKE_ERR_CLIENT, - "An operation key must be a string."); - } - else { - char *name = (char *)PyUnicode_AsUTF8(py_dict_key); - if (!strcmp(name, "op")) { - continue; - } - else if (!strcmp(name, "bin")) { - py_bin = value; - } - else if (!strcmp(name, "index")) { - py_index = value; - } - else if (!strcmp(name, "val")) { - py_value = value; - } - else if (!strcmp(name, "key")) { - py_key = value; - } - else if (!strcmp(name, "range")) { - py_range = value; - } - else if (!strcmp(name, "map_policy")) { - py_map_policy = value; - } - else if (!strcmp(name, "return_type")) { - py_return_type = value; - } - else if (strcmp(name, "inverted") == 0) { - continue; - } - else if (strcmp(name, "ctx") == 0) { - CONVERT_PY_CTX_TO_AS_CTX(); - ctx_ref = (ctx_in_use ? &ctx : NULL); - } - else if (strcmp(name, "map_order") == 0) { - py_map_order = value; - } - else if (strcmp(name, "persist_index") == 0) { - py_persist_index = value; - } - else { - as_error_update( - err, AEROSPIKE_ERR_PARAM, - "Operation can contain only op, bin, index, key, val, " - "return_type and map_policy keys"); - goto CLEANUP; - } - } - } - *op = op_code; if (py_bin) { @@ -1425,33 +1384,35 @@ PyObject *AerospikeClient_Touch(AerospikeClient *self, PyObject *args, return PyLong_FromLong(0); } -// error indicator must be checked after this call -static void get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, - long *op_code_ref) +static as_status get_op_code_from_py_op_dict(as_error *err, PyObject *op_dict, + long *op_code_ref) { PyObject *py_operation = PyDict_GetItemString(op_dict, PY_OPERATION_KEY); if (!py_operation) { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "Operation must contain an \"op\" entry"); - goto error; + return as_error_update(err, AEROSPIKE_ERR_PARAM, + "Operation must contain an \"op\" entry"); } if (!PyLong_Check(py_operation)) { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "Operation must be an integer"); - goto error; + return as_error_update(err, AEROSPIKE_ERR_PARAM, + "Operation must be an integer"); } long op_code = PyLong_AsLong(py_operation); if (op_code == -1 && PyErr_Occurred()) { + if (PyErr_ExceptionMatches(PyExc_OverflowError)) { + as_error_update(err, AEROSPIKE_ERR_PARAM, + "Operation code too large"); + } + else { + as_error_update(err, AEROSPIKE_ERR_PARAM, + "Unable to convert Python operation to C long"); + } PyErr_Clear(); - as_error_update(err, AEROSPIKE_ERR_PARAM, - "Operation must be an integer"); - goto error; + return err->code; } - *op_code_ref = op_code; -error: - return; + *op_code_ref = op_code; + return AEROSPIKE_OK; } static as_status invertIfSpecified(as_error *err, PyObject *op_dict, From 3dbea4dd632d85f8c919634239514f10cf2def26 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Wed, 9 Oct 2024 14:21:09 -0700 Subject: [PATCH 08/11] uhh --- src/main/client/operate.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index a3dcf755a..bc1dc5d85 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -306,6 +306,7 @@ bool opRequiresKey(int op) // {OP_LIST_APPEND, as_operations_list_append}, // }; +// as_ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_vector *unicodeStrVector, as_static_pool *static_pool, as_operations *ops, long *op, long *ret_type) @@ -341,6 +342,12 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, Py_ssize_t pos = 0; + if (!PyDict_Check(py_op_dict)) { + as_error_update(err, AEROSPIKE_ERR_PARAM, + "Op is not a Python dictionary"); + return err->code; + } + // Get required op dictionary entries if (get_op_code_from_py_op_dict(err, py_op_dict, &op_code)) { return err->code; @@ -846,13 +853,6 @@ static PyObject *AerospikeClient_Operate_Invoke(AerospikeClient *self, goto CLEANUP; } - if (!PyDict_Check(py_op_dict)) { - as_error_update( - err, AEROSPIKE_ERR_PARAM, - "Operation at list index %d is not a Python dictionary", i); - goto CLEANUP; - } - if (add_op(self, err, py_op_dict, unicodeStrVector, &static_pool, &ops, &operation, &return_type) != AEROSPIKE_OK) { goto CLEANUP; From deeb9335a1ebcb02e5bb43f16cacf1b2f3de0aba Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Wed, 9 Oct 2024 16:15:21 -0700 Subject: [PATCH 09/11] move to top switch --- src/main/client/operate.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index bc1dc5d85..5f131141d 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -378,6 +378,25 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_operations_list_append(ops, bin, (ctx_in_use ? &ctx : NULL), (policy_in_use ? &list_policy : NULL), val); break; + case AS_OPERATOR_TOUCH: + if (py_value) { + if (pyobject_to_index(self, err, py_value, &ttl) != AEROSPIKE_OK) { + goto CLEANUP; + } + ops->ttl = ttl; + } + as_operations_add_touch(ops); + break; + case AS_OPERATOR_READ: + as_operations_add_read(ops, bin); + break; + case AS_OPERATOR_DELETE: + as_operations_add_delete(ops); + break; + case AS_OPERATOR_WRITE: + CONVERT_VAL_TO_AS_VAL(); + as_operations_add_write(ops, bin, (as_bin_value *)put_val); + break; } /* Handle the list operations with a helper in the cdt_list_operate.c file */ @@ -612,26 +631,6 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, } } break; - case AS_OPERATOR_TOUCH: - if (py_value) { - if (pyobject_to_index(self, err, py_value, &ttl) != AEROSPIKE_OK) { - goto CLEANUP; - } - ops->ttl = ttl; - } - as_operations_add_touch(ops); - break; - case AS_OPERATOR_READ: - as_operations_add_read(ops, bin); - break; - case AS_OPERATOR_DELETE: - as_operations_add_delete(ops); - break; - case AS_OPERATOR_WRITE: - CONVERT_VAL_TO_AS_VAL(); - as_operations_add_write(ops, bin, (as_bin_value *)put_val); - break; - //------- MAP OPERATIONS --------- case OP_MAP_SET_POLICY: as_operations_map_set_policy(ops, bin, ctx_ref, &map_policy); From 9854cb19c05fc221906348a4940acbe315b567cf Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 10 Oct 2024 08:01:36 -0700 Subject: [PATCH 10/11] WIP --- src/main/client/operate.c | 215 +++++++++++++++++++++----------------- 1 file changed, 121 insertions(+), 94 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index 5f131141d..20874b246 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -81,8 +81,8 @@ static inline bool isExprOp(int op); if (PyObject_HasAttrString(exception_type, "key")) { \ PyObject_SetAttrString(exception_type, "key", py_key); \ } \ - if (PyObject_HasAttrString(exception_type, "bin")) { \ - PyObject_SetAttrString(exception_type, "bin", py_bin); \ + if (PyObject_HasAttrString(exception_type, "bin_name")) { \ + PyObject_SetAttrString(exception_type, "bin_name", py_bin); \ } \ PyErr_SetObject(exception_type, py_err); \ Py_DECREF(py_err); \ @@ -135,7 +135,7 @@ static as_status invertIfSpecified(as_error *err, PyObject *op_dict, * * @param py_list The List. * @param operation The operation to perform. - * @param py_bin The bin name to perform operation. + * @param py_bin The bin_name name to perform operation. * @param py_value The value to perform operation. * @param py_initial_val The initial value for increment operation. * @@ -149,7 +149,7 @@ PyObject *create_pylist(PyObject *py_list, long operation, PyObject *py_bin, py_list = PyList_New(0); PyDict_SetItemString(dict, "op", PyLong_FromLong(operation)); if (operation != AS_OPERATOR_TOUCH) { - PyDict_SetItemString(dict, "bin", py_bin); + PyDict_SetItemString(dict, "bin_name", py_bin); } PyDict_SetItemString(dict, "val", py_value); @@ -294,19 +294,12 @@ bool opRequiresKey(int op) op == OP_MAP_GET_BY_KEY_RANGE); } -// struct aerospike_helper_codes_to_add_op_methods { -// // Internal code only used by aerospike_helpers -// // Represents the operation to add in the C client -// unsigned int code; -// void *add_op_method; -// }; +static bool op_requires_bin_name[] = { + [AS_OPERATOR_READ] = true, [AS_OPERATOR_WRITE] = true, + [AS_OPERATOR_INCR] = true, [AS_OPERATOR_APPEND] = true, + [AS_OPERATOR_PREPEND] = true, +}; -// struct aerospike_helper_codes_to_add_op_methods map[] = -// { -// {OP_LIST_APPEND, as_operations_list_append}, -// }; - -// as_ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_vector *unicodeStrVector, as_static_pool *static_pool, as_operations *ops, long *op, long *ret_type) @@ -317,7 +310,6 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_cdt_ctx ctx; as_cdt_ctx *ctx_ref = NULL; bool ctx_in_use = false; - char *bin = NULL; char *val = NULL; long offset = 0; long ttl = 0; @@ -353,31 +345,58 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, return err->code; } - // Validate op dictionary using hashset? + const char *bin_name = NULL; + if (op_requires_bin_name[op_code] == true) { + PyObject *py_key_for_bin_name = PyUnicode_FromString("bin_name"); + if (py_key_for_bin_name == NULL) { + return NULL; + } + + PyObject *py_bin_name = + PyDict_GetItemWithError(py_op_dict, py_key_for_bin_name); + Py_DECREF(py_key_for_bin_name); + if (py_bin_name == NULL) { + if (!PyErr_Occurred()) { + // Key not found + return as_error_update(err, AEROSPIKE_ERR_PARAM, + "This operation requires a bin_name"); + } + else { + return NULL; + } + } + + // Should be valid as long as dictionary and the bin_name entry exists + bin_name = PyUnicode_AsUTF8(py_bin_name); + if (bin_name == NULL) { + return NULL; + } + } + + // TODO: use python hashset? + const char *valid_keys[] = {"op", "bin_name", "index", + "key", "val", "return_type"}; while (PyDict_Next(py_op_dict, &pos, &py_dict_key, &py_dict_value)) { if (!PyUnicode_Check(py_dict_key)) { return as_error_update(err, AEROSPIKE_ERR_CLIENT, "An operation key must be a string."); } - else if (strcmp(dict_key, "ctx") == 0) { - CONVERT_PY_CTX_TO_AS_CTX(); - ctx_ref = (ctx_in_use ? &ctx : NULL); - } - else { - as_error_update( - err, AEROSPIKE_ERR_PARAM, - "Operation can contain only op, bin, index, key, val, " - "return_type and map_policy keys"); - goto CLEANUP; + + const char *dict_key = PyUnicode_AsUTF8(py_dict_key); + for (unsigned long i = 0; + i < sizeof(valid_keys) / sizeof(valid_keys[0]); i++) { + if (strcmp(dict_key, valid_keys[i])) { + as_error_update( + err, AEROSPIKE_ERR_PARAM, + "Operation can contain only op, bin_name, index, key, val, " + "return_type and map_policy keys"); + goto CLEANUP; + } } } // No way to define an array of function pointers with differing arguments switch (op_code) { - case OP_LIST_APPEND: - as_operations_list_append(ops, bin, (ctx_in_use ? &ctx : NULL), - (policy_in_use ? &list_policy : NULL), val); - break; case AS_OPERATOR_TOUCH: if (py_value) { if (pyobject_to_index(self, err, py_value, &ttl) != AEROSPIKE_OK) { @@ -388,14 +407,14 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_operations_add_touch(ops); break; case AS_OPERATOR_READ: - as_operations_add_read(ops, bin); + as_operations_add_read(ops, bin_name); break; case AS_OPERATOR_DELETE: as_operations_add_delete(ops); break; case AS_OPERATOR_WRITE: CONVERT_VAL_TO_AS_VAL(); - as_operations_add_write(ops, bin, (as_bin_value *)put_val); + as_operations_add_write(ops, bin_name, (as_bin_value *)put_val); break; } @@ -435,15 +454,15 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, if (py_bin) { if (PyUnicode_Check(py_bin)) { py_ustr = PyUnicode_AsUTF8String(py_bin); - bin = strdup(PyBytes_AsString(py_ustr)); - as_vector_append(unicodeStrVector, &bin); + bin_name = strdup(PyBytes_AsString(py_ustr)); + as_vector_append(unicodeStrVector, &bin_name); Py_DECREF(py_ustr); } else if (PyUnicode_Check(py_bin)) { - bin = (char *)PyUnicode_AsUTF8(py_bin); + bin_name = (char *)PyUnicode_AsUTF8(py_bin); } else if (PyByteArray_Check(py_bin)) { - bin = PyByteArray_AsString(py_bin); + bin_name = PyByteArray_AsString(py_bin); } else { as_error_update(err, AEROSPIKE_ERR_PARAM, @@ -452,10 +471,10 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, } if (self->strict_types) { - if (strlen(bin) > AS_BIN_NAME_MAX_LEN) { + if (strlen(bin_name) > AS_BIN_NAME_MAX_LEN) { as_error_update( err, AEROSPIKE_ERR_BIN_NAME, - "A bin name should not exceed 15 characters limit"); + "A bin_name name should not exceed 15 characters limit"); goto CLEANUP; } } @@ -542,7 +561,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, if (PyUnicode_Check(py_value)) { py_ustr1 = PyUnicode_AsUTF8String(py_value); val = strdup(PyBytes_AsString(py_ustr1)); - as_operations_add_append_str(ops, bin, val); + as_operations_add_append_str(ops, bin_name, val); as_vector_append(unicodeStrVector, &val); Py_DECREF(py_ustr1); } @@ -555,7 +574,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, AEROSPIKE_OK) { goto CLEANUP; } - as_operations_add_append_rawp(ops, bin, bytes->value, + as_operations_add_append_rawp(ops, bin_name, bytes->value, bytes->size, true); } } @@ -566,8 +585,8 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_binop *binop = &pointer_ops->binops.entries[pointer_ops->binops.size++]; binop->op = AS_OPERATOR_APPEND; - initialize_bin_for_strictypes(self, err, py_value, binop, bin, - static_pool); + initialize_bin_for_strictypes(self, err, py_value, binop, + bin_name, static_pool); } } break; @@ -575,7 +594,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, if (PyUnicode_Check(py_value)) { py_ustr1 = PyUnicode_AsUTF8String(py_value); val = strdup(PyBytes_AsString(py_ustr1)); - as_operations_add_prepend_str(ops, bin, val); + as_operations_add_prepend_str(ops, bin_name, val); as_vector_append(unicodeStrVector, &val); Py_DECREF(py_ustr1); } @@ -588,7 +607,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, AEROSPIKE_OK) { goto CLEANUP; } - as_operations_add_prepend_rawp(ops, bin, bytes->value, + as_operations_add_prepend_rawp(ops, bin_name, bytes->value, bytes->size, true); } } @@ -599,8 +618,8 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_binop *binop = &pointer_ops->binops.entries[pointer_ops->binops.size++]; binop->op = AS_OPERATOR_PREPEND; - initialize_bin_for_strictypes(self, err, py_value, binop, bin, - static_pool); + initialize_bin_for_strictypes(self, err, py_value, binop, + bin_name, static_pool); } } break; @@ -613,11 +632,11 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, "integer value exceeds sys.maxsize"); } } - as_operations_add_incr(ops, bin, offset); + as_operations_add_incr(ops, bin_name, offset); } else if (PyFloat_Check(py_value)) { double_offset = PyFloat_AsDouble(py_value); - as_operations_add_incr_double(ops, bin, double_offset); + as_operations_add_incr_double(ops, bin_name, double_offset); } else { if (!self->strict_types || @@ -626,150 +645,157 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_binop *binop = &pointer_ops->binops.entries[pointer_ops->binops.size++]; binop->op = AS_OPERATOR_INCR; - initialize_bin_for_strictypes(self, err, py_value, binop, bin, - static_pool); + initialize_bin_for_strictypes(self, err, py_value, binop, + bin_name, static_pool); } } break; //------- MAP OPERATIONS --------- case OP_MAP_SET_POLICY: - as_operations_map_set_policy(ops, bin, ctx_ref, &map_policy); + as_operations_map_set_policy(ops, bin_name, ctx_ref, &map_policy); break; case OP_MAP_CREATE:; as_map_order order = (as_map_order)PyLong_AsLong(py_map_order); bool persist_index = PyObject_IsTrue(py_persist_index); - as_operations_map_create_all(ops, bin, ctx_ref, order, persist_index); + as_operations_map_create_all(ops, bin_name, ctx_ref, order, + persist_index); break; case OP_MAP_PUT: CONVERT_VAL_TO_AS_VAL(); CONVERT_KEY_TO_AS_VAL(); - as_operations_map_put(ops, bin, ctx_ref, &map_policy, put_key, put_val); + as_operations_map_put(ops, bin_name, ctx_ref, &map_policy, put_key, + put_val); break; case OP_MAP_PUT_ITEMS: CONVERT_VAL_TO_AS_VAL(); - as_operations_map_put_items(ops, bin, ctx_ref, &map_policy, + as_operations_map_put_items(ops, bin_name, ctx_ref, &map_policy, (as_map *)put_val); break; case OP_MAP_INCREMENT: CONVERT_VAL_TO_AS_VAL(); CONVERT_KEY_TO_AS_VAL(); - as_operations_map_increment(ops, bin, ctx_ref, &map_policy, put_key, - put_val); + as_operations_map_increment(ops, bin_name, ctx_ref, &map_policy, + put_key, put_val); break; case OP_MAP_DECREMENT: CONVERT_VAL_TO_AS_VAL(); CONVERT_KEY_TO_AS_VAL(); - as_operations_map_decrement(ops, bin, ctx_ref, &map_policy, put_key, - put_val); + as_operations_map_decrement(ops, bin_name, ctx_ref, &map_policy, + put_key, put_val); break; case OP_MAP_SIZE: - as_operations_map_size(ops, bin, ctx_ref); + as_operations_map_size(ops, bin_name, ctx_ref); break; case OP_MAP_CLEAR: - as_operations_map_clear(ops, bin, ctx_ref); + as_operations_map_clear(ops, bin_name, ctx_ref); break; case OP_MAP_REMOVE_BY_KEY: CONVERT_KEY_TO_AS_VAL(); - as_operations_map_remove_by_key(ops, bin, ctx_ref, put_key, + as_operations_map_remove_by_key(ops, bin_name, ctx_ref, put_key, return_type); break; case OP_MAP_REMOVE_BY_KEY_LIST: CONVERT_VAL_TO_AS_VAL(); - as_operations_map_remove_by_key_list(ops, bin, ctx_ref, + as_operations_map_remove_by_key_list(ops, bin_name, ctx_ref, (as_list *)put_val, return_type); break; case OP_MAP_REMOVE_BY_KEY_RANGE: CONVERT_VAL_TO_AS_VAL(); CONVERT_KEY_TO_AS_VAL(); - as_operations_map_remove_by_key_range(ops, bin, ctx_ref, put_key, + as_operations_map_remove_by_key_range(ops, bin_name, ctx_ref, put_key, put_val, return_type); break; case OP_MAP_REMOVE_BY_VALUE: CONVERT_VAL_TO_AS_VAL(); - as_operations_map_remove_by_value(ops, bin, ctx_ref, put_val, + as_operations_map_remove_by_value(ops, bin_name, ctx_ref, put_val, return_type); break; case OP_MAP_REMOVE_BY_VALUE_LIST: CONVERT_VAL_TO_AS_VAL(); - as_operations_map_remove_by_value_list(ops, bin, ctx_ref, + as_operations_map_remove_by_value_list(ops, bin_name, ctx_ref, (as_list *)put_val, return_type); break; case OP_MAP_REMOVE_BY_VALUE_RANGE: CONVERT_VAL_TO_AS_VAL(); CONVERT_RANGE_TO_AS_VAL(); - as_operations_map_remove_by_value_range(ops, bin, ctx_ref, put_val, + as_operations_map_remove_by_value_range(ops, bin_name, ctx_ref, put_val, put_range, return_type); break; case OP_MAP_REMOVE_BY_INDEX: - as_operations_map_remove_by_index(ops, bin, ctx_ref, index, + as_operations_map_remove_by_index(ops, bin_name, ctx_ref, index, return_type); break; case OP_MAP_REMOVE_BY_INDEX_RANGE: if (pyobject_to_index(self, err, py_value, &offset) != AEROSPIKE_OK) { goto CLEANUP; } - as_operations_map_remove_by_index_range(ops, bin, ctx_ref, index, + as_operations_map_remove_by_index_range(ops, bin_name, ctx_ref, index, offset, return_type); break; case OP_MAP_REMOVE_BY_RANK: - as_operations_map_remove_by_rank(ops, bin, ctx_ref, index, return_type); + as_operations_map_remove_by_rank(ops, bin_name, ctx_ref, index, + return_type); break; case OP_MAP_REMOVE_BY_RANK_RANGE: if (pyobject_to_index(self, err, py_value, &offset) != AEROSPIKE_OK) { goto CLEANUP; } - as_operations_map_remove_by_rank_range(ops, bin, ctx_ref, index, offset, - return_type); + as_operations_map_remove_by_rank_range(ops, bin_name, ctx_ref, index, + offset, return_type); break; case OP_MAP_GET_BY_KEY: CONVERT_KEY_TO_AS_VAL(); - as_operations_map_get_by_key(ops, bin, ctx_ref, put_key, return_type); + as_operations_map_get_by_key(ops, bin_name, ctx_ref, put_key, + return_type); break; case OP_MAP_GET_BY_KEY_RANGE: CONVERT_RANGE_TO_AS_VAL(); CONVERT_KEY_TO_AS_VAL(); - as_operations_map_get_by_key_range(ops, bin, ctx_ref, put_key, + as_operations_map_get_by_key_range(ops, bin_name, ctx_ref, put_key, put_range, return_type); break; case OP_MAP_GET_BY_KEY_LIST: CONVERT_VAL_TO_AS_VAL(); - as_operations_map_get_by_key_list(ops, bin, ctx_ref, (as_list *)put_val, - return_type); + as_operations_map_get_by_key_list(ops, bin_name, ctx_ref, + (as_list *)put_val, return_type); break; case OP_MAP_GET_BY_VALUE: CONVERT_VAL_TO_AS_VAL(); - as_operations_map_get_by_value(ops, bin, ctx_ref, put_val, return_type); + as_operations_map_get_by_value(ops, bin_name, ctx_ref, put_val, + return_type); break; case OP_MAP_GET_BY_VALUE_RANGE: CONVERT_VAL_TO_AS_VAL(); CONVERT_RANGE_TO_AS_VAL(); - as_operations_map_get_by_value_range(ops, bin, ctx_ref, put_val, + as_operations_map_get_by_value_range(ops, bin_name, ctx_ref, put_val, put_range, return_type); break; case OP_MAP_GET_BY_VALUE_LIST: CONVERT_VAL_TO_AS_VAL(); - as_operations_map_get_by_value_list(ops, bin, ctx_ref, + as_operations_map_get_by_value_list(ops, bin_name, ctx_ref, (as_list *)put_val, return_type); break; case OP_MAP_GET_BY_INDEX: - as_operations_map_get_by_index(ops, bin, ctx_ref, index, return_type); + as_operations_map_get_by_index(ops, bin_name, ctx_ref, index, + return_type); break; case OP_MAP_GET_BY_INDEX_RANGE: if (pyobject_to_index(self, err, py_value, &offset) != AEROSPIKE_OK) { goto CLEANUP; } - as_operations_map_get_by_index_range(ops, bin, ctx_ref, index, offset, - return_type); + as_operations_map_get_by_index_range(ops, bin_name, ctx_ref, index, + offset, return_type); break; case OP_MAP_GET_BY_RANK: - as_operations_map_get_by_rank(ops, bin, ctx_ref, index, return_type); + as_operations_map_get_by_rank(ops, bin_name, ctx_ref, index, + return_type); break; case OP_MAP_GET_BY_RANK_RANGE: if (pyobject_to_index(self, err, py_value, &offset) != AEROSPIKE_OK) { goto CLEANUP; } - as_operations_map_get_by_rank_range(ops, bin, ctx_ref, index, offset, - return_type); + as_operations_map_get_by_rank_range(ops, bin_name, ctx_ref, index, + offset, return_type); break; default: @@ -796,7 +822,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, * @param err The as_error to be populated by the function * with the encountered error if any. * @param key The C client's as_key that identifies the record. - * @param py_list The list containing op, bin and value. + * @param py_list The list containing op, bin_name and value. * @param py_meta The metadata for the operation. * @param py_policy Python dict used to populate the operate_policy or map_policy. ******************************************************************************************************* @@ -965,7 +991,7 @@ PyObject *AerospikeClient_Operate(AerospikeClient *self, PyObject *args, * @param err The as_error to be populated by the function * with the encountered error if any. * @param key The C client's as_key that identifies the record. - * @param py_list The list containing op, bin and value. + * @param py_list The list containing op, bin_name and value. * @param py_meta The metadata for the operation. * @param operate_policy_p The value for operate policy. ******************************************************************************************************* @@ -1192,7 +1218,7 @@ PyObject *AerospikeClient_OperateOrdered(AerospikeClient *self, PyObject *args, /** ******************************************************************************************************* - * Appends a string to the string value in a bin. + * Appends a string to the string value in a bin_name. * * @param self AerospikeClient object * @param args The args is a tuple object containing an argument @@ -1211,7 +1237,7 @@ PyObject *AerospikeClient_Append(AerospikeClient *self, PyObject *args, PyObject *py_append_str = NULL; // Python Function Keyword Arguments - static char *kwlist[] = {"key", "bin", "val", "meta", "policy", NULL}; + static char *kwlist[] = {"key", "bin_name", "val", "meta", "policy", NULL}; if (PyArg_ParseTupleAndKeywords(args, kwds, "OOO|OO:append", kwlist, &py_key, &py_bin, &py_append_str, &py_meta, &py_policy) == false) { @@ -1239,7 +1265,7 @@ PyObject *AerospikeClient_Append(AerospikeClient *self, PyObject *args, /** ******************************************************************************************************* - * Prepends a string to the string value in a bin + * Prepends a string to the string value in a bin_name * * @param self AerospikeClient object * @param args The args is a tuple object containing an argument @@ -1258,7 +1284,7 @@ PyObject *AerospikeClient_Prepend(AerospikeClient *self, PyObject *args, PyObject *py_prepend_str = NULL; // Python Function Keyword Arguments - static char *kwlist[] = {"key", "bin", "val", "meta", "policy", NULL}; + static char *kwlist[] = {"key", "bin_name", "val", "meta", "policy", NULL}; if (PyArg_ParseTupleAndKeywords(args, kwds, "OOO|OO:prepend", kwlist, &py_key, &py_bin, &py_prepend_str, &py_meta, &py_policy) == false) { @@ -1287,7 +1313,7 @@ PyObject *AerospikeClient_Prepend(AerospikeClient *self, PyObject *args, /** ******************************************************************************************************* - * Increments a numeric value in a bin. + * Increments a numeric value in a bin_name. * * @param self AerospikeClient object * @param args The args is a tuple object containing an argument @@ -1306,7 +1332,8 @@ PyObject *AerospikeClient_Increment(AerospikeClient *self, PyObject *args, PyObject *py_offset_value = 0; // Python Function Keyword Arguments - static char *kwlist[] = {"key", "bin", "offset", "meta", "policy", NULL}; + static char *kwlist[] = {"key", "bin_name", "offset", + "meta", "policy", NULL}; if (PyArg_ParseTupleAndKeywords(args, kwds, "OOO|OO:increment", kwlist, &py_key, &py_bin, &py_offset_value, &py_meta, &py_policy) == false) { From 9a354a541405401f49db14423a5902cdbcf7e05d Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 10 Oct 2024 12:36:38 -0700 Subject: [PATCH 11/11] m --- src/main/client/operate.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/main/client/operate.c b/src/main/client/operate.c index 20874b246..b12b6e9b8 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -102,12 +102,6 @@ static inline bool isExprOp(int op); } \ } -#define CONVERT_VAL_TO_AS_VAL() \ - if (pyobject_to_val(self, err, py_value, &put_val, static_pool, \ - SERIALIZER_PYTHON) != AEROSPIKE_OK) { \ - return err->code; \ - } - #define CONVERT_KEY_TO_AS_VAL() \ if (pyobject_to_val(self, err, py_key, &put_key, static_pool, \ SERIALIZER_PYTHON) != AEROSPIKE_OK) { \ @@ -294,17 +288,22 @@ bool opRequiresKey(int op) op == OP_MAP_GET_BY_KEY_RANGE); } -static bool op_requires_bin_name[] = { - [AS_OPERATOR_READ] = true, [AS_OPERATOR_WRITE] = true, - [AS_OPERATOR_INCR] = true, [AS_OPERATOR_APPEND] = true, - [AS_OPERATOR_PREPEND] = true, -}; +static bool op_requires_bin_name[] = {[AS_OPERATOR_READ] = true, + [AS_OPERATOR_WRITE] = true, + [AS_OPERATOR_INCR] = true, + [AS_OPERATOR_APPEND] = true, + [AS_OPERATOR_PREPEND] = true, + // Last element of enum + [AS_OPERATOR_HLL_MODIFY] = false}; + +static bool op_requires_value[] = {[AS_OPERATOR_WRITE] = true, + // TODO + [AS_OPERATOR_HLL_MODIFY] = false}; as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, as_vector *unicodeStrVector, as_static_pool *static_pool, as_operations *ops, long *op, long *ret_type) { - as_val *put_val = NULL; as_val *put_key = NULL; as_val *put_range = NULL; as_cdt_ctx ctx; @@ -373,6 +372,14 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, } } + as_val *put_val = NULL; + if (op_requires_value[op_code]) { + if (pyobject_to_val(self, err, py_value, &put_val, static_pool, + SERIALIZER_PYTHON) != AEROSPIKE_OK) { + return err->code; + } + } + // TODO: use python hashset? const char *valid_keys[] = {"op", "bin_name", "index", "key", "val", "return_type"}; @@ -397,6 +404,7 @@ as_status add_op(AerospikeClient *self, as_error *err, PyObject *py_op_dict, // No way to define an array of function pointers with differing arguments switch (op_code) { + // TODO case AS_OPERATOR_TOUCH: if (py_value) { if (pyobject_to_index(self, err, py_value, &ttl) != AEROSPIKE_OK) {