maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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@xxxxxxxxxxx