Fix handling of size with DATE parameters (#178)

* fix size of DATE parameters

This commit fixes the handling of the columns size of the DATE type parameters.

The previous implementation always expected the size of an ISO8601
timestamp, since DATE and TIME vals area ultimately parsed as a
timestamp. However the column size was passed through unchanged. With
this commit, the size is read as passed in by the application and
adjusted as needed.

* address PR review notes, fixing logging messages

- fix missing arguments to logging macros;
- improve phrasing.

* address PR notes: fix logging format specifier

- s/%d/%zu

Author: bpintea

driver/convert.c

@@ -3118,24 +3118,26 @@ static int print_timestamp(TIMESTAMP_STRUCT *tss, BOOL iso8601,
 			tss->hour, tss->minute, tss->second,
 			/* fraction is always provided, but only printed if 'decdigits' */
 			decdigits, nsec);
+	if (n <= 0) {
+		return n;
+	}
+
 	if ((int)lim < n) {
 		n = (int)lim;
 	}
-	if (0 < n) {
-		if (iso8601) {
-			dest[DATE_TEMPLATE_LEN] = L'T';
-			/* The SQL column sizes are considered for ISO format too, to
-			 * allow the case where the client app specifies a timestamp with
-			 * non-zero seconds, but wants to cut those away in the parameter.
-			 * The 'Z' would then be on top of the colsize. */
-			dest[n] = L'Z';
-			n ++;
-			dest[n] = L'\0';
-		}
-		DBG("printed UTC %s timestamp (colsz: %lu, decdig: %hd): "
-			"[%d] `" LWPDL "`.", iso8601 ? "ISO8601" : "SQL",
-			(SQLUINTEGER)colsize, decdigits, n, n, dest);
+	if (iso8601) {
+		dest[DATE_TEMPLATE_LEN] = L'T';
+		/* The SQL column sizes are considered for ISO format too, to
+		 * allow the case where the client app specifies a timestamp with
+		 * non-zero seconds, but wants to cut those away in the parameter.
+		 * The 'Z' would then be on top of the colsize. */
+		dest[n] = L'Z';
+		n ++;
 	}
+	dest[n] = L'\0';
+	DBG("printed UTC %s timestamp (colsz: %lu, decdig: %hd): "
+		"[%d] `" LWPDL "`.", iso8601 ? "ISO8601" : "SQL",
+		(SQLUINTEGER)colsize, decdigits, n, n, dest);
 
 	return n;
 }
@@ -4254,10 +4256,80 @@ static SQLRETURN struct_to_iso8601_timestamp(esodbc_stmt_st *stmt,
 	return SQL_SUCCESS;
 }
 
+/* apply corrections depending on the (column) size and decimal digits
+ * values given at binding time: nullify or trim the resulted string:
+ * https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size
+ * */
+static SQLRETURN size_decdigits_for_iso8601(esodbc_rec_st *irec,
+	SQLULEN *_colsize, SQLSMALLINT *_decdigits)
+{
+	SQLULEN colsize;
+	SQLSMALLINT decdigits;
+	esodbc_stmt_st *stmt = HDRH(irec->desc)->stmt;
+
+	colsize = get_param_size(irec);
+	DBGH(stmt, "requested column size: %llu.", colsize);
+
+	decdigits = get_param_decdigits(irec);
+	DBGH(stmt, "requested decimal digits: %llu.", decdigits);
+	if (ESODBC_MAX_SEC_PRECISION < decdigits) {
+		WARNH(stmt, "requested decimal digits adjusted from %hd to %d (max).",
+			decdigits, ESODBC_MAX_SEC_PRECISION);
+		decdigits = ESODBC_MAX_SEC_PRECISION;
+	}
+
+	switch (irec->es_type->data_type) {
+		case SQL_TYPE_TIME:
+			if (colsize) {
+				if (colsize < TIME_TEMPLATE_LEN(0) ||
+					colsize == TIME_TEMPLATE_LEN(1) - 1 /* `:ss.`*/) {
+					ERRH(stmt, "invalid column size value: %llu; allowed: "
+						"8 or 9 + fractions count.", colsize);
+					RET_HDIAGS(stmt, SQL_STATE_HY104);
+				}
+				colsize += DATE_TEMPLATE_LEN + /* ` `/`T` */1;
+			}
+			break;
+		case SQL_TYPE_DATE:
+			/* if origin is a timestamp (struct or string), the time part
+			 * needs to be zeroed. */
+			if (colsize) {
+				if (colsize != DATE_TEMPLATE_LEN) {
+					ERRH(stmt, "invalid column size value: %llu; allowed: "
+						"%zu.", colsize, DATE_TEMPLATE_LEN);
+					RET_HDIAGS(stmt, SQL_STATE_HY104);
+				}
+				colsize += /* ` `/`T` */1 + TIME_TEMPLATE_LEN(0);
+			}
+			if (decdigits) {
+				ERRH(stmt, "invalid decimal digits %hd for TIME type.",
+					decdigits);
+				RET_HDIAGS(stmt, SQL_STATE_HY104);
+			}
+			break;
+		case SQL_TYPE_TIMESTAMP:
+			if (colsize && (colsize < TIMESTAMP_NOSEC_TEMPLATE_LEN ||
+					colsize == 17 || colsize == 18)) {
+				ERRH(stmt, "invalid column size value: %llu; allowed: "
+					"16, 19 or 20 + fractions count.", colsize);
+				RET_HDIAGS(stmt, SQL_STATE_HY104);
+			}
+			break;
+		default:
+			assert(0);
+	}
+
+	DBGH(stmt, "applying: column size: %llu, decimal digits: %hd.",
+		colsize, decdigits);
+	*_colsize = colsize;
+	*_decdigits = decdigits;
+	return SQL_SUCCESS;
+}
+
 SQLRETURN c2sql_date_time(esodbc_rec_st *arec, esodbc_rec_st *irec,
 	SQLULEN pos, char *dest, size_t *len)
 {
-#	define ZERO_TIME_Z "00:00:00Z"
+	static const wstr_st time_0_z = WSTR_INIT("00:00:00Z");
 	esodbc_stmt_st *stmt;
 	void *data_ptr;
 	SQLLEN *octet_len_ptr;
@@ -4285,24 +4357,9 @@ SQLRETURN c2sql_date_time(esodbc_rec_st *arec, esodbc_rec_st *irec,
 	/* pointer to app's buffer */
 	data_ptr = deferred_address(SQL_DESC_DATA_PTR, pos, arec);
 
-	/* apply corrections depending on the (column) size and decimal digits
-	 * values given at binding time: nullify or trim the resulted string:
-	 * https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size
-	 * */
-	colsize = get_param_size(irec);
-	DBGH(stmt, "requested column size: %llu.", colsize);
-	if (colsize && (colsize < sizeof("yyyy-mm-dd hh:mm") - 1 ||
-			colsize == 17 || colsize == 18)) {
-		ERRH(stmt, "invalid column size value: %llu; allowed: 16, 19, 20+f.",
-			colsize);
-		RET_HDIAGS(stmt, SQL_STATE_HY104);
-	}
-	decdigits = get_param_decdigits(irec);
-	DBGH(stmt, "requested decimal digits: %llu.", decdigits);
-	if (ESODBC_MAX_SEC_PRECISION < decdigits) {
-		WARNH(stmt, "requested decimal digits adjusted from %hd to %d (max).",
-			decdigits, ESODBC_MAX_SEC_PRECISION);
-		decdigits = ESODBC_MAX_SEC_PRECISION;
+	ret = size_decdigits_for_iso8601(irec, &colsize, &decdigits);
+	if (! SQL_SUCCEEDED(ret)) {
+		return ret;
 	}
 
 	/*INDENT-OFF*/
@@ -4317,7 +4374,7 @@ SQLRETURN c2sql_date_time(esodbc_rec_st *arec, esodbc_rec_st *irec,
 			}
 			/* disallow DATE <-> TIME conversions */
 			if ((irec->es_type->data_type == SQL_C_TYPE_TIME &&
-					format == SQL_C_TYPE_DATE) || (format == SQL_C_TYPE_TIME &&
+					format == SQL_TYPE_DATE) || (format == SQL_TYPE_TIME &&
 					irec->es_type->data_type == SQL_C_TYPE_DATE)) {
 				ERRH(stmt, "TIME-DATE conversions are not possible.");
 				RET_HDIAGS(stmt, SQL_STATE_22018);
@@ -4353,27 +4410,27 @@ SQLRETURN c2sql_date_time(esodbc_rec_st *arec, esodbc_rec_st *irec,
 	 * expense. */
 	/* Adapt the resulting ISO8601 value to the target data type */
 	switch (irec->es_type->data_type) {
-		case SQL_C_TYPE_TIME:
+		case SQL_TYPE_TIME:
 			/* shift value + \0 upwards over the DATE component */
 			/* Note: by the book, non-0 fractional seconds in timestamp should
 			 * lead to 22008 a failure. However, ES/SQL's TIME supports
 			 * fractions, so will just ignore this provision. */
 			cnt -= DATE_TEMPLATE_LEN + /*'T'*/1;
 			wmemmove(wbuff, wbuff + DATE_TEMPLATE_LEN + /*'T'*/1, cnt + 1);
 			break;
-		case SQL_C_TYPE_DATE:
+		case SQL_TYPE_DATE:
 			/* if origin is a timestamp (struct or string), the time part
 			 * needs to be zeroed. */
 			if (ctype == SQL_C_TYPE_TIMESTAMP ||
-				format == SQL_C_TYPE_TIMESTAMP) {
+				format == SQL_TYPE_TIMESTAMP) {
 				assert(ISO8601_TIMESTAMP_MIN_LEN <= cnt);
 				wmemcpy(wbuff + DATE_TEMPLATE_LEN + /*'T'*/1,
-					MK_WPTR(ZERO_TIME_Z), sizeof(ZERO_TIME_Z) /*+\0*/);
+					(wchar_t *)time_0_z.str, time_0_z.cnt + /*\0*/1);
 				cnt = ISO8601_TIMESTAMP_MIN_LEN;
 			}
 			break;
 		default:
-			assert(irec->es_type->data_type == SQL_C_TYPE_TIMESTAMP);
+			assert(irec->es_type->data_type == SQL_TYPE_TIMESTAMP);
 	}
 	DBGH(stmt, "converted value: [%zu] `" LWPDL "`.", cnt, cnt, wbuff);
 
@@ -4383,7 +4440,6 @@ SQLRETURN c2sql_date_time(esodbc_rec_st *arec, esodbc_rec_st *irec,
 
 	dest[(*len) ++] = '"';
 	return SQL_SUCCESS;
-#	undef ZERO_TIME_Z
 }
 
 /* parses an interval literal string from app's char/wchar_t buffer */

driver/util.h

@@ -350,6 +350,8 @@ char *cstr_hex_dump(const cstr_st *buff);
 	(sizeof("hh:mm:ss") - /*\0*/1 + /*'.'*/!!prec + prec)
 #define TIMESTAMP_TEMPLATE_LEN(prec)	\
 	(DATE_TEMPLATE_LEN + /*' '*/1 + TIME_TEMPLATE_LEN(prec))
+#define TIMESTAMP_NOSEC_TEMPLATE_LEN	\
+	(DATE_TEMPLATE_LEN + /*' '*/1 + sizeof("hh:mm") - /*\0*/1)
 
 
 #endif /* __UTIL_H__ */

test/test_conversion_c2sql_date.cc

@@ -36,6 +36,18 @@ TEST_F(ConvertC2SQL_Date, Date2Date)
 		"\"value\": \"1234-12-23T00:00:00Z\"}]");
 }
 
+TEST_F(ConvertC2SQL_Date, CStr_Date2Date_size10)
+{
+	SQLCHAR val[] = "2000-01-01"; // treated as utc, since apply_tz==FALSE
+	ret = SQLBindParameter(stmt, 1, SQL_PARAM_INPUT, SQL_C_CHAR,
+			SQL_TYPE_DATE, /*size*/10, /*decdigits*/0, val, sizeof(val),
+			/*IndLen*/NULL);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+
+	assertRequest("[{\"type\": \"DATE\", "
+		"\"value\": \"2000-01-01T00:00:00Z\"}]");
+}
+
 TEST_F(ConvertC2SQL_Date, CStr_Date2Date)
 {
 	SQLCHAR val[] = "2000-01-01"; // treated as utc, since apply_tz==FALSE
@@ -100,6 +112,18 @@ TEST_F(ConvertC2SQL_Date, WStr_Timestamp2Date)
 		"\"value\": \"1234-12-23T00:00:00Z\"}]");
 }
 
+TEST_F(ConvertC2SQL_Date, WStr_Timestamp2Date_size10)
+{
+	SQLWCHAR val[] = L"1234-12-23T12:34:56.7890123Z";
+	ret = SQLBindParameter(stmt, 1, SQL_PARAM_INPUT, SQL_C_WCHAR,
+			SQL_TYPE_DATE, /*size*/10, /*decdigits*/0, val, sizeof(val),
+			/*IndLen*/NULL);
+	ASSERT_TRUE(SQL_SUCCEEDED(ret));
+
+	assertRequest("[{\"type\": \"DATE\", "
+		"\"value\": \"1234-12-23T00:00:00Z\"}]");
+}
+
 /* note: test name used in test */
 TEST_F(ConvertC2SQL_Date, Timestamp2Date)
 {

test/test_conversion_c2sql_time.cc

@@ -75,11 +75,11 @@ TEST_F(ConvertC2SQL_Time, WStr_Timestamp2Time_colsize_16)
 {
 	SQLWCHAR val[] = L"1234-12-23T12:34:56.7890123Z";
 	ret = SQLBindParameter(stmt, 1, SQL_PARAM_INPUT, SQL_C_WCHAR,
-			SQL_TYPE_TIME, /*size*/16, /*decdigits*/0, val, sizeof(val),
+			SQL_TYPE_TIME, /*size*/8, /*decdigits*/0, val, sizeof(val),
 			/*IndLen*/NULL);
 	ASSERT_TRUE(SQL_SUCCEEDED(ret));
 
-	assertRequest("[{\"type\": \"TIME\", \"value\": \"12:34Z\"}]");
+	assertRequest("[{\"type\": \"TIME\", \"value\": \"12:34:56Z\"}]");
 }
 
 /* note: test name used in test */