Empty postgres ranges don't seem supported

mpflanzer commented 1 month ago

Setup

Versions

Rust:
Diesel: master
Database: postgres
Operating System: Arch Linux

Feature Flags

diesel: ["postgres", "chrono", "serde_json"]

Problem Description

The FromSql<Range<ST>, Pg> for (Bound<T>, Bound<T>) trait doesn't seem to support empty ranges: https://github.com/diesel-rs/diesel/blob/381be195688db339fe2927e49bc818ab86754dd9/diesel/src/pg/types/ranges.rs#L79

What are you trying to accomplish?

Reading rows containing empty ranges from a postgresql database

What is the expected output?

Not sure, there is no obvious candidate to represent an empty range as (Bound, Bound).

What is the actual output?

get_results fails with the following error when rows contain an empty value as range

Error deserializing field 'interval': failed to fill whole buffer

Are you seeing any additional errors?

Steps to reproduce

CREATE TABLE "records"(
  "id" BIGSERIAL NOT NULL PRIMARY KEY,
  "interval" TSRANGE NOT NULL DEFAULT 'empty'::tsrange,
);

diesel::table! {
    records (id) {
        id -> Int8,
        interval -> Tsrange,
    }
}

    pub type Interval = (Bound<NaiveDateTime>, Bound<NaiveDateTime>);

    #[derive(Queryable, Selectable, Identifiable, Associations, Clone, Debug)]
    #[diesel(table_name = schema::records)]
    #[diesel(check_for_backend(diesel::pg::Pg))]
    pub struct Record {
        pub id: i64,
        pub interval: Interval,
    }

            let results = schema::records::table
                .select(Record::as_select())
                .get_results(conn)?;

Checklist

[x] I have already looked over the issue tracker and the discussion forum for similar possible closed issues.
[x] This issue can be reproduced on Rust's stable channel. (Your issue will be closed if this is not the case)
[x] This issue can be reproduced without requiring a third party crate

weiznich commented 1 month ago

Thanks for reporting. That's definitively a bug.

I think we should be able to fix that by just checking here (https://github.com/diesel-rs/diesel/blob/381be195688db339fe2927e49bc818ab86754dd9/diesel/src/pg/types/ranges.rs#L83) if flags contains RangeFlags::Empty and if that's the case return an artifical empty range (e.g. (Bound::Excluded(0), Bound::Excluded(0))) there.

I would be happy to accept a PR for that if it also includes a simple test here: https://github.com/diesel-rs/diesel/blob/381be195688db339fe2927e49bc818ab86754dd9/diesel_tests/tests/types.rs#L1405

mpflanzer commented 1 month ago

Ok, I'll have a go at it. I just wasn't sure how you'd like to represent the empty range.

mpflanzer commented 1 month ago

I created a MR but it's not working yet: https://github.com/diesel-rs/diesel/pull/4295

My idea was to use Default::default to get the dummy value for the Bound::Excluded. But the types in the time crate don't implement Default. For the chrono time types and the numeric types it should work.

mpflanzer commented 1 month ago

The PR is now in a working state. A Defaultable trait is now implemented for all the supported postgres range types.

guissalustiano commented 3 weeks ago

Resolved by https://github.com/diesel-rs/diesel/pull/4295

diesel-rs / diesel