Bug #31246 Opening rowset with extra fields leads to incorrect SQL INSERT
Submitted: 27 Sep 2007 15:26 Modified: 12 Oct 2007 13:07
Reporter: Craig Holmquist Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / ODBC Severity:S3 (Non-critical)
Version:3.51.20 OS:Microsoft Windows (XP SP2)
Assigned to: Bogdan Degtyariov CPU Architecture:Any

[27 Sep 2007 15:26] Craig Holmquist
Description:
Opening a cursor/rowset with certain fields, then attempting to add a new row to that set with fewer fields sends an INSERT statement to the server with invalid syntax.  This happens if the "missing" fields are integers.  If the missing fields are varchar, the driver sends an empty string, even if the column has a different default.

In the test case provided below, the table has the following columns, with first two specified as the primary key:

field1 VARCHAR(255) NOT NULL
field2 VARCHAR(255) NOT NULL
field3 int DEFAULT NULL
field4 VARCHAR(50) DEFAULT NULL

Opening the set with all four, then attempting to add a new row with only the first two yields this statement (from the server log):

INSERT INTO `testadd`(`field1`,`field2`,`field3`,`field4`) VALUES ('1','2',,'')

Note that:
1. field3 has NO value at all
2. field4 uses an empty string instead of its prescribed default (NULL)

I reproduced this with 3.51.18 and 3.51.20.

How to repeat:
//This test case was written with Microsoft Visual Studio 2003 and requires MFC.

#include <afx.h>
#include <afxdb.h>

class CTestAdd : public CRecordset
{
	public:
	CTestAdd(CDatabase &db);
	
	virtual CString GetDefaultSQL();
	virtual void DoFieldExchange(CFieldExchange* pFX);
	
	CString m_str1, m_str2;
};

CTestAdd::CTestAdd(CDatabase &db)
:	CRecordset(&db)
{
	m_nFields = 2;
}

CString CTestAdd::GetDefaultSQL()
{
	return _T("SELECT field1, field2 FROM testadd");
}

void CTestAdd::DoFieldExchange(CFieldExchange* pFX)
{
	pFX->SetFieldType(CFieldExchange::outputColumn);
	RFX_Text(pFX, _T("[field1]"), m_str1);
	RFX_Text(pFX, _T("[field2]"), m_str2);
}

int main(int argc, CHAR* argv[])
{
	if(argc != 3)
	{
		printf("Must specify user name and password.\r\n");
		return 0;
	}
	
	CString strConnect;
	strConnect.Format("DRIVER=MySQL ODBC \
3.51;Driver;SERVER=localhost;USER=%s;PASSWORD=%s", argv[1], argv[2]);
	
	CDatabase db;
	
	db.OpenEx(strConnect);

	db.ExecuteSQL("DROP DATABASE IF EXISTS testadd");
	db.ExecuteSQL("CREATE DATABASE testadd");
	db.ExecuteSQL("USE testadd");
	
	db.ExecuteSQL("CREATE TABLE testadd (field1 VARCHAR(255) NOT NULL, field2 VARCHAR(255) NOT NULL, field3 int DEFAULT NULL, field4 VARCHAR(50) DEFAULT NULL, PRIMARY KEY(field1, field2))");
	
	CTestAdd r(db);
	r.Open(-1, _T("SELECT field1, field2, field3, field4 FROM testadd WHERE field1 = '1' AND field2 = '2'"));
	
	bool bFailed = false;
	if(r.IsBOF() || r.IsEOF())
	{
		try
		{
			r.AddNew();
			r.m_str1 = _T("1");
			r.m_str2 = _T("2");
			r.Update();
		}
		catch(CDBException *p)
		{
			p->Delete();
			bFailed = true;
		}
	}
	r.Close();
	
	if(bFailed)
	{
		printf(_T("Operation failed.\r\n"));
	}
	
	return 0;
}

Suggested fix:
If the ODBC cursor library (ODBCCR32.DLL, distributed with Windows) is used, it sends an INSERT statement with only the provided fields, allowing the server to fill in the default values for the rest:

INSERT INTO `testadd`(`field1`,`field2`) VALUES ('1','2')

Ideally, myODBC should do the same.  At the very least, it should immediately return an error condition if the INSERT is attempted with unbound fields rather than constructing and executing an invalid SQL string.
[4 Oct 2007 10:32] Tonci Grgin
Hi Craig and thanks for your report. Verified as described with test case attached on MySQL server 5.0.50 on WinXP Pro SP2 host using MyODBC 3.51.21 rev.803.

071004 12:26:33	      2 Connect     root@localhost on 
		      2 Query       SET SQL_AUTO_IS_NULL = 0
071004 12:26:35	      2 Query       DROP DATABASE IF EXISTS testadd
071004 12:26:36	      2 Query       CREATE DATABASE testadd
071004 12:26:37	      2 Query       USE testadd
071004 12:26:39	      2 Query       CREATE TABLE testadd (field1 VARCHAR(100) NOT NULL, field2 VARCHAR(100) NOT NULL, field3 int DEFAULT NULL, field4 VARCHAR(50) DEFAULT NULL, PRIMARY KEY(field1,field2))
071004 12:26:42	      2 Query       SELECT field1, field2, field3, field4 FROM testadd WHERE field1 = '1' AND field2 = '2'
071004 12:26:46	      2 Query       INSERT INTO `testadd`(`field1`,`field2`,`field3`,`field4`) VALUES ('1','2',,'')
071004 12:26:56	      2 Quit
[4 Oct 2007 10:32] Tonci Grgin
DM trace file

Attachment: DMTrace.zip (application/zip, text), 2.39 KiB.

[5 Oct 2007 11:57] Bogdan Degtyariov
Patch

Attachment: patch31246.diff (application/octet-stream, text), 2.21 KiB.

[5 Oct 2007 15:14] Bogdan Degtyariov
Patch with the test case that checks the result

Attachment: patch31246v2.diff (application/octet-stream, text), 2.92 KiB.

[5 Oct 2007 22:04] Bogdan Degtyariov
The fix for this bug was committed to the source repository, and will be
in 3.51.21.
[12 Oct 2007 13:07] MC Brown
A note has been added to the 3.51.21 changelog: 

When using a rowset/cursor and add a new row with a number of fields, subsequent rows with fewer fields will include the original fields from the previous row in the final INSERT statement.