[ruby-dbi-users] Proposing patch for sqlite3 driver

erik at hollensbe.org erik at hollensbe.org
Wed Oct 1 16:59:26 EDT 2008



On Wed, 01 Oct 2008 13:55:11 +0200, Jarl Friis <jarl at gavia.dk> wrote:
> Erik Hollensbe <erik at hollensbe.org> writes:
> 
>> The problem with this is that while postgres's date format is variable,
> the
>> parsing routes we use with DateTime are not. A smarter solution needs to
> be
>> used, as a "strptime until it doesn't break" solution will likely cause
> more
>> problems than it resolves.
> 
> I am not sure on what you mean by "strptime until it doesn't break",
> and I wonder what problems you have in mind when you say "will likely
> cause more problems than it resolves."

I'll explain this below.
 
> Anyway I think that my proposal is more robust than
> current implementation, because:
> 1) most installations/configurations will accept ISO dates.
> 2) dates formated in ISO8601 are not unambigous.
> 3) The current solution silently create corrupt data if you pg is
>    configured to use dmy.
> 
> As you mention my solution still requires the configuration to accept
> iso formats, but it will crash early[1] if pgsql is not configured so,
> which is good. This is not the case with current DBD:Pg

If you s/ISO/DMY/g or s/ISO/MDY/g, you have the same exact problem with a
different date format. DBI is all about doing it the way you want in your
code, not the way I want you to, and changing the date format to something
just as fixed and arbitrary when reality has already confirmed that is not
a sound solution. This is no more robust than the current solution that is
currently in place, which was based off similar assumptions you're echoing
here.

I guess what I'm saying is, if we do it your way, there will be a bug next
week with a DMY=only guy having the same problem, and considering my
current availability I can't afford that kind of busywork. I would prefer
to do it right the first time, where the format is generated based on what
the database it's connected to supports. Doing this is currently
non-trivial, the inbound conversion maps have other issues that need to be
addressed in a future version of DBI for other reasons.

However, nothing is stopping you from passing a string yourself (which
could be the result of DateTime.now.strptime if you want) or even
overriding the existing conversion map. Both of these are documented
liberally. I also have serious doubts that it does this silently, as
postgres has typically been *very* anal about getting the format wrong in
the past, but I have no ability to test it at this moment.

> So all-in-all I consider my proposal at least as good as the current
> implementation. Yet I agree that it could be nice with an even better
> solution. The low-level pg-driver accepts a DateTime, could that be used
> in stead of handing over a string? Or should we do such thing as
> to_timestamp('2008-09-30 12:34:34.123456','YYYY-MM-DD HH:MM:SS.US');
> when writing to the database? In that case we still need to do
> something when reading from the database.

The problem you have identified is spot on, but your solution suits you and
not the general case. While I can understand your concern:

1) Nothing is preventing you from working around it at this time.
2) Changing the format without changing the logic serves a different subset
of people, it doesn't fix the problem.
3) I have, at this point, integrated and reverted several "it doesn't work
my way so here's a patch to fix it" patches that have gone to great lengths
to waste my time and embarrass me as I deal with frustrated people that I
just broke everything for when it was less specific.

Now that you understand my reasons for rejecting this idea, please be clear
that:

1) Type management is new in 0.4.0. Anything (yes, *anything*) other than a
string that worked before was quite literally non-deterministic and could
be easily described as "working on accident".
2) Type management is new, there are going to be bugs, and I can count the
number of people on two fingers that went out of their way to test and
report bugs with the version I advertised the alpha version before release
(one of them is me).
3) Nothing is stopping you from completely turning it off or just passing a
string that it would be coerced to anyways.

Now, I don't want to come off as bitter, because I'm really not, but I
don't think it's reasonable to expect me to handle this in the manner you'd
currently prefer. My workload is very high this week and I promise, as soon
as I have free time, I will focus my efforts on resolving this.

Thanks again for your time and comments,

-Erik



More information about the ruby-dbi-users mailing list