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

Fix time zone compatibility #1631

Merged
merged 8 commits into from
Aug 13, 2019

Conversation

HangyuanLiu
Copy link
Contributor

Fix time zone compatibility with old fe

…to time-zone-be-function

Conflicts:
	be/src/exprs/timestamp_functions.cpp
	be/test/exprs/timestamp_functions_test.cpp
	fe/src/main/java/org/apache/doris/qe/Coordinator.java
@@ -182,7 +182,8 @@ struct TPlanFragmentExecParams {
struct TQueryGlobals {
// String containing a timestamp set as the current time.
1: required string now_string
2: optional string time_zone
2: optional i64 timestamp
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some comments to describe timestamp and now_string's relation

Suggested change
2: optional i64 timestamp
2: optional i64 timestamp_ms

if (query_globals.__isset.time_zone) {
_timezone = query_globals.time_zone;
_timestamp_ms = query_globals.timestamp;
} else if (query_globals.now_string != "") {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this string be empty? I think it is no need to test it

@HangyuanLiu HangyuanLiu force-pushed the fix-time-zone-compatibility branch from 82db5ec to 3312e6c Compare August 13, 2019 06:35
@HangyuanLiu HangyuanLiu force-pushed the fix-time-zone-compatibility branch from 3312e6c to 3fbd5bc Compare August 13, 2019 06:52
imay
imay previously approved these changes Aug 13, 2019
Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@morningman morningman merged commit 199ff96 into apache:master Aug 13, 2019
@imay imay mentioned this pull request Sep 26, 2019
SWJTU-ZhangLei pushed a commit to SWJTU-ZhangLei/incubator-doris that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants