MySQL Bugs: #31246: Opening rowset with extra fields leads to incorrect SQL INSERT

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:	Windows (XP SP2)
Assigned to:	Bogdan Degtyariov	CPU Architecture:	Any

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.

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

DM trace file

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

Patch

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

Patch with the test case that checks the result

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

The fix for this bug was committed to the source repository, and will be
in 3.51.21.

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.