On 10/07/2014 02:45 AM, Matthew McGillis via RT wrote:
Show quoted text> Queue: DBIx-Class
> Ticket <URL:
https://rt.cpan.org/Ticket/Display.html?id=99334 >
>
> Did some more digging.
>
> This is what I found that is so far working for me in the DBIx::Class code.
>
> 1) To fix the error about date format not supported
>
> Comment out this in: DBIx::Class::Storage::DBI::MSSQL
>
> #__PACKAGE__->datetime_parser_type (
> # 'DBIx::Class::Storage::DBI::MSSQL::DateTime::Format'
> #);
>
> The DBIx::Class::Storage::DBI::MSSQL::DateTime::Format doesn't appear to really be different from the default other than it is dummer.
This is incorrect. The default formatter has no concept of microseconds
- if you tried to execute the test suite with the above change you'd
have noticed that t/inflate/datetime_mssql.t started failing. The proper
fix is to add a sub format_date { ... } and sub parse_date { ... }with
the appropriate code in them.
Show quoted text>
> 2 To fix the warning message about DateTime object usage in searches.
>
> Note: I have never seen this because I'm usually not using the current DBIx:Class code the version of code I'm generally using does not generate this warning.
>
> The DBIx::Class 0.082801 code has:
>
> if (
> ! $ENV{DBIC_DT_SEARCH_OK}
> and
> $op eq 'select'
> and
> first {
> length ref $_->[1]
> and
> blessed($_->[1])
> and
> $_->[1]->isa('DateTime')
> } @$bind
> ) {
> carp_unique 'DateTime objects passed to search() are not supported '
> . 'properly (InflateColumn::DateTime formats and settings are not '
> . 'respected.) See "Formatting DateTime objects in queries" in '
> . 'DBIx::Class::Manual::Cookbook. To disable this warning for good '
> . 'set $ENV{DBIC_DT_SEARCH_OK} to true'
> }
>
> I added this just before the above as a quick fix for better transparent support of DateTime objects in queries I expect someone familiar with the code can do a much better job of this but this seems to work for me (working out a generic api for any InflateColumn seems to me like it would make since):
>
> $bind = [ map {
> if (ref($_->[1]) eq 'DateTime') {
> if (exists($_->[0]{sqlt_datatype})) {
> if ($_->[0]{sqlt_datatype} eq 'date') {
> $_->[1]=$self->datetime_parser->format_date($_->[1]);
> } elsif ($_->[0]{sqlt_datatype} eq 'datetime') {
> $_->[1]=$self->datetime_parser->format_datetime($_->[1]);
> }
> # add other formats if we support them ?time? maybe.
> }
> }
> $_;
> } @$bind];
>
Let's step back and think about this. There is a rather verbose warning
in place, in one of the more widely used modules on CPAN. If there
indeed was a way to solve the problem *completely*, a solution (no
matter how contrived) would have been put in place instead of this warning.
The reason your proposal is not acceptable is that any column_info
metadata that drives how exactly the given times are interpreted are not
consulted. So now everyone who used the 'timezone' option of IC::DT is
going to start getting incorrect values shoved into the database.
The fundamental problem stems from the entire InflateColumn
infrastructure being misdesigned in a way that makes it not suitable for
use in a search() context. In fact this is the oldest open bug in the
queue:
https://rt.cpan.org/Ticket/Display.html?id=43075. The real answer
(which is currently blocked by lack of tuits) is to add an InflateColumn
replacement, and use it for row-data-independent inflators (like DT
inflation) instead. At the rate of current dev this feature is unlikely
to come in before next spring.
Note that I am giving you the above verbose explanation not as an
"intimidation tool", but simply as a better reason why the second part
of your report is not acceptable for mainlining. For the time being the
only official way to use DT in search()es remains the one described
here:
https://metacpan.org/pod/release/RIBASUSHI/DBIx-Class-0.082801/lib/DBIx/Class/Manual/Cookbook.pod#Formatting-DateTime-objects-in-queries
Your original issue of lacking a "date" inflator/deflator is however
indeed fixable. Or as you noted earlier - switching the datatype to
"datetime" is a quick workaround for this (I will still fix the date
situation when I get to it, likely next maj, release in a months or so).
Cheers and thanks for looking into this ;)
