From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov <kshcherbatov@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v1 05/12] sql: put analyze helpers to FuncDef cache Date: Tue, 9 Jul 2019 18:56:34 +0300 [thread overview] Message-ID: <9F3E18B0-FBB3-40CC-ACF3-95003DA43EFD@tarantool.org> (raw) In-Reply-To: <aef7b0f5603e29f307336ea896bdbbe706931728.1562584567.git.kshcherbatov@tarantool.org> Prior patches are OK and I guess can be pushed - they are quite trivial (except for previous one - which introduces persistent functions in Tarantool). Hence you can swap order of patches (i.e. this and previous one) so that push series of patches independently from non-trivial. Also, please fix compilation errors which appear on Travis. > On 8 Jul 2019, at 14:26, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote: > > Previously analyze functions refer to statically defined > service FuncDef context. We need to change this approach due we > going to rework the builtins functions machinery in following > patches. > > Needed for #4113, #2200, #2233 > — Unfortunately, analyze is currently disabled, so there’s no opportunity to check functionality of this patch :) > @@ -986,8 +956,11 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space) > sqlVdbeAddOp3(v, OP_MakeRecord, stat_key_reg, > pk_part_count, key_reg); > assert(chng_reg == (stat4_reg + 1)); > + struct FuncDef *push_func = > + sqlFindFunction(sql_get(), "_sql_stat_push", 3, 0); > + assert(push_func != NULL); > sqlVdbeAddOp4(v, OP_Function0, 1, stat4_reg, tmp_reg, > - (char *)&statPushFuncdef, P4_FUNCDEF); > + (char *)push_func, P4_FUNCDEF); > sqlVdbeChangeP5(v, 3); > sqlVdbeAddOp2(v, OP_Next, idx_cursor, next_row_addr); > /* Add the entry to the stat1 table. */ > @@ -1774,3 +1747,58 @@ fail: > box_txn_rollback(); > return -1; > } > + > +/** > + * The implementation of the sql_record() function. This function > + * accepts a single argument of any type. The return value is a > + * formatted database record (a blob) containing the argument > + * value. > + * > + * This is used to convert the value stored in the 'sample' > + * column of the sql_stat4 table to the record format sql uses > + * internally. > + */ > +static void > +sql_record_func(sql_context * context, int argc, sql_value ** argv) I guess there’s no need in this function at all. Could you simply remove it? > +{ > + const int file_format = 1; > + u32 iSerial; /* Serial type */ > + int nSerial; /* Bytes of space for iSerial as varint */ > + u32 nVal; /* Bytes of space required for argv[0] */ > + int nRet; > + sql *db; > + u8 *aRet; > + > + UNUSED_PARAMETER(argc); > + iSerial = sqlVdbeSerialType(argv[0], file_format, &nVal); > + nSerial = sqlVarintLen(iSerial); > + db = sql_context_db_handle(context); > + > + nRet = 1 + nSerial + nVal; > + aRet = sqlDbMallocRawNN(db, nRet); > + if (aRet == 0) { > + context->is_aborted = true; > + } else { > + aRet[0] = nSerial + 1; > + putVarint32(&aRet[1], iSerial); > + sqlVdbeSerialPut(&aRet[1 + nSerial], argv[0], iSerial); > + sql_result_blob(context, aRet, nRet, SQL_TRANSIENT); > + sqlDbFree(db, aRet); > + } > +} > + > +#define FUNCTION_ANALYZE(zName, nArg, xFunc) \ > + {nArg, SQL_FUNC_CONSTANT, NULL, 0, xFunc, 0, #zName, {0},\ > + FIELD_TYPE_ANY, false} Could you move this macro to other FUNCTION_… ones (sqlInt.h)? > + > +void > +sql_register_analyze_builtins(void) > +{ > + static FuncDef aAnalyzeTableFuncs[] = { Please, use names according to our code style.
next prev parent reply other threads:[~2019-07-09 15:56 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-08 11:26 [PATCH v1 00/12] sql: uniform SQL and Lua functions subsystem Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 01/12] sql: rfc for SQL and Lua functions Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 10/12] sql: remove SQL_PreferBuiltin flag Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 11/12] sql: move LIKE UConverter object to collation library Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 12/12] sql: use schema's func hash instead of FuncDef hash Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 02/12] sql: get rid of SOUNDEX, MATCH Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 03/12] sql: get rid of LIKELY, UNLIKELY and LIKEHOOD Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 04/12] box: introduce Lua persistent functions Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 05/12] sql: put analyze helpers to FuncDef cache Kirill Shcherbatov 2019-07-09 15:56 ` n.pettik [this message] 2019-07-08 11:26 ` [PATCH v1 06/12] sql: rework LIKE case-insensitive mode Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 07/12] sql: replace bool is_derived_coll marker with flag Kirill Shcherbatov 2019-07-09 16:13 ` [tarantool-patches] " n.pettik 2019-07-08 11:26 ` [PATCH v1 08/12] sql: refactor builtins signatures with port Kirill Shcherbatov 2019-07-08 11:26 ` [PATCH v1 09/12] box: use own vtab per each function object Kirill Shcherbatov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=9F3E18B0-FBB3-40CC-ACF3-95003DA43EFD@tarantool.org \ --to=korablev@tarantool.org \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v1 05/12] sql: put analyze helpers to FuncDef cache' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox