Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

date_add/date_sub can return mysql.Time. #9813

Closed
winoros opened this issue Mar 19, 2019 · 0 comments · Fixed by #35009
Closed

date_add/date_sub can return mysql.Time. #9813

winoros opened this issue Mar 19, 2019 · 0 comments · Fixed by #35009
Assignees
Labels
component/expression priority/P4 Minor issue, awaiting more evidence before prioritizing type/compatibility

Comments

@winoros
Copy link
Member

winoros commented Mar 19, 2019

Bug Report

Although in MySQL's docs:

The return value depends on the arguments:

DATE if the date argument is a DATE value and your calculations involve only YEAR, MONTH, and DAY parts (that is, no time parts).

DATETIME if the first argument is a DATETIME (or TIMESTAMP) value, or if the first argument is a DATE and the unit value uses HOURS, MINUTES, or SECONDS.

String otherwise.

But in its actual implementation:

bool Item_date_add_interval::resolve_type(THD *) {
  maybe_null = true;

  /*
    The field type for the result of an Item_date function is defined as
    follows:

    - If first arg is a MYSQL_TYPE_DATETIME result is MYSQL_TYPE_DATETIME
    - If first arg is a MYSQL_TYPE_DATE and the interval type uses hours,
      minutes or seconds then type is MYSQL_TYPE_DATETIME.
    - Otherwise the result is MYSQL_TYPE_STRING
      (This is because you can't know if the string contains a DATE, MYSQL_TIME
    or DATETIME argument)
  */
  enum_field_types arg0_data_type = args[0]->data_type();
  uint8 interval_dec = 0;
  if (int_type == INTERVAL_MICROSECOND ||
      (int_type >= INTERVAL_DAY_MICROSECOND &&
       int_type <= INTERVAL_SECOND_MICROSECOND))
    interval_dec = DATETIME_MAX_DECIMALS;
  else if (int_type == INTERVAL_SECOND && args[1]->decimals > 0)
    interval_dec = MY_MIN(args[1]->decimals, DATETIME_MAX_DECIMALS);

  if (arg0_data_type == MYSQL_TYPE_DATETIME ||
      arg0_data_type == MYSQL_TYPE_TIMESTAMP) {
    uint8 dec = MY_MAX(args[0]->datetime_precision(), interval_dec);
    set_data_type_datetime(dec);
  } else if (arg0_data_type == MYSQL_TYPE_DATE) {
    if (int_type <= INTERVAL_DAY || int_type == INTERVAL_YEAR_MONTH)
      set_data_type_date();
    else
      set_data_type_datetime(interval_dec);
  } else if (arg0_data_type == MYSQL_TYPE_TIME) {
    uint8 dec = MY_MAX(args[0]->time_precision(), interval_dec);
    set_data_type_time(dec);
  } else {
    /* Behave as a usual string function when return type is VARCHAR. */
    set_data_type_char(MAX_DATETIME_FULL_WIDTH, default_charset());
  }
  if (value.alloc(max_length)) return true;

  return false;
}

The return type can also be Time, which is Duration in TiDB.

So currently if the arg[0] is Duration we'll get the result with the year, month and day field. Which is not a reasonable return value.
We need to cover this unexpected case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression priority/P4 Minor issue, awaiting more evidence before prioritizing type/compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant