Refactoring of time handling objects by elliVM · Pull Request #572 · teragrep/pth_10

elliVM · 2025-04-23T07:38:18Z

Refactor the responsibilities of time handling objects

Parsing of incoming pure string values

Introduces new way of parsing and handling timestamps.
Incoming pure string values of timestamp and time format are handled by the DPLTimestampString object that accepts the value, accepted parsing formats and the base time, user can provide own timeformat in other case a set list of default time formats is used. Parsing result is accepted only if a single value is matched against the provided formats. DPLTimestampString has asDPLTimestamp method that produces a DPLTimestamp interface that represents the parsed timestamp, if no acceptable parsing result was found the result will be a stub object.

DPL timestamp interface to represent a parsed timestamp

DPLTimestamp interface has a method to produces a Java's ZonedDateTime object for the timestamp that can be used to calculate epoch seconds and various other time values for the timestamp

`DPLTimestamp` implementing classes

AbsoluteTimestamp is responsible for handling dates eg. 2020-01-01.

RelativeTimestamp calculates relative values e.g. -1day that are calculated from the provided base time.

Test cases are updated to use UTC without reliance on system default and changing of JVM default timezone.

kortemik · 2025-04-23T08:55:01Z

this alters the jvm state as setDefault for TimeZone will change the state globally and modifying global state in a test case is a no-go. please see https://docs.oracle.com/javase/8/docs/api/java/util/TimeZone.html#getDefault--

eemhu · 2025-04-23T09:08:07Z

I wonder if it would be better to add an additional ctor to InstantTimestamp that allows defining the timezone. This ctor could be used in tests. The default ctor would use the system timezone.

elliVM · 2025-04-24T09:49:50Z

Will refactor to have a constructor with a timezone and otherwise use system default timezone

eemhu

Changes look good to me, but should the DefaultTimeFormatTest and DPLTimeFormatTest also be changed to use the new ctor?

elliVM · 2025-04-25T06:10:26Z

yea I will update the other tests also

elliVM · 2025-04-25T06:51:48Z

Made #573 for other test cases still using TimeZone.setDefault("Europe/Helsinki")

eemhu

builds ok, LGTM

elliVM · 2025-04-29T09:24:30Z

Noted that If the value for example 2024-10-31T10:10:10Z contains zone information like Z zulu time (UTC) it will take precedence over SimpleDateFormat.setTimezone when calling SimpleDateFormat.parse(time). Meaning that the resulting instant is always for UTC and the method ZoneId value is ignored. This happens if DefaultTimeFormat is used when time format is empty or null.

Meaning that if timestamp has valid zone info it will overwrite timezone set by setTimezone method

elliVM · 2025-05-02T09:08:09Z

Some DPL tests have time value with lower case z at the end, e.g. earliest=2020-12-12T00:00:00z. I suspect it was meant to set a timezone like UTC? This was ignored by the old SimpleDateFormat when parsing the value. In the documentation z was used only in the format as a "general time zone" but in a parsed value it has no function.
Replaced the legacy SimpleDateFormat with a DateTimeFormatter, this means that the z input now throws an exception instead of being ignored.

This happens only if the default absolute timestamp formats are used to parse the value.

Fix?

Change lowercase z to Z in java and this would apply a UTC to these values and ignore the system default. z would be an alias to Z (non ISO 8601).
Change test cases to use Z instead of z , only accept Zin the query and have z throw an exception (ISO 8601).
Enforce the zone from the class member field (System default timezone outside of test cases). This would remove the ability to set time zones in the values when no time format is given.
Treat all values as uppercased

elliVM · 2025-05-02T12:18:53Z

rebased

elliVM · 2025-05-12T08:06:12Z

Refactoring time relative objects

elliVM · 2025-05-16T10:40:09Z

rebased

elliVM · 2025-05-19T06:22:31Z

Refactored relative time objects

eemhu · 2025-09-19T12:00:10Z

