maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

I couple of small comments in addition to Sanjas review:

> > +longlong sequence_definition::truncate_value(const Longlong_hybrid& original)
> > +{
> > +  if (is_unsigned)
> > +    return original.to_ulonglong(value_type_max());
> > +  else if (original.is_unsigned_outside_of_signed_range())
> > +    return value_type_max();
> > +  else
> > +    return original.value() > value_type_max() ? value_type_max()
> > +      : original.value() < value_type_min() ? value_type_min()
> > +      : original.value();

  Please remove all 'else' above.
  not needed and makes lines shorter.

  Please cache also original.value() in a local variable.
  Yes, it's a currently defined as { return m_value; } but as this may change
  in the future, it's better to be sure and store in a variable.
  Doing that will also make the code text sligtly shorter:

  Please also adjust the indentention to be as follows:

   return (value > value_type_max() ? value_type_max() :
          value < value_type_min() ? value_type_min()
          value);
In other words
- Operators last on the line
- Long operations should be surronded by braces to make it easier for the editor
  to align things properly.


> > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> > index e3298a4a6c1..27ce8bcdbfc 100644
> > --- a/sql/sql_yacc.yy
> > +++ b/sql/sql_yacc.yy
> > @@ -2605,13 +2611,25 @@ sequence_defs:
> >          ;
> >
> >  sequence_def:
> > -          MINVALUE_SYM opt_equal sequence_truncated_value_num
> > +          AS int_type field_options
> > +          {

Please create a local variable:
                 sequence_definition *seq= lex->create_info.seq_create_info;
and use it below. Makes the long lines much shorter!

> > +            if (unlikely(Lex->create_info.seq_create_info->used_fields &
> > +                         seq_field_used_as))
> > +              my_yyabort_error((ER_DUP_ARGUMENT, MYF(0), "AS"));
> > +            if ($3 & ZEROFILL_FLAG)
> > +                my_yyabort_error((ER_BAD_OPTION_VALUE, MYF(0), "ZEROFILL", "AS"));
> > +            Lex->create_info.seq_create_info->value_type = $2->field_type();
> > +            Lex->create_info.seq_create_info->is_unsigned = $3 & UNSIGNED_FLAG ? true : false;
> > +            Lex->create_info.seq_create_info->used_fields|= seq_field_used_as;
> > +          }

Regards,
Monty