vapor / postgres-nio

🐘 Non-blocking, event-driven Swift client for PostgreSQL.
https://api.vapor.codes/postgresnio/documentation/postgresnio/
MIT License
304 stars 70 forks source link

TIMESTAMP columns don't return the same value as was inserted #453

Closed adam-fowler closed 4 months ago

adam-fowler commented 4 months ago

Describe the bug

TIMESTAMP WITHOUT TIME ZONE columns don't return the same value as was inserted

To Reproduce

_ = try await client.withConnection { connection in
    try await connection.query(
        """
        CREATE TABLE IF NOT EXISTS timestampTests (
            "id" uuid PRIMARY KEY,
            "expires" timestamp
        )
        """,
        logger: logger
    )
    let id = UUID()
    let date = Date.now
    try await connection.query(
        "INSERT INTO timestampTests (id, expires) VALUES (\(id), \(date))",
        logger: logger
    )
    let stream = try await connection.query(
        "SELECT expires FROM timestampTests WHERE id = \(id)",
        logger: logger
    )
    let result = try await stream.decode(Date.self).first { _ in true }
    XCTAssertEqual(result, date)
}

Expected behavior

Date inserted is the same as date extracted regardless of whether WITH TIME ZONE is included in column definition.

Environment

PostgresNIO: 1.20.2 swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5) Target: arm64-apple-macosx14.0

Other context

This is on a Mac set to GMT which is weird, I might expect this when the timezone is not UTC+0.

gwynne commented 4 months ago

This is expected behavior, due to the interaction between Date's assumptions about its input and the stated behavior of Postgres time stamp columns, per section 8.5.1.3 of the Postgres manual:

When a timestamp with time zone value is output, it is always converted from UTC to the current timezone zone, and displayed as local time in that zone.

(I recognize this statement is about timestamps with time zone - it is this behavior that the conversion logic used for Date is (somewhat unintentionally) exploiting.)

adam-fowler commented 4 months ago

The bug is then if you set your column to just TIMESTAMP you have a discrepancy between the type the database is storing and the type that PostgresNIO is writing so you get an unwanted offset applied.

I found if I change Date.psqlType to .timestamp no offset is applied. But TIMESTAMP WITH TIME ZONE values are probably more useful so it should probably stay as is.

gwynne commented 4 months ago

I found if I change Date.psqlType to .timestamp no offset is applied. But TIMESTAMP WITH TIME ZONE values are probably more useful so it should probably stay as is.

Ah, yeah, that'll do it. Yet another case of "yeah maybe the design that doesn't acknowledge that there's often more than one way to specify any given type isn't so great". In any event, even if changing it was a functional positive in this case, we couldn't do so anyway for compatibility reasons. (Always fun, those...)