vef: allow VDFs as type operations#59
Conversation
malone-at-work
changed the title
malone-at-work
force-pushed
the
maloney/vdf/1
branch
2 times, most recently
from
e8474ac to
dc017d6
Compare
malone-at-work
force-pushed
the
maloney/vdf/1
branch
2 times, most recently
from
5dfb1d6 to
36aa905
Compare
| Complex cx = load_complex(in->bin_value); | ||
| int written = | ||
| snprintf(out->str_buf, out->max_str_len, "(%g,%g)", cx.re, cx.im); | ||
| if (written < 0 || static_cast<size_t>(written) >= out->max_str_len) { |
There was a problem hiding this comment.
Does written < 0 mean the buffer is too small necessarily?
There was a problem hiding this comment.
according to man the only other reason is invalid characters in the format string, which would be a compile time error with most tool chains.
| // Compare VDF for ORDER BY, indexes: (COMPLEX, COMPLEX) -> INT | ||
| void complex_compare(vef_context_t *ctx, vef_invalue_t *in_l, | ||
| vef_invalue_t *in_r, vef_vdf_result_t *out) { | ||
| if (in_l->bin_len < kComplexSize || in_r->bin_len < kComplexSize) { |
There was a problem hiding this comment.
I wonder why we only checked this direction, instead of !=
There was a problem hiding this comment.
Fixed to use the style I had previously changed the arithmetic functions below to.
| // working correctly with hash joins and EXCEPT operations. | ||
| void complex2_hash(vef_context_t *ctx, vef_invalue_t *in, | ||
| vef_vdf_result_t *out) { | ||
| if (in->bin_len < kComplexSize) { |
There was a problem hiding this comment.
Why do we do this? Is it because we have no way to flag an error here? We could flag an error, but i don't think call sites would be able to handle it well.
There was a problem hiding this comment.
'twas what the previous code did, I was trying not to change semantics.
This change does give us the opportunity to return an error, and fall back to the standard memory hash. So I will convert it, if you think this is best.
|
|
||
| void ReturnError(const char *err_msg, vef_vdf_result_t *result) { | ||
| result->type = VEF_RESULT_ERROR; | ||
| snprintf(result->error_msg, VEF_MAX_ERROR_LEN, "%s", err_msg); |
There was a problem hiding this comment.
The implementations of ReturnError are so different from complex and this. Should we not be opinionated about which is better?
There was a problem hiding this comment.
I would like to replace the raw ABI types in all of these with more C++ equivalents.
| *length = 0; | ||
| return true; | ||
| // STRING -> TVECTOR | ||
| // Dimension is inferred from out->max_bin_len / 4. |
There was a problem hiding this comment.
Ok, but this is going to change in my next change (well, once type params are plumbed). Just FYI.
|
This PR is blocked on investigating empty key comparison in Temp Tables. |
Extend vef_type_desc_t (protocol 2) with encode/decode/compare/hash_vdf_name fields so extensions can name a registered VDF as the implementation of each type operation instead of providing a raw function pointer. Server-side: resolve VDF names at extension registration, validate signatures (including rejecting prerun/postrun hooks), and encapsulate fn-ptr vs VDF dispatch in new EncodeOp/DecodeOp/CompareOp/HashOp wrapper classes. These wrappers are registered in the TypeDescriptor; all call sites use invoke() which hide the protocol differences SDK: add overloaded .encode()/.decode()/.compare()/.hash() builder methods, that take the VDF. Convert vsql-complex and vsql-tvector examples to the new style.
malone-at-work
force-pushed
the
maloney/vdf/1
branch
from
0f39469 to
2192f26
Compare
|
Blocking issue was fixed in "fix custom type compare bug (#64)" I will merge when tests pass. |
2 participants
Extend vef_type_desc_t (protocol 2) with encode/decode/compare/hash_vdf_name fields so extensions can name a registered VDF as the implementation of each type operation instead of providing a raw function pointer.
Server-side: resolve VDF names at extension registration, validate signatures (including rejecting prerun/postrun hooks), and encapsulate fn-ptr vs VDF dispatch in new EncodeOp/DecodeOp/CompareOp/HashOp wrapper classes.
These wrappers are registered in the TypeDescriptor; all call sites use invoke() which hide the protocol differences
SDK: add overloaded .encode()/.decode()/.compare()/.hash() builder methods, that take the VDF. Convert vsql-complex and vsql-tvector examples to the new style.