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

Refactor currentTimeFunctionClass and utcTimeFunctionClass #27760

Open
wjhuang2016 opened this issue Sep 2, 2021 · 6 comments
Open

Refactor currentTimeFunctionClass and utcTimeFunctionClass #27760

wjhuang2016 opened this issue Sep 2, 2021 · 6 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement

Comments

@wjhuang2016
Copy link
Member

wjhuang2016 commented Sep 2, 2021

Enhancement

currentTimeFunctionClass is very simlier to currentTimeFunctionClass. We can see from the MySQL's code:

class Item_func_curtime_local final : public Item_func_curtime {
 protected:
  Time_zone *time_zone() override;

 public:
  Item_func_curtime_local(const POS &pos, uint8 dec_arg)
      : Item_func_curtime(pos, dec_arg) {}
  const char *func_name() const override { return "curtime"; }
};

class Item_func_curtime_utc final : public Item_func_curtime {
 protected:
  Time_zone *time_zone() override;

 public:
  Item_func_curtime_utc(const POS &pos, uint8 dec_arg)
      : Item_func_curtime(pos, dec_arg) {}
  const char *func_name() const override { return "utc_time"; }
};

This means that we can reuse some codes.
After investigation, I found that utcTimeFunctionClass's flen is more accurate than currentTimeFunctionClass. We can fix it by using utcTimeFunctionClass's logic.

@wjhuang2016 wjhuang2016 added type/enhancement help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Sep 2, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Sep 3, 2021

@jackwener you can comment /assign to assign your self

@tisonkun
Copy link
Contributor

tisonkun commented Sep 3, 2021

This time I just do for you (x

@jackwener
Copy link
Contributor

This time I just do for you (x

Thank you !
Some changes misled me.

@tisonkun
Copy link
Contributor

tisonkun commented Sep 3, 2021

@jackwener never mind. I'd like to see more conversation instead of commands. For example, what do you think to fix this issue? Or it is simple enough to just push a PR?

@jackwener
Copy link
Contributor

@wjhuang2016 Hi, there may be some typo in your description due to c/v.

@wjhuang2016
Copy link
Member Author

@wjhuang2016 Hi, there may be some typo in your description due to c/v.

Yeah, I've fixed it.

@jackwener jackwener removed their assignment Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement
Projects
None yet
Development

No branches or pull requests

3 participants