+    private long offsetAmount() {
+        final String validAmountString = new ValidOffsetAmountText(offsetText).read();
+        return Long.parseLong(validAmountString);
+    }
+
+    private OffsetUnit offsetUnit() {
+        final String validUnitString = new ValidOffsetUnitText(offsetText).read();


maybe these two should be their own objects?

eemhu · 2025-09-19T12:31:16Z

                            List<String> expectedResults = Collections
                                    .singletonList(
-                                            "978303661" // +0300 timezone
+                                            "978310861" // +0300 timezone


expected result changed?

Tests default timezone changed to UTC I updated the comment

eemhu · 2025-09-19T12:32:11Z

+    }
+
+    @Test
+    public void testUserDefinedFormat() {


test invalid user defined format also

added invalid input test

eemhu · 2025-09-19T12:38:24Z

            // Assert equals with expected
            Assertions.assertEquals(1644487237L, lstA.get(0).getLong(0));
-            Assertions.assertEquals(1645048800L, lstB.get(0).getLong(0));
+            Assertions.assertEquals(1645056000L, lstB.get(0).getLong(0));


expected changed?

eemhu · 2025-09-19T12:41:04Z

-            Instant now = timestamp.toInstant().plus(-3, ChronoUnit.HOURS);
-
-            String expected = String.valueOf(now.getEpochSecond()).substring(0, 7); // don't check the seconds within a minute, as the query takes some time and might be a few seconds off
+            long latestEpoch = ZonedDateTime.now(utcZone).minusHours(3).toEpochSecond();


should be able to define startTime now so no substring trickery needed

Cleaned up the relativeTimeTest all tests now directly assert the expected epochs against the actualy epochs in the query

elliVM · 2025-09-29T07:40:23Z

Rebased

… missing package renaming

…d of find() to force matching from start of string

…rt time to dpl context used by relative timestamps

…mes and add equals + hashcode + contract tests where missing

…rserConfig, update tests to set query start time before earliest

…is no user defined provided + cleanup and fixes from review

…e assertions to DPLTimeFormatString and to DPLFormat implementing classes.

…use system default timezone, add zone id to Relative_time UDF

…nused object DPLTimeFormat and test

… now. update time stamps tests to use UTC and set query start time

elliVM self-assigned this Apr 23, 2025

elliVM requested a review from eemhu April 23, 2025 07:39

eemhu reviewed Apr 25, 2025

View reviewed changes

elliVM commented Apr 25, 2025

View reviewed changes

Comment thread src/test/java/com/teragrep/pth10/DefaultTimeFormatTest.java

elliVM commented Apr 25, 2025

View reviewed changes

Comment thread src/test/java/com/teragrep/pth10/DefaultTimeFormatTest.java

eemhu previously approved these changes Apr 25, 2025

View reviewed changes

elliVM added the review label Apr 25, 2025

kortemik requested changes Apr 28, 2025

View reviewed changes

Comment thread src/main/java/com/teragrep/pth10/ast/time/InstantTimestamp.java Outdated

elliVM changed the title ~~InstantTimestampTest test timezone fix~~ Refactoring of time handling objects May 2, 2025

elliVM marked this pull request as draft May 2, 2025 09:40

elliVM removed the review label May 2, 2025

elliVM force-pushed the instant-timestamp-test-fix branch from 743fff7 to cbd07aa Compare May 2, 2025 12:18

kortemik reviewed May 13, 2025

View reviewed changes

Comment thread src/main/java/com/teragrep/pth10/ast/commands/transformstatement/convert/Ctime.java

elliVM force-pushed the instant-timestamp-test-fix branch from a2a1884 to 7c00fc5 Compare May 16, 2025 10:37

elliVM marked this pull request as ready for review May 19, 2025 06:32

eemhu self-requested a review May 19, 2025 06:47

This was referenced May 19, 2025

decrease earliest epoch value of query xml by 3 hours query XML #415

Open

Issue_458: Improved error message for incorrect timestamp format #519

Open

eemhu reviewed May 21, 2025

View reviewed changes

Comment thread src/main/java/com/teragrep/pth10/ast/time/DefaultFormatAbsoluteTimestamp.java Outdated

eemhu reviewed Sep 19, 2025

View reviewed changes

elliVM force-pushed the instant-timestamp-test-fix branch 2 times, most recently from 8857d24 to 9764a82 Compare September 29, 2025 07:37

elliVM added 20 commits October 7, 2025 09:32

rebase

d623c95

apply spotless and remove empty files after rebase squash, fix rebase…

7a62fec

… missing package renaming

test reads of DPLTimeFormatTextTest, simplify code and fix comments

5455510

make ValidOffsetAmountText main constructor private

e87bbce

remove digit check boolean and use just regex, use lookingAt() instea…

047a428

…d of find() to force matching from start of string

make DPLTimestampImpl main ctor private, rename mktime shorthand values

20ed971

implement DPLTimeFormat interface as a parsing pattern. add query sta…

4358717

…rt time to dpl context used by relative timestamps

rename CustomTimeFormat to UserDefinedTimeFormat. improve variable na…

ae18cac

…mes and add equals + hashcode + contract tests where missing

do not set query start time if not set when setting earliest in DPLPa…

a4edc12

…rserConfig, update tests to set query start time before earliest

add start time to DPLParserCatalystContext ctor, update tests

7b2432b

remove support for bot user defined and default formats, use default …

15c2f97

…is no user defined provided + cleanup and fixes from review

remove test cases reliance on JVM timezone replace with UTC, add valu…

e133576

…e assertions to DPLTimeFormatString and to DPLFormat implementing classes.

update tests and some test data to use UTC, remove constructors that …

ce7f36c

…use system default timezone, add zone id to Relative_time UDF

improve logging levels and messages + add missing final keyword

87c9db7

update expected exception message in DPLTimestampStringTest, remove u…

5b4acc9

…nused object DPLTimeFormat and test

remove unused object DefaultFormatAbsoluteTimestamp and test

61e5806

update comment

5d05c86

test invalid user timeformat in DPLTimestampStringTest

43df277

fix timestamps to use start time instead of creating new when calling…

f563f1b

… now. update time stamps tests to use UTC and set query start time

update pth_15 to 3.0.0

a0d9d47

elliVM force-pushed the instant-timestamp-test-fix branch from 9764a82 to a0d9d47 Compare October 7, 2025 06:33

elliVM dismissed eemhu’s stale review via a0d9d47 February 28, 2026 20:37

Conversation

elliVM commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refactor the responsibilities of time handling objects

Parsing of incoming pure string values

DPL timestamp interface to represent a parsed timestamp

DPLTimestamp implementing classes

Uh oh!

kortemik commented Apr 23, 2025

Uh oh!

eemhu commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elliVM commented Apr 24, 2025

Uh oh!

eemhu left a comment

Choose a reason for hiding this comment

Uh oh!

elliVM commented Apr 25, 2025

Uh oh!

Uh oh!

Uh oh!

elliVM commented Apr 25, 2025

Uh oh!

eemhu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elliVM commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elliVM commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elliVM commented May 2, 2025

Uh oh!

elliVM commented May 12, 2025

Uh oh!

Uh oh!

elliVM commented May 16, 2025

Uh oh!

elliVM commented May 19, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elliVM commented Sep 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

elliVM commented Apr 23, 2025 •

edited

Loading

`DPLTimestamp` implementing classes

eemhu commented Apr 23, 2025 •

edited

Loading

elliVM commented Apr 29, 2025 •

edited

Loading

elliVM commented May 2, 2025 •

edited

Loading