Skip to content

Conversation

@dekimir
Copy link
Owner

@dekimir dekimir commented Jul 22, 2025

Copy of trinodb#26259

skyglass added 3 commits July 22, 2025 12:03
- implemented additional timestamp with precision data type for ExasolClient
- added correspondent tests for timestamp with precision data type
- implemented additional timestamp with local time zone data type for ExasolClient
- implemented special addRoundTrip method with additional column expression parameter for the predicate to fix the test predicate assumption
- all timestamp with timezone tests use input literal timestamp string, which is interpreted as a JVM timestamp string ("America/Bahia_Banderas") but expected literal strings are interpreted as UTC strings. Therefore the difference between these timestamps is 6 hours (with DST) or 5 (without DST); for some historical values, like 1970 for example, the difference can be 7, which is expected behaviour for timestamp with local time zone
- removed mapper classes and moved the logic to ExasolClient, based on the PR suggesions
- removed unnecessary javadocs
- improved javadocs and comments for testing timestamp with time zone by explaining why only jvm time zone is currently used for testing timestamp with time zone
- removed using * import in ExasolClient based on the PR suggestions
- removed TestExasolTimestampMapping and moved testing timestamp and timestamp with timezone to TestExasolTypeMapping, based on the PR suggestions
- defined test methods as package-private, based on the PR suggestions
@dekimir
Copy link
Owner Author

dekimir commented Jul 22, 2025

/suggest-unit-tests

@github-actions
Copy link

Based on the changes you've made, you have added valuable unit tests for timestamp and timestamp with time zone handling, which is great. However, there are a few more areas where additional tests could provide further coverage:

  1. Error Handling Tests:

    • Test cases for edge scenarios such as invalid timestamp formats or out-of-range values.
    • Verify that the client correctly handles SQL exceptions and translates them into appropriate Trino exceptions.
  2. Boundary Value Tests:

    • Test the minimum and maximum values that Exasol can handle for timestamps.
    • Ensure that values close to these boundaries are correctly converted and stored.
  3. Time Zone Conversion Tests:

    • Add more comprehensive tests for time zone conversions, especially for different session time zones.
    • Test how the client handles daylight saving time (DST) transitions.
  4. Precision Handling Tests:

    • Test various precisions for timestamps to ensure that the client can handle different levels of precision correctly.
    • Verify that the client can handle both short and long precision timestamps as expected.
  5. Null Value Tests:

    • Ensure that null values are correctly handled during read and write operations for timestamps.
  6. Performance Tests:

    • Although not strictly unit tests, consider adding performance benchmarks to ensure that the new timestamp handling does not introduce significant overhead.

Here’s an example of additional tests you might consider adding:

@Test
void testTimestampBoundaryValues() {
    testTimestamp("TIMESTAMP '0001-01-01 00:00:00.000000'");
    testTimestamp("TIMESTAMP '9999-12-31 23:59:59.999999'");
}

@Test
void testTimestampWithTimeZoneBoundaryValues() {
    testTimestampWithTimeZone("TIMESTAMP '0001-01-01 00:00:00.000000'");
    testTimestampWithTimeZone("TIMESTAMP '9999-12-31 23:59:59.999999'");
}

@Test
void testNullTimestamp() {
    Session session = Session.builder(getSession()).build();
    SqlDataTypeTest.create()
        .addRoundTrip("timestamp", "NULL", createTimestampType(3), "CAST(NULL AS TIMESTAMP)")
        .execute(getQueryRunner(), session, exasolCreateAndInsert(TEST_SCHEMA + "." + "test_null_timestamp"));
}

@Test
void testNullTimestampWithTimeZone() {
    Session session = Session.builder(getSession()).build();
    SqlDataTypeTest.create()
        .addRoundTrip("timestamp with time zone", "NULL", createTimestampWithTimeZoneType(3), "CAST(NULL AS TIMESTAMP WITH TIME ZONE)")
        .execute(getQueryRunner(), session, exasolCreateAndInsert(TEST_SCHEMA + "." + "test_null_timestamp_with_timezone"));
}

These additional tests will help ensure that your changes are robust and handle a wide range of scenarios.

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