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 deserialization issue of TimeProcessor #40776

Merged
merged 2 commits into from
Apr 3, 2019

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Apr 3, 2019

TimeProcessor didn't implement getWriteableName() so the one from
the parent was used which returned the NAME of the parent. This
caused TimeProcessor objects to be deserialised into
DateTimeProcessor.

Moreover, added a restriction to run the TIME related integration tests
only in UTC timezone.

Fixes: #40717

@matriv matriv added >test-failure Triaged test failures from CI :Analytics/SQL SQL querying v8.0.0 v7.2.0 labels Apr 3, 2019
@matriv matriv requested a review from astefan April 3, 2019 11:26
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

TimeProcessor didn't implement `getWriteableName()` so the one from
the parent was used which returned the `NAME` of the parent. This
caused `TimeProcessor` objects to be deserialised into
DateTimeProcessor.

Moreover, added a restriction to run the TIME related integration tests
only in UTC timezone.

Fixes: elastic#40717
@@ -1,26 +1,26 @@
//
// TIME
//
// All tests are run with UTC timezone
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a GH issue for the timezone problem with Time data type? If so, I would add it here and in the CsvSpecTestCase class as a TODO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I can create one

Copy link
Contributor

@astefan astefan 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 one comment.

@matriv matriv merged commit cfea348 into elastic:master Apr 3, 2019
@matriv matriv deleted the fix-40717 branch April 3, 2019 14:42
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 3, 2019
…leniency

* elastic/master:
  SQL: Fix deserialisation issue of TimeProcessor (elastic#40776)
  Improve GCS docs for using keystore (elastic#40605)
  Add Restore Operation to SnapshotResiliencyTests (elastic#40634)
  Small refactorings to analysis components (elastic#40745)
  SQL: Fix display size for DATE/DATETIME (elastic#40669)
  add HLRC protocol tests for transform state and stats (elastic#40766)
  Inline TransportReplAction#registerRequestHandlers (elastic#40762)
  remove experimental label from search_as_you_type documentation (elastic#40744)
  Remove some abstractions from `TransportReplicationAction` (elastic#40706)
  Upgrade to latest build scan plugin (elastic#40702)
  Use default memory lock setting in testing (elastic#40730)
  Add Bulk Delete Api to BlobStore (elastic#40322)
  Remove yaml skips older than 7.0 (elastic#40183)
  Docs: Move id in the java-api (elastic#40748)
matriv added a commit that referenced this pull request Apr 3, 2019
TimeProcessor didn't implement `getWriteableName()` so the one from
the parent was used which returned the `NAME` of the parent. This
caused `TimeProcessor` objects to be deserialised into
DateTimeProcessor.

Moreover, added a restriction to run the TIME related integration tests
only in UTC timezone.

Fixes: #40717

(cherry picked from commit cfea348)
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
TimeProcessor didn't implement `getWriteableName()` so the one from
the parent was used which returned the `NAME` of the parent. This
caused `TimeProcessor` objects to be deserialised into
DateTimeProcessor.

Moreover, added a restriction to run the TIME related integration tests
only in UTC timezone.

Fixes: elastic#40717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/SQL SQL querying >test-failure Triaged test failures from CI v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SQL [CI]: time.testTimeAsHavingFilter failure
4 participants