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

SQL: Fix BasicFormatter NPE #37804

Merged
merged 4 commits into from
Jan 24, 2019
Merged

SQL: Fix BasicFormatter NPE #37804

merged 4 commits into from
Jan 24, 2019

Conversation

astefan
Copy link
Contributor

@astefan astefan commented Jan 24, 2019

Lacking tests for this made the bug go through and discovered by manual testing. This PR should fix the bug and, also, adds tests. Bug introduced with #37602

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM. Left 2 comments about logging in tests.

+ "1953-09-02T00:00:00.000Z", result[2]);
assertEquals("dog |1.7976931348623157E308|123124.888 |9912 |goat |"
+ "2000-03-15T21:34:37.443Z", result[3]);
logger.info(result[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't. Not paying attention. Thanks for catching.

@astefan
Copy link
Contributor Author

astefan commented Jan 24, 2019

@elasticmachine run elasticsearch-ci/1

@astefan astefan merged commit 163a27b into elastic:master Jan 24, 2019
@astefan astefan deleted the fix_formatter_npe branch January 24, 2019 13:40
astefan added a commit that referenced this pull request Jan 24, 2019
(cherry picked from commit 163a27b)
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.

None yet

4 participants