Re: [PATCH] Remove unncessary localtime() calls during data type conversion

On Tue, Sep 1, 2015 at 6:11 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> This changes behaviour, when converting from text to date column. Without
> this patch, if the string you're converting from is missing a
> year/month/date field, it will be filled with the current date.

Waking up an old thread, after being reminded by some colleagues that
there are no improvements in the upstream code yet regarding that...

> This modification to the result-conversions test case shows the difference:
>
> --- a/test/src/result-conversions-test.c
> +++ b/test/src/result-conversions-test.c
> @@ -626,6 +626,8 @@ int main(int argc, char **argv)
>         test_conversion("float8", "Infinity", SQL_C_DOUBLE, "SQL_C_DOUBLE",
> 20);
>         test_conversion("float8", "-Infinity", SQL_C_DOUBLE, "SQL_C_DOUBLE",
> 20);
>
> +       test_conversion("text", "", SQL_C_TYPE_DATE, "SQL_C_TYPE_DATE", 20);
> +
>         /* Clean up */
>         test_disconnect();
>
> We don't currently contain tests like that in the test suite, as mentioned
> in the comment earlier in result-conversions-test.c:

Yes, I can see the difference. I am noticing that only SQL_C_TYPE_DATE
and SQL_C_TYPE_TIMESTAMP are actually converting the non-defined
fields using the localtime values. SQL_C_TYPE_TIME is not impacted at
all. Is that expected?

>>         /*
>>          * Converting arbitrary things to date and timestamp produces
>> results that
>>          * depend on the current timestamp, because the driver substitutes
>> the
>>          * current year/month/datefor missing values. Disable for now, to
>> get a
>>          * reproducible result.
>>          */
>
>
> Would be nice to include those tests, perhaps by printing the difference to
> current date instead of the absolute date.

Yeah, I have done that in the attached with 0001, however I don't
really see how we can actually remove those comments, because some of
the tests include a fixed timestamp, and need to use a fixed
timestamp.

I am attaching patch 0002 that is a rebase of the main patch for HEAD,
that introduces no regressions based on the tests I have added in
0001. 0002 is proving to improve performance of the ODBC driver by
more or less 20% by reducing those localtime() calls in perf specs for
an application that is pretty hot with SQLFetch. That's quite
something.
--
Michael