Обсуждение: Regression tests for functions of info.c
Hi all, Please find attached a patch adding regression tests for the functions in info.c. The following functions, that use hardcoded column names for their results are checked: - SQLGetTypeInfo - SQLTables - SQLColumns - SQLSpecialColumns - SQLStatistics - SQLPrimaryKeys - SQLForeignKeys - SQLProcedureColumns - SQLTablePrivileges Note that the number of columns used for some of those functions, like SQLGetTypeInfo, has increased since 0x0300, so those regression tests are not compatible with versions of odbc older than 0x0300 and would need an alternative output. The patch attached does not have the alternative output for ODBCVER < 0x0300. Do we honestly need it? Regards -- Michael
Вложения
On 03/13/2014 03:52 PM, Michael Paquier wrote: > Please find attached a patch adding regression tests for the functions > in info.c. The following functions, that use hardcoded column names > for their results are checked: > - SQLGetTypeInfo > - SQLTables > - SQLColumns > - SQLSpecialColumns > - SQLStatistics > - SQLPrimaryKeys > - SQLForeignKeys > - SQLProcedureColumns > - SQLTablePrivileges > Note that the number of columns used for some of those functions, like > SQLGetTypeInfo, has increased since 0x0300, so those regression tests > are not compatible with versions of odbc older than 0x0300 and would > need an alternative output. Hmm, that's fine as far as it goes, but surely we need to test that they actually return correct data, not just the column names? I envisioned the "catalogfunctions" test case to eventually cover these, although at the moment it only tests SQLTables. The ODBC standard calls these "catalog functions", per http://msdn.microsoft.com/en-us/library/ms711722%28v=vs.85%29.aspx. From that list, you're missing SQLColumnPrivileges and SQLProcedures. > The patch attached does not have the > alternative output for ODBCVER < 0x0300. Do we honestly need it? Certainly not after you cleanup that removes #ifdefs :-). If we still care for ODBCVER < 0x0300 in 9.3, we could just not merge this until the cleanup, but I don't think we care. If someone's building with ODBCVER < 0x0300, he can just ignore the failure. - Heikki
On Thu, Mar 13, 2014 at 11:01 PM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > Hmm, that's fine as far as it goes, but surely we need to test that they > actually return correct data, not just the column names? The point of this test was to cover the issue related to the column names: e: http://www.postgresql.org/message-id/5315E622.2010904@ntlworld.com. If we want to test all those functions by checking their data, we will need roughly to either choose and/or create one/two table(s) with pk and fk, a procedure, a data type, and an index for SQLStatistics. I imagine that raw data will be more helpful than querying catalog in this case except for the data type. > I envisioned the "catalogfunctions" test case to eventually cover these, > although at the moment it only tests SQLTables. The ODBC standard calls > these "catalog functions", per > http://msdn.microsoft.com/en-us/library/ms711722%28v=vs.85%29.aspx. From > that list, you're missing SQLColumnPrivileges and SQLProcedures. I lack of caffeine. -- Michael
On Thu, Mar 13, 2014 at 11:01 PM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > On 03/13/2014 03:52 PM, Michael Paquier wrote: >> >> Please find attached a patch adding regression tests for the functions >> in info.c. The following functions, that use hardcoded column names >> for their results are checked: >> - SQLGetTypeInfo >> - SQLTables >> - SQLColumns >> - SQLSpecialColumns >> - SQLStatistics >> - SQLPrimaryKeys >> - SQLForeignKeys >> - SQLProcedureColumns >> - SQLTablePrivileges >> Note that the number of columns used for some of those functions, like >> SQLGetTypeInfo, has increased since 0x0300, so those regression tests >> are not compatible with versions of odbc older than 0x0300 and would >> need an alternative output. > > > Hmm, that's fine as far as it goes, but surely we need to test that they > actually return correct data, not just the column names? > > I envisioned the "catalogfunctions" test case to eventually cover these, > although at the moment it only tests SQLTables. The ODBC standard calls > these "catalog functions", per > http://msdn.microsoft.com/en-us/library/ms711722%28v=vs.85%29.aspx. From > that list, you're missing SQLColumnPrivileges and SQLProcedures. OK, I have been working more on this patch and have integrated all the tests directly in catalogfunctions. I have also extended as well the functions in common.c to be able to print results for arrays of column IDs. I found that this became necessary as the current logic is able to print results only for all the columns, and there are some data like table OID or user name that we do not want as output in the test suite. Also, I think that I have found a bug, or let's say a limitation with SQLColumnPrivileges, which does not work with unixodbc 2.3.2. I tried to use this function on LInux and OSX, each time it failed with the following error: ! IM001=[unixODBC][Driver Manager]Driver does not support this function The patch I am attaching has some commented code for the test case of SQLColumnPrivileges, but I disabled it. For example I tried to call it like that but it failed: rc = SQLColumnPrivileges(hstmt, NULL, 0, (SQLCHAR *) "public", SQL_NTS, (SQLCHAR *) "testtab1", SQL_NTS, (SQLCHAR *) "id", SQL_NTS); Comments and feedback are welcome. -- Michael
Вложения
On 03/18/2014 03:08 PM, Michael Paquier wrote: > On Thu, Mar 13, 2014 at 11:01 PM, Heikki Linnakangas > <hlinnakangas@vmware.com> wrote: >> On 03/13/2014 03:52 PM, Michael Paquier wrote: >>> >>> Please find attached a patch adding regression tests for the functions >>> in info.c. The following functions, that use hardcoded column names >>> for their results are checked: >>> - SQLGetTypeInfo >>> - SQLTables >>> - SQLColumns >>> - SQLSpecialColumns >>> - SQLStatistics >>> - SQLPrimaryKeys >>> - SQLForeignKeys >>> - SQLProcedureColumns >>> - SQLTablePrivileges >>> Note that the number of columns used for some of those functions, like >>> SQLGetTypeInfo, has increased since 0x0300, so those regression tests >>> are not compatible with versions of odbc older than 0x0300 and would >>> need an alternative output. >> >> >> Hmm, that's fine as far as it goes, but surely we need to test that they >> actually return correct data, not just the column names? >> >> I envisioned the "catalogfunctions" test case to eventually cover these, >> although at the moment it only tests SQLTables. The ODBC standard calls >> these "catalog functions", per >> http://msdn.microsoft.com/en-us/library/ms711722%28v=vs.85%29.aspx. From >> that list, you're missing SQLColumnPrivileges and SQLProcedures. > OK, I have been working more on this patch and have integrated all the > tests directly in catalogfunctions. I have also extended as well the > functions in common.c to be able to print results for arrays of column > IDs. I found that this became necessary as the current logic is able > to print results only for all the columns, and there are some data > like table OID or user name that we do not want as output in the test > suite. Thanks, pushed. > Also, I think that I have found a bug, or let's say a limitation with > SQLColumnPrivileges, which does not work with unixodbc 2.3.2. I tried > to use this function on LInux and OSX, each time it failed with the > following error: > ! IM001=[unixODBC][Driver Manager]Driver does not support this function > The patch I am attaching has some commented code for the test case of > SQLColumnPrivileges, but I disabled it. > > For example I tried to call it like that but it failed: > rc = SQLColumnPrivileges(hstmt, NULL, 0, > (SQLCHAR *) "public", SQL_NTS, > (SQLCHAR *) "testtab1", SQL_NTS, > (SQLCHAR *) "id", SQL_NTS); > Comments and feedback are welcome. Hmm, it seems that we don't publish the SQLColumnPrivileges function to the driver manager. See PGAPI_GetFunctions30 in odbcapi30.c: > if (ci->drivers.lie) > SQL_FUNC_ESET(pfExists, SQL_API_SQLCOLUMNPRIVILEGES); /* 56 */ So you have to specify the "Lie" configuration option to enable SQLColumnPrivileges. Got to love the name of that option :-). I wonder why. Perhaps because PostgreSQL used to not support per-column permissions, and no-one's gotten around to implement SQLColumnPrivileges properly since they was added to the backend? - Heikki