Skip to content
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

Support inserting PG::Interval instances #272

Open
Blacksmoke16 opened this issue Sep 11, 2023 · 3 comments · May be fixed by #283
Open

Support inserting PG::Interval instances #272

Blacksmoke16 opened this issue Sep 11, 2023 · 3 comments · May be fixed by #283

Comments

@Blacksmoke16
Copy link

Blacksmoke16 commented Sep 11, 2023 •
edited

I ran into an interesting case where in order to insert data in a prepared statement, you need to use Time::Span, but in order to read it out, as noted in the README, you need to use PG::Interval.

Is there a reason PG::Interval shouldn't delegate to #to_span to make it compatible in both directions?

CREATE TABLE interval_testing
(
    id       BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
    duration INTERVAL NOT NULL
);
require "pg"

struct Testing
  include DB::Serializable

  getter id : Int64
  getter duration : PG::Interval
end

DB.open "postgres://administrator:4T5VohI8Zl6IySNemf@192.168.100.201:5432/reporting_warehouse" do |db|
  # Unhandled exception: invalid input syntax for type interval: "PG::Interval(@microseconds=0, @days=1, @months=0)" (PQ::PQError)
  db.exec "INSERT INTO reporting.interval_testing (duration) VALUES ($1);", PG::Interval.new days: 1

  pp db.query_one "SELECT * FROM reporting.interval_testing WHERE id = 1;", as: Testing
end
@will
Copy link
Owner

will commented Sep 22, 2023

Is there a reason PG::Interval shouldn't delegate to #to_span to make it compatible in both directions?

I've been thinking about this for the last little bit, and can't come up with any reasons not to. Do you want to send in a PR fix it?

The only general thing to be aware of with postgres intervals it that it stores hours differently from days, and doesn't normalize by default. So you can have 2 days 31 hours, and 3 days 7 hours and it'll keep track that precision. But it'll know they're equal.

select a, b, a=b from (select '2 days 31:00:00'::interval a, '3 days 7 hours'::interval b);
        a        |        b        | ?column?
-----------------+-----------------+----------
 2 days 31:00:00 | 3 days 07:00:00 | t

however for this issue, I don't think that trivia is relevant? But just wanted to bring it up in case.

@jwoertink
Copy link
Contributor

Does this matter on the crystal side that something like 1.year is not a Time::Span?

@jwoertink jwoertink mentioned this issue Sep 22, 2023
Open
@bcardiff
Copy link
Collaborator

I don't see a problem in allowing PG::Interval on prepared statements.

Whether that's by calling to_span or directly encoding the value is an implementation detail.

I think is valuable to have the PG types as the safe options to handle the precision supported by the db, despite how a std-lib type might seem a 99% replacement.

@Blacksmoke16 Blacksmoke16 linked a pull request Mar 8, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support encoding PG::Interval params Blacksmoke16/crystal-pg
4 participants
@will @jwoertink @bcardiff @Blacksmoke16