Hi Sergei, Alexander! Maybe it's better to remove TIMESTAMP/TRANSACTION auto-detection as it requires too much code to make it properly? And make TIMESTAMP by default. On Mon, Apr 9, 2018 at 5:49 PM, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Alexander!
On Apr 09, Alexander Barkov wrote:
Note, history_point adds around 20 shift/reduce conflicts. I'd like to resolve them.
good :)
Might've been better to look at cmp_type() though.
A new method in Type_handler would be even better.
okay
void Vers_history_point::resolve_unit(bool timestamps_only) { if (item && unit == VERS_UNDEFINED) { if (item->type() == Item::FIELD_ITEM || timestamps_only) unit= VERS_TIMESTAMP; else if (item->result_type() == INT_RESULT || item->result_type() == REAL_RESULT) unit= VERS_TRX_ID; else unit= VERS_TIMESTAMP; } }
Why DECIMAL_RESULT is not handled?
Looks like a bug. I think it should be.
Should I report a bug, or will you do?
you do, please.
Why only Item::FIELD_ITEM is checked?
https://github.com/tempesta-tech/mariadb/issues/397
Vers_history_point::resolve_unit is called too early, fields are not fixed yet, so item->result_type() crashed.
What about other Item types? Their result_type() methods is called in non-fixed state. I'd say this is a bug.
I agree
Are you going to fix this? Is it reported?
I don't think so.
Btw, this makes me think that a DBUG_ASSERT must be added into Field::result_type(), like this:
Item_result result_type() const { DBUG_ASSERT(fixed); --- to be added return type_handler()->result_type(); }
makes sense
So, because of the problem with result_type() being called in non-fixed state, we have a rule "history_point" which catches most useful constant and functions and resolves them immediately to VERS_TIMESTAMP, while other Item times are resolved in Vers_history_point::resolve_unit().
In case of a fixed-type Items, for example INT and REAL functions, they get resolved as VERS_TRX_ID.
Hybrid type Items, e.g. function COALESCE or CASE, in non-fixed state return REAL_RESULT. So they get resolved as VERS_TRX_ID. Even if their appear to be of a temporal data type later.
this is clearly a bug too.
I propose 1. Either we fix the code to call resolve_unit() in fixed state. 2. Or, if it's too hard, at least temporary disallow hybrid data type Items to be used in AS OF.
Is N1 doable quickly?
I didn't look into it yet
I'm saying "something like" because TRANSACTION_SYM will cause a conflict with simple_expr. So probably it should be cought somehow else, by catching Item_ident and checking it name.
Btw, an SP variable with name "TRANSACTION" does not work well:
Good point. That's a bug.
Should I report a bug, or will you do?
please do
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
-- All the best, Aleksey Midenkov @midenok