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 <s...@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 secur...@mariadb.org -- All the best, Aleksey Midenkov @midenok _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp