Hi, Alexander! On Apr 09, Alexander Barkov wrote: > Hi Sergei, > > I'm looking at this use rule in sql_yacc.yy: > > history_point: > temporal_literal > { $$= Vers_history_point(VERS_TIMESTAMP, $1); } > | function_call_keyword_timestamp > { $$= Vers_history_point(VERS_TIMESTAMP, $1); } > | opt_history_unit simple_expr > { $$= Vers_history_point($1, $2); } > ; > > AS OF DATE 'xxx' - returns Vers_history_point(VERS_TIMESTAMP) > AS OF DATE('xxx') - returns Vers_history_point(VERS_UNDEFINED)
I suspect that's ok, because with VERS_UNDEFINED it'll look at the result_type() and will change to VERS_TIMESTAMP. Might've been better to look at cmp_type() though. > AS OF TIMESTAMP('2001-01-01') - returns Vers_history_point(VERS_TIMESTAMP) > AS OF COALESCE(TIMESTAMP('2001-01-01')) - returns > Vers_history_point(VERS_UNDEFINED) Same. > Perhaps this is not expected. > > Also, this code looks suspicious: > > 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. > 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. > Can't "history_point" be simplified to something like: > > history_point: > simple_expr { $$= Vers_history_point(VERS_UNDEFINED, $1); } > | TRANSACTION_SYM simple_expr { $$= Vers_history_point(VERS_TRX_ID, $2); } > ; > > 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. Now I can think of more ways of introducing the identifier TRANSACTION there, like CREATE TABLE t1 (TRANSACTION int); CREATE TABLE t2 (...) WITH SYSTEM VERSIONING; ... SELECT (SELECT * FROM t1 FOR SYSTEM TIME AS OF TRANSACTION) FROM t1; Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ 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