From 11de42bb0e115c2825b0380cfa3b4e7dbb929503 Mon Sep 17 00:00:00 2001 From: Bernd Helmle Date: Tue, 8 Dec 2015 11:14:34 +0100 Subject: [PATCH] Rework datetime/date conversion routines. The former coding was far too careless while converting PostgreSQL temporal values into Informix datum like date or datetime. The biggest problem was that the former coding treated Informix DATE and DATETIME nearly identical during conversion and fails to distinguish their different binary representation at ESQL/C level. This lead to various confusion, including scribbling beyond allocated memory. Fix this by introducing a routine ifxSetDate() which handles Informix DATE only and leave the work for DATETIME target types to setIfxDateTimestamp(), but preserve the possibility to transform PostgreSQL DATE to Informix DATETIME as well. Another issue was the treatment of microseconds during conversion from a PostgreSQL TIMESTAMP to its ISO string representation. PostgreSQL uses a 6 digit microsecond precision, while Informix only accepts 5 digits. This is a problem when passing a converted ISO string into an Informix conversion routine. We always claimed to *not* support fractions when retrieving temporal values, so omit microseconds fractions from a converted timestamp value and just pass up to milliseconds fractions only. --- ifx_connection.ec | 44 ++++++ ifx_conv.c | 365 +++++++++++++++++++++++++++++----------------- ifx_fdw.c | 37 ++++- ifx_fdw.h | 3 + ifx_type_compat.h | 3 + ifx_utils.c | 6 +- 6 files changed, 323 insertions(+), 135 deletions(-) diff --git a/ifx_connection.ec b/ifx_connection.ec index 3e12010..75103cf 100644 --- a/ifx_connection.ec +++ b/ifx_connection.ec @@ -1419,6 +1419,50 @@ void ifxSetTimestampFromString(IfxStatementInfo *info, int ifx_attnum, } } +/* + * Converts a date string into a + * DATE value and stores them in the SQLDA structure. + * + * We use rstrdate() internally which relies on DBDATE + * format strings. Since we automatically set an explicit value + * here, we expect the input string always being in the format + * YYYY-MM-DD. + * + * datestr must be a valid null terminated C string referencing + * the date string. + */ +void ifxSetDateFromString(IfxStatementInfo *info, + int ifx_attnum, + char *datestr) +{ + struct sqlda *ifx_sqlda; + struct sqlvar_struct *ifx_value; + mint converrcode; + + /* + * Set NULL indicator + */ + if (ifxSetSqlVarIndicator(info, + ifx_attnum, + info->ifxAttrDefs[ifx_attnum].indicator) != INDICATOR_NOT_NULL) + return; + + ifx_sqlda = (struct sqlda *)info->sqlda; + ifx_value = ifx_sqlda->sqlvar + ifx_attnum; + + /* + * We get the DATE value as an ANSI string formatted value + */ + if ((converrcode = rstrdate(datestr, + (int *)ifx_value->sqldata)) < 0) + { + /* An error ocurred, tell the caller something went wrong + * with this conversion */ + info->ifxAttrDefs[ifx_attnum].indicator = INDICATOR_NOT_VALID; + info->ifxAttrDefs[ifx_attnum].converrcode = converrcode; + } +} + /* * Convert a time string given in the format * hh24:mi:ss into an Informix DATETIME value. diff --git a/ifx_conv.c b/ifx_conv.c index fbf7049..36dff0c 100644 --- a/ifx_conv.c +++ b/ifx_conv.c @@ -1065,6 +1065,103 @@ void setIfxInterval(IfxFdwExecutionState *state, } } +/* + * Assigns the specified Datum as an Informix DATE value + * to the current SQLDA structure provided by the given + * execution state handle. + * + * Conversion is supported from a PostgreSQL DATE source type + * to an Informix DATE type only. + */ +void setIfxDate(IfxFdwExecutionState *state, + TupleTableSlot *slot, + int attnum) +{ + char *strval = NULL; + Datum datval; + + /* Sanity check, SQLDA available? */ + Assert(state->stmt_info.sqlda != NULL); + Assert(IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum)) == IFX_DATE); + + /* + * Only IFX_DATE is supported here as a target type. + */ + if (IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum)) == IFX_DATE) + { + /* + * Target type must be an Informix DATE type. However, we convert + * the PostgreSQL datum into it's ANSI string representation first, which + * makes it easier to pass them over to Informix. A date is always formatted + * as yyyy-mm-dd, ifxSetDateFromString() will transfer it back into ESQL/C binary + * representation. + */ + + /* + * Don't try to convert a NULL datum. + */ + if (! IFX_ATTR_ISNULL_P(state, IFX_ATTR_PARAM_ID(state, attnum)) + && IFX_ATTR_IS_VALID_P(state, IFX_ATTR_PARAM_ID(state, attnum))) + { + PG_TRY(); + { + text *format; + + /* + * Cast the DATE datum to a timestamp first and convert it + * into a ISO8601 compatible string format. Inefficient, but we do + * it this way to be independent from any locale and give a + * static input format to our Informix conversion routines. + */ + format = cstring_to_text( + ifxGetIntervalFormatString( + ifxGetTemporalQualifier(&(state->stmt_info), + IFX_ATTR_PARAM_ID(state, attnum)), + FMT_PG)); + datval = DirectFunctionCall2(timestamp_to_char, + slot->tts_values[attnum], + PointerGetDatum(format)); + + pfree(format); + } + PG_CATCH(); + { + ifxRewindCallstack(&state->stmt_info); + PG_RE_THROW(); + } + PG_END_TRY(); + } + } + else + { + /* unknown target type */ + elog(ERROR, "informix_fdw could not convert local type %u to target type %d", + PG_ATTRTYPE_P(state, attnum), + IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum))); + } + + strval = text_to_cstring(DatumGetTextP(datval)); + + /* + * Sanity check, C string has maximum allowed buffer length? + */ + Assert(strlen(strval) <= IFX_DATETIME_BUFFER_LEN); + + elog(DEBUG4, "informix_fdw: attnum %d, converted temporal value \"%s\"", + attnum, strval); + + /* + * Put the converted DATE string into SQLDA + */ + ifxSetDateFromString(&state->stmt_info, + IFX_ATTR_PARAM_ID(state, attnum), + strval); + + /* cleanup */ + if (strval != NULL) + pfree(strval); +} + /* * Encapsulates assignment from a PostgreSQL * timestamp or date value into an informix DATETIME @@ -1079,158 +1176,166 @@ void setIfxDateTimestamp(IfxFdwExecutionState *state, */ Assert(state->stmt_info.sqlda != NULL); - switch(IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum))) + if (IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum)) == IFX_DTIME) { - case IFX_DTIME: - case IFX_DATE: + Datum datval; + char *strval = NULL; + + /* + * Target type must be an Informix timestamp or date value. + * + * NOTE: Informix DATE and DATETIME value doesn't understand time zones, nor + * do they have the same precision. Because of this we always + * convert a TIMESTAMP or DATE value into its ANSI string format + * (that is yyyy-mm-dd hh24:mi:ss) first. + */ + + + /* + * Don't try to convert a NULL timestamp value into a character string, + * but step into the conversion routine to set all required + * SQLDA info accordingly. + */ + if (! IFX_ATTR_ISNULL_P(state, IFX_ATTR_PARAM_ID(state, attnum)) + && IFX_ATTR_IS_VALID_P(state, IFX_ATTR_PARAM_ID(state, attnum))) { - /* - * Target type must be an Informix timestamp or date value. - * - * NOTE: Informix DATE and DATETIME value doesn't understand time zones, nor - * do they have the same precision. Because of this we always - * convert a TIMESTAMP or DATE value into its ANSI string format - * (that is yyyy-mm-dd hh24:mi:ss) first. - */ - Datum datval; - char *strval = NULL; + text *format; - /* - * Don't try to convert a NULL timestamp value into a character string, - * but step into the conversion routine to set all required - * SQLDA info accordingly. - */ - if (! IFX_ATTR_ISNULL_P(state, IFX_ATTR_PARAM_ID(state, attnum)) - && IFX_ATTR_IS_VALID_P(state, IFX_ATTR_PARAM_ID(state, attnum))) + PG_TRY(); { - text *format; - - PG_TRY(); + /* + * Make a string from the given DATE or TIMESTAMP(TZ) + * datum in ANSI format. + */ + switch(PG_ATTRTYPE_P(state, attnum)) { - /* - * Make a string from the given DATE or TIMESTAMP(TZ) - * datum in ANSI format. - */ - switch(PG_ATTRTYPE_P(state, attnum)) + case TIMESTAMPTZOID: { - case TIMESTAMPTZOID: - { - format = cstring_to_text("yyyy-mm-dd hh24:mi:ss"); - datval = DirectFunctionCall2(timestamptz_to_char, - slot->tts_values[attnum], - PointerGetDatum(format)); - break; - } - case TIMESTAMPOID: - { - format = cstring_to_text("yyyy-mm-dd hh24:mi:ss"); - datval = DirectFunctionCall2(timestamp_to_char, - slot->tts_values[attnum], - PointerGetDatum(format)); - break; - } - case DATEOID: - { - regproc castproc; - Datum castval; + format = cstring_to_text(ifxGetIntervalFormatString( + ifxGetTemporalQualifier(&(state->stmt_info), + IFX_ATTR_PARAM_ID(state, attnum)), + FMT_PG)); + datval = DirectFunctionCall2(timestamptz_to_char, + slot->tts_values[attnum], + PointerGetDatum(format)); + break; + } + case DATEOID: + { + /* + * Cast any DATEOID datum to timestamp first, as we'd like to call + * to_char directly which is only present for timestamp datums. + * + * This is what effectively also happens if you call to_char() on the SQL + * level, too. + */ + Datum castval; + regproc castfunc; - /* - * Cast it to a timestamp without time zone value. - */ - castproc = getTypeCastFunction(state, DATEOID, TIMESTAMPOID); - castval = OidFunctionCall1(castproc, slot->tts_values[attnum]); + castfunc = getTypeCastFunction(state, DATEOID, TIMESTAMPOID); + castval = OidFunctionCall1(castfunc, slot->tts_values[attnum]); - /* - * Now try to convert it into a valid string. - */ - format = cstring_to_text("yyyy-mm-dd"); - datval = DirectFunctionCall2(timestamp_to_char, - castval, - PointerGetDatum(format)); - break; - } - case TIMEOID: - { - regproc castproc; - Datum castval; + format = cstring_to_text(ifxGetIntervalFormatString( + ifxGetTemporalQualifier(&(state->stmt_info), + IFX_ATTR_PARAM_ID(state, attnum)), + FMT_PG)); - /* - * Cast the time value into an interval type, - * which can then be converted into a valid text - * string, suitable to be passed down to Informix. - */ - castproc = getTypeCastFunction(state, TIMEOID, INTERVALOID); - castval = OidFunctionCall1(castproc, slot->tts_values[attnum]); + datval = DirectFunctionCall2(timestamp_to_char, + castval, + PointerGetDatum(format)); - /* - * Now try to convert the value - */ - format = cstring_to_text("hh24:mi:ss"); - datval = DirectFunctionCall2(interval_to_char, - castval, - PointerGetDatum(format)); - break; - } - default: - /* we shouldn't reach this, so error out hard */ - ereport(ERROR, - (errcode(ERRCODE_FDW_INVALID_DATA_TYPE), - errmsg("informix_fdw unsupported type OID \"%u\" for conversion", - PG_ATTRTYPE_P(state, attnum)))); - break; + break; + + } + case TIMESTAMPOID: + { + format = cstring_to_text(ifxGetIntervalFormatString( + ifxGetTemporalQualifier(&(state->stmt_info), + IFX_ATTR_PARAM_ID(state, attnum)), + FMT_PG)); + datval = DirectFunctionCall2(timestamp_to_char, + slot->tts_values[attnum], + PointerGetDatum(format)); + break; } + case TIMEOID: + { + regproc castproc; + Datum castval; - strval = text_to_cstring(DatumGetTextP(datval)); - } - PG_CATCH(); - { - ifxRewindCallstack(&state->stmt_info); - PG_RE_THROW(); - } - PG_END_TRY(); + /* + * Cast the time value into an interval type, + * which can then be converted into a valid text + * string, suitable to be passed down to Informix. + */ + castproc = getTypeCastFunction(state, TIMEOID, INTERVALOID); + castval = OidFunctionCall1(castproc, slot->tts_values[attnum]); - /* - * Sanity check, C string valid? - */ - Assert(strval != NULL); - Assert(strlen(strval) <= IFX_DATETIME_BUFFER_LEN); + /* + * Now try to convert the value + */ + format = cstring_to_text("hh24:mi:ss"); + datval = DirectFunctionCall2(interval_to_char, + castval, + PointerGetDatum(format)); + break; + } + default: + /* we shouldn't reach this, so error out hard */ + ereport(ERROR, + (errcode(ERRCODE_FDW_INVALID_DATA_TYPE), + errmsg("informix_fdw unsupported type OID \"%u\" for conversion", + PG_ATTRTYPE_P(state, attnum)))); + break; + } - /* cleanup */ - pfree(format); + strval = text_to_cstring(DatumGetTextP(datval)); } - - elog(DEBUG4, "informix_fdw: attnum %d, converted temporal value \"%s\"", - attnum, strval); + PG_CATCH(); + { + ifxRewindCallstack(&state->stmt_info); + PG_RE_THROW(); + } + PG_END_TRY(); /* - * Okay, try to store the value into SQLDA + * Sanity check, C string valid? */ - if (PG_ATTRTYPE_P(state, attnum) != TIMEOID) - { - ifxSetTimestampFromString(&state->stmt_info, - IFX_ATTR_PARAM_ID(state, attnum), - strval); - } - else - { - ifxSetTimeFromString(&state->stmt_info, - IFX_ATTR_PARAM_ID(state, attnum), - strval); - } + Assert(strval != NULL); + Assert(strlen(strval) <= IFX_DATETIME_BUFFER_LEN); - /* ...and we're done */ - if (strval != NULL) - pfree(strval); + /* cleanup */ + pfree(format); + } - break; + elog(DEBUG4, "informix_fdw: attnum %d, converted temporal value \"%s\"", + attnum, strval); + + if (PG_ATTRTYPE_P(state, attnum) == TIMEOID) + { + ifxSetTimeFromString(&state->stmt_info, + IFX_ATTR_PARAM_ID(state, attnum), + strval); } - default: - /* unknown target type */ - elog(ERROR, "informix_fdw could not convert local type %u to target type %d", - PG_ATTRTYPE_P(state, attnum), - IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum))); - } + else + { + ifxSetTimestampFromString(&state->stmt_info, + IFX_ATTR_PARAM_ID(state, attnum), + strval); + } + + /* ...and we're done */ + if (strval != NULL) + pfree(strval); + } + else + { + /* unknown target type */ + elog(ERROR, "informix_fdw could not convert local type %u to target type %d", + PG_ATTRTYPE_P(state, attnum), + IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum))); + } } /* diff --git a/ifx_fdw.c b/ifx_fdw.c index f88ab23..599e984 100644 --- a/ifx_fdw.c +++ b/ifx_fdw.c @@ -1064,11 +1064,44 @@ static void ifxColumnValuesToSqlda(IfxFdwExecutionState *state, setIfxCharString(state, attnum, buf, buflen); break; } + case DATEOID: + /* + * We support conversion from a PostgreSQL DATE type either to + * an Informix DATETIME or DATE data type. However, we have to take care + * to choose the right conversion, since DATE and DATETIME aren't binary + * compatible types in Informix and thus require special handling. + * + * So, depending on the target column, choose the right conversion routine. + * We check specificially for DATE, all other DATETIME conversion could + * be handled by setIfxDateTimestamp(). + */ + if (IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum)) == IFX_DATE) + { + setIfxDate(state, slot, attnum); + break; + } + + /* fall through in case target column is DTIME */ case TIMESTAMPTZOID: case TIMESTAMPOID: case TIMEOID: - case DATEOID: - setIfxDateTimestamp(state, slot, attnum); + /* + * Be paranoid: DATE should already be handled above, but make + * sure we have a compatible target type. DTIME is the only allowed + * target type at this point. + */ + if (IFX_ATTRTYPE_P(state, IFX_ATTR_PARAM_ID(state, attnum)) == IFX_DTIME) + { + setIfxDateTimestamp(state, slot, attnum); + } + else + { + ifxRewindCallstack(&state->stmt_info); + elog(ERROR, "informix_fdw: cannot convert type oid %u of attnum %d to informix type id %d", + PG_ATTRTYPE_P(state, attnum), + attnum, + state->stmt_info.ifxAttrDefs[attnum].type); + } break; case INTERVALOID: setIfxInterval(state, slot, attnum); diff --git a/ifx_fdw.h b/ifx_fdw.h index 3a3a6a9..cd6f351 100644 --- a/ifx_fdw.h +++ b/ifx_fdw.h @@ -380,6 +380,9 @@ void setIfxCharString(IfxFdwExecutionState *state, void setIfxDateTimestamp(IfxFdwExecutionState *state, TupleTableSlot *slot, int attnum); +void setIfxDate(IfxFdwExecutionState *state, + TupleTableSlot *slot, + int attnum); void setIfxInterval(IfxFdwExecutionState *state, TupleTableSlot *slot, int attnum); diff --git a/ifx_type_compat.h b/ifx_type_compat.h index a32cb4c..5fd84de 100644 --- a/ifx_type_compat.h +++ b/ifx_type_compat.h @@ -696,6 +696,9 @@ void ifxSetTimestampFromString(IfxStatementInfo *info, int ifx_attnum, char *dtstring); void ifxSetTimeFromString(IfxStatementInfo *info, int ifx_attnum, char *timestr); +void ifxSetDateFromString(IfxStatementInfo *info, + int ifx_attnum, + char *datestr); void ifxSetText(IfxStatementInfo *info, int ifx_attnum, char *value); void ifxSetSimpleLO(IfxStatementInfo *info, int ifx_attnum, char *buf, int buflen); diff --git a/ifx_utils.c b/ifx_utils.c index 14f6cc2..36867b8 100644 --- a/ifx_utils.c +++ b/ifx_utils.c @@ -61,8 +61,8 @@ static ifxTemporalFormatIdent ifxTemporalFormat[] = { "%F", "MS" }, { "%2F", "MS" }, { "%3F", "MS" }, - { "%4F", "US" }, - { "%5F", "US" } + { "%4F", "MS" }, + { "%5F", "MS" } }; /* @@ -425,7 +425,7 @@ char *ifxGetIntervalFormatString(IfxTemporalRange range, IfxFormatMode mode) break; case IFX_TU_SECOND: /* must be fraction here */ - //appendStringInfoString(&strbuf, "."); + appendStringInfoString(&strbuf, "."); case IFX_TU_F1: case IFX_TU_F2: case IFX_TU_F3: