Skip to content

Handle Date Underflow #612

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Handle Date Underflow #612

wants to merge 2 commits into from

Conversation

kamuli4
Copy link

@kamuli4 kamuli4 commented Feb 17, 2025

Attempting to resolve #595

Takes in a date (MM-DD-YYYY) and formats it to ISO 8601 (YYYY-MM-DD), then compares if it is below the lowest PostgreSQL date (4713 BC or -4712-01-01 in ISO 8601). Truncates the entered date to the lowest Postgres if it is further in the past than Postgres allows. Similar logic is applied if the date is further in the future than Postgres allows. Returns the user-entered date in ISO 8601 format if it is a valid date

@JelteF
Copy link
Collaborator

JelteF commented Feb 17, 2025

I'm a bit confused. How is this supposed to work? These functions are not being called afaict.

@kamuli4
Copy link
Author

kamuli4 commented Feb 19, 2025 •
edited
Loading

I should've asked in the original issue, but I did a quick look and didn't see any files that looked like they control date formatting and verification, could you point me to the file(s) that handle that feature? Would it be the original file referenced in #595 or src/pgduckdb_types by any chance?

@JelteF
Copy link
Collaborator

JelteF commented Feb 20, 2025 •
edited
Loading

I updated the original issue to include this query that triggers these underflowing dates:

SELECT * FROM duckdb.query($$
    select
        '5877642-06-25 (BC)'::date as date,
        '290309-12-22 (BC) 00:00:00'::timestamp as timestamp,
        '290309-12-22 (BC) 00:00:00'::timestamp_s as timestamp_s,
        '290309-12-22 (BC) 00:00:00'::timestamp_ms as timestamp_ms,
        '290309-12-22 (BC) 00:17:30+00:17'::timestamptz as timestamptz,
$$);
     date      │          timestamp           │         timestamp_s          │         timestamp_ms         │           timestamptz
───────────────┼──────────────────────────────┼──────────────────────────────┼──────────────────────────────┼─────────────────────────────────
 5881580-07-14 │ 294247-01-10 08:01:49.551616 │ 294247-01-10 08:01:49.551616 │ 294247-01-10 08:01:49.551616 │ 294247-01-10 09:02:19.551616+01
(1 row)

And yes this conversion between PG and DuckDB date format is being done in the pgduckdb_types.cpp file.

@JelteF
Copy link
Collaborator

JelteF commented Feb 20, 2025

The conversion is done in these functions to be specific:

pg_duckdb/src/pgduckdb_types.cpp

Lines 215 to 255 in 82d3a0f

inline Datum
ConvertDateDatum(const duckdb::Value &value) {
duckdb::date_t date = value.GetValue<duckdb::date_t>();
return date.days - pgduckdb::PGDUCKDB_DUCK_DATE_OFFSET;
}
static Datum
ConvertIntervalDatum(const duckdb::Value &value) {
duckdb::interval_t duckdb_interval = value.GetValue<duckdb::interval_t>();
Interval *pg_interval = static_cast<Interval *>(palloc(sizeof(Interval)));
pg_interval->month = duckdb_interval.months;
pg_interval->day = duckdb_interval.days;
pg_interval->time = duckdb_interval.micros;
return IntervalPGetDatum(pg_interval);
}
inline Datum
ConvertTimestampDatum(const duckdb::Value &value) {
// Extract raw int64_t value of timestamp
int64_t rawValue = value.GetValue<int64_t>();
// Handle specific Timestamp unit(sec, ms, ns) types
switch (value.type().id()) {
case duckdb::LogicalType::TIMESTAMP_MS:
// 1 ms = 10^3 micro-sec
rawValue *= 1000;
break;
case duckdb::LogicalType::TIMESTAMP_NS:
// 1 ns = 10^-3 micro-sec
rawValue /= 1000;
break;
case duckdb::LogicalType::TIMESTAMP_S:
// 1 s = 10^6 micro-sec
rawValue *= 1000000;
break;
default:
// Since we don't want to handle anything here
break;
}
return rawValue - pgduckdb::PGDUCKDB_DUCK_TIMESTAMP_OFFSET;
}

@JelteF
Copy link
Collaborator

JelteF commented Mar 10, 2025

Closing this in favor of #653

@JelteF JelteF closed this Mar 10, 2025
@shah-nirmit shah-nirmit mentioned this pull request Mar 11, 2025
Merged
JelteF pushed a commit that referenced this pull request Mar 18, 2025
@shah-nirmit
Add boundary check for converting duckdb date/timestamps to PG dates/…
ebef8c8 
…timestamps (#653)

Postgres and DuckDB support different date and timestamp ranges. This
starts to error out when either one receives a value outside of this
range. This also starts converting special values for the `infinity` and
`-infininity` correctly, because these are also not the same between the
two databases.

Fixes #595
Fixes #612
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handle dates that underflow differently
2 participants
@kamuli4 @JelteF