Skip to content

Conversation

@XuQianJin-Stars
Copy link
Contributor

@XuQianJin-Stars XuQianJin-Stars commented Jan 8, 2019

What is the purpose of the change

(This PR is designed to solve the problem of The bug of Error parsing ExpressionParser.)

Brief change log

  • I will change WEEKS to WEEK, as follows:

    before

      case expr ~ _ ~ (WEEKS.key | WEEKS.key) => toMilliInterval(expr, 7 * MILLIS_PER_DAY)

    after

      case expr ~ _ ~ (WEEKS.key | WEEK.key) => toMilliInterval(expr, 7 * MILLIS_PER_DAY)

Verifying this change

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • I added a test to ScalarFunctionsTest.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (yes)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (yes)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no )
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (yes / no)

  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@xueyumusic
Copy link
Contributor

LGTM, it is a bug and the test didn't cover..thanks, @XuQianJin-Stars

@sunjincheng121
Copy link
Member

Hi @XuQianJin-Stars Thanks for the PR. Please clean up the PR summary. You can refer to the PR example that has been completed. e.g.: #7373
Best,
Jincheng

@XuQianJin-Stars
Copy link
Contributor Author

hi @sunjincheng121
thankyou, Thanks for reminding me that I forgot to clean up the PR before
Best,
qianjin

@XuQianJin-Stars
Copy link
Contributor Author

hi @xueyumusic can leave a contact information to add a friend?
Best,
qianjin

@XuQianJin-Stars
Copy link
Contributor Author

hi @sunjincheng121 @xueyumusic now there is a test broken in CI environment,It is not caused by the currently modified PR

Copy link
Contributor

@twalthr twalthr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @XuQianJin-Stars. Merging...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants