Refactor DateLiteral class in FE #1644

HangyuanLiu · 2019-08-15T02:36:16Z

1、Add fe time zone function support
2、Refactor DateLiteral class in FE
#1583

…tion

…u/incubator-doris into fe-time-zone-function

…u/incubator-doris into fe-time-zone-function # Conflicts: # fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

…u/incubator-doris into fe-time-zone-function

…to fe-time-zone-function Conflicts: fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

morningman · 2019-08-20T10:46:41Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

    }

    @Override
    public boolean isMinValue() {
        switch (type.getPrimitiveType()) {
            case DATE:
-                return this.date.compareTo(TimeUtils.MIN_DATE) == 0;
+                return this.getStringValue().compareTo("1900-01-01") == 0;


I think min value and max value can be a static final member of DateLiteral？So that you can just use "=" to check this

We can create a static for min/max value. But we can't not use == to check if it is a minimal value. Because user can construct another object with minimal value string

ok

We can create a static for min/max value. But we can't not use == to check if it is a minimal value. Because user can construct another object with minimal value string

ok

morningman · 2019-08-20T10:51:36Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

+                .toString(FormatBuilder(pattern).toFormatter());
+    }
+
+    private static DateTimeFormatterBuilder FormatBuilder(String pattern) throws AnalysisException{


Suggested change

private static DateTimeFormatterBuilder FormatBuilder(String pattern) throws AnalysisException{

private static DateTimeFormatterBuilder formatBuilder(String pattern) throws AnalysisException{

morningman · 2019-08-20T10:52:34Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

+        return hour;
+    }
+
+    public void setHour(long hour) {


Do we really need these set or get functions? If not, it is better to remove them.

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

imay · 2019-08-20T12:13:36Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

-        date = new Date(in.readLong());
+        if (Catalog.getCurrentCatalogJournalVersion() >= FeMetaVersion.VERSION_59) {
+            long packed_time = in.readLong();
+            microsecond = (packed_time % (1L << 24));


make it a function

imay · 2019-08-20T12:15:14Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

+            second = hms % (1 << 6);
+            minute = (hms >> 6) % (1 << 6);
+            hour = (hms >> 12);
+            this.type = Type.DATETIME;


why DATETIME? I think it may be DATE

The dates read from the meta are all datetime types, and Doris explicitly specifies type DATE/DATETIME by setType()

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

imay · 2019-08-20T12:25:04Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

+        return dateLiteral;
+    }
+
+    public String dateFormat(String pattern) throws AnalysisException{


You should add comments for this function. It's better to give some example that what function do

imay · 2019-08-20T12:28:03Z

fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

+        DateLiteral dateLiteral = (DateLiteral) date;
+
+        DateLiteral result = new DateLiteral(dateLiteral);
+        result.setDay(dateLiteral.getDay() + day.getLongValue());


how about "2010-01-31" + 10

…tion # Conflicts: # fe/src/main/java/org/apache/doris/common/FeMetaVersion.java # fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

…u/incubator-doris into fe-time-zone-function

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

morningman · 2019-08-21T12:39:18Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

    }

    @Override
    public void readFields(DataInput in) throws IOException {
        super.readFields(in);
-        date = new Date(in.readLong());
+        if (Catalog.getCurrentCatalogJournalVersion() >= FeMetaVersion.VERSION_59) {


VERSION_59 is used by support strict mode, change to VERSION_60

imay · 2019-08-22T06:21:00Z

fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

+            return new DateLiteral(dateTime.getYear(), dateTime.getMonthOfYear(), dateTime.getDayOfMonth());
+        } else {
+            return new DateLiteral(dateTime.getYear(), dateTime.getMonthOfYear(), dateTime.getDayOfMonth(),
+                    dateTime.getHourOfDay(), dateTime.getMinuteOfHour(), dateTime.getSecondOfMinute());


Why not move this logic to DateLiteral. And I think we can make formatter static which is thread-safe.

imay · 2019-08-22T06:24:44Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java

-        date = new Date(in.readLong());
+        if (Catalog.getCurrentCatalogJournalVersion() >= FeMetaVersion.VERSION_60) {
+            fromPackedDatetime(in.readLong());
+        } else {


I think we should persist type for later use. Or if we want to persist Date, we will have to change the format we use.

…to fe-time-zone-function

imay · 2019-08-26T15:59:49Z

fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

@@ -56,8 +49,13 @@
     */
    @FEFunction(name = "timediff", argTypes = { "DATETIME", "DATETIME" }, returnType = "TIME")
    public static FloatLiteral timeDiff(LiteralExpr first, LiteralExpr second) throws AnalysisException {
-        long timediff = (getTime(first) - getTime(second)) / 1000;
-        return new FloatLiteral((double)timediff, Type.TIME);
+        SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");


why "yyyy-MM-dd" other than "yyyy-MM-dd hh:mm:ss"

I think we can write a function getTime() for DateLiteral.

imay · 2019-08-26T16:06:01Z

fe/src/main/java/org/apache/doris/analysis/DateLiteral.java


 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;


And I see you use SimpleDateFormat and DateTimeFormatter.
Can we unify them to DateTimeFormatter?

imay · 2019-08-26T16:10:20Z

fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

            throw new AnalysisException("unixtime should larger than zero");
        }
-        Date date = new Date(unixTime.getLongValue() * 1000);
-        return new StringLiteral(dateFormat(date, "%Y-%m-%d %H:%i:%S"));
+        SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");


I think we can create a constructor of DateLiteral with unix timestamp and timezone.
Then this logic can be included in DateLiteral

imay

LGTM

morningman

LGTM

… missing out file (apache#1644)

HangyuanLiu added 7 commits August 13, 2019 20:24

add fe time zone

a60451c

add fe time zone

701d448

add

194f5e9

Merge remote-tracking branch 'upstream/master' into fe-time-zone-func…

37d7dfa

…tion

Merge branch 'fe-time-zone-function' of https://github.com/HangyuanLi…

5d85ff4

…u/incubator-doris into fe-time-zone-function

add fe time zone

b575d4c

add fe time zone

98ab140

HangyuanLiu closed this Aug 19, 2019

HangyuanLiu added 11 commits August 19, 2019 14:26

add fe time zone

40c0dfe

Merge branch 'fe-time-zone-function' of https://github.com/HangyuanLi…

e5ac2fe

…u/incubator-doris into fe-time-zone-function # Conflicts: # fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

Merge branch 'fe-time-zone-function' of https://github.com/HangyuanLi…

01c8557

…u/incubator-doris into fe-time-zone-function # Conflicts: # fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

add

58e856b

add

00f523d

add

3a376c0

add

4c486b9

add

6f12379

add

e4917d0

Merge branch 'fe-time-zone-function' of https://github.com/HangyuanLi…

740b0bc

…u/incubator-doris into fe-time-zone-function

refactor date literal

05335df

HangyuanLiu reopened this Aug 19, 2019

HangyuanLiu changed the title ~~Fe time zone function~~ Refactor DateLiteral class in FE Aug 19, 2019

Merge branch 'master' of https://github.com/apache/incubator-doris in…

98d6668

…to fe-time-zone-function Conflicts: fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

HangyuanLiu force-pushed the fe-time-zone-function branch from 3438f2f to 98d6668 Compare August 19, 2019 13:48

morningman requested changes Aug 20, 2019

View reviewed changes

imay requested changes Aug 20, 2019

View reviewed changes

HangyuanLiu added 4 commits August 21, 2019 13:31

add

c587d55

Merge remote-tracking branch 'upstream/master' into fe-time-zone-func…

23e4fc1

…tion # Conflicts: # fe/src/main/java/org/apache/doris/common/FeMetaVersion.java # fe/src/main/java/org/apache/doris/rewrite/FEFunctions.java

add

6db4c68

Merge branch 'fe-time-zone-function' of https://github.com/HangyuanLi…

a4427f7

…u/incubator-doris into fe-time-zone-function

HangyuanLiu force-pushed the fe-time-zone-function branch from 12e3678 to a4427f7 Compare August 21, 2019 06:36

morningman requested changes Aug 21, 2019

View reviewed changes

HangyuanLiu force-pushed the fe-time-zone-function branch from 298dd35 to 58093b8 Compare August 21, 2019 14:27

imay reviewed Aug 22, 2019

View reviewed changes

function name to camel

266e74b

HangyuanLiu force-pushed the fe-time-zone-function branch from 58093b8 to 266e74b Compare August 25, 2019 03:18

Merge branch 'master' of https://github.com/apache/incubator-doris in…

a3f5672

…to fe-time-zone-function

HangyuanLiu force-pushed the fe-time-zone-function branch 2 times, most recently from 9ac46ab to 63071d4 Compare August 26, 2019 14:28

imay reviewed Aug 26, 2019

View reviewed changes

HangyuanLiu force-pushed the fe-time-zone-function branch from 63071d4 to 08bf2ec Compare August 27, 2019 04:14

add master forward time zone support

2412538

HangyuanLiu force-pushed the fe-time-zone-function branch from 08bf2ec to 2412538 Compare August 27, 2019 04:59

imay approved these changes Aug 27, 2019

View reviewed changes

morningman approved these changes Aug 27, 2019

View reviewed changes

morningman merged commit 0c2e344 into apache:master Aug 27, 2019

imay mentioned this pull request Sep 26, 2019

Release Notes 0.11.0 #1891

Closed

HangyuanLiu deleted the fe-time-zone-function branch May 25, 2020 01:56

SWJTU-ZhangLei added a commit to SWJTU-ZhangLei/incubator-doris that referenced this pull request Jul 25, 2023

[feature](selectdb-cloud) Fix test_schema_change_with_readd_cluster…

737fad2

… missing out file (apache#1644)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor DateLiteral class in FE #1644

Refactor DateLiteral class in FE #1644

HangyuanLiu commented Aug 15, 2019 •

edited

Loading

morningman Aug 20, 2019

imay Aug 20, 2019

morningman Aug 20, 2019

morningman Aug 20, 2019

morningman Aug 20, 2019

HangyuanLiu Aug 20, 2019

imay Aug 20, 2019

imay Aug 20, 2019

HangyuanLiu Aug 21, 2019

imay Aug 20, 2019

imay Aug 20, 2019

morningman Aug 21, 2019

imay Aug 22, 2019

imay Aug 22, 2019

imay Aug 26, 2019

imay Aug 26, 2019

imay Aug 26, 2019

imay left a comment

morningman left a comment

	private static DateTimeFormatterBuilder FormatBuilder(String pattern) throws AnalysisException{
	private static DateTimeFormatterBuilder formatBuilder(String pattern) throws AnalysisException{

Refactor DateLiteral class in FE #1644

Refactor DateLiteral class in FE #1644

Conversation

HangyuanLiu commented Aug 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imay left a comment

Choose a reason for hiding this comment

morningman left a comment

Choose a reason for hiding this comment

HangyuanLiu commented Aug 15, 2019 •

edited

Loading