From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
To: tarantool-patches@freelists.org,
Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v1 0/4] sql: store regular identifiers in case-normal form
Date: Fri, 22 Feb 2019 15:38:56 +0300 [thread overview]
Message-ID: <f2329866-7dcc-50d1-e98d-4c5b995b1a87@tarantool.org> (raw)
In-Reply-To: <d1588da2-f1ef-fe84-597d-090a2987f68e@tarantool.org>
> Talking about the patchset - I do not understand a point of
> the first 3 patches. I see, that the only reason why you
> pass the parser is a wish to set nErr. Why instead not to just
> do diag_set, return -1, and check for that in all upper functions,
> until parser is already in one of them? This how all box functions
> work with SQL right now - they do not take Vdbe, nor Parse. They
> just set diag and return -1.
The problem is that when the family of functions calling sql_normalize_name return NULL,
it is assumed that a memory allocation error has occurred, although this is not the case.
The API for working with expressions is heterogeneous, and in some cases already accepts Parser -
for example sqlPExpr, sqlPExprAddSelect.
Moreover, in the same way, to set a third-party error it is used in sql Expr: see sqlExprCheckHeight
that set
sqlErrorMsg(pParse, "Expression tree is too large (maximum depth %d)", mxHeight);
Thus, I do not introduce new practices to the API, but on the contrary, bring it to a more consistent state.
The alternative that you propose to use the forced setting SQL_TARANTOOL_ERROR (in some cases) three functions.
the stack if the function returned NULL and !db.mallocFailed. This will work, however, for the same sqlPExpr analogy
will be incorrect: it is not true that if the expression construction function returned NULL, a Tarantool error occurred.
This will confuse the already conflicting SQL code.
next prev parent reply other threads:[~2019-02-22 12:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-15 13:30 [tarantool-patches] " Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 1/4] sql: patch sql_name_from_token to use Parser Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 2/4] sql: patch sql_trigger_step_allocate " Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 3/4] sql: patch sql_expr_create routine " Kirill Shcherbatov
2019-02-15 13:30 ` [tarantool-patches] [PATCH v1 4/4] sql: store regular identifiers in case-normal form Kirill Shcherbatov
2019-02-22 12:20 ` [tarantool-patches] Re: [PATCH v1 0/4] " Vladislav Shpilevoy
2019-02-22 12:38 ` Kirill Shcherbatov [this message]
2019-02-22 12:43 ` Vladislav Shpilevoy
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=f2329866-7dcc-50d1-e98d-4c5b995b1a87@tarantool.org \
--to=kshcherbatov@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=v.shpilevoy@tarantool.org \
--subject='[tarantool-patches] Re: [PATCH v1 0/4] sql: store regular identifiers in case-normal form' \
/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