Hi Udai,
as you may have expected I have some comments regarding your patch.
@TEST 1355: Have you tried 'DOUBLE PRECISION' instead of 'DOUBLE'? Having said that 'REAL' is also fine even if its resolution is smaller. Also add to the comment in test case that mssql should be enable when CASTOR-2584 is resolved.
@TEST 972: This error seams to be related to CASTOR-2221. I assigned this issue to you as you should be able to fix this. For the time being exclude the test and add a comment to the test class why it has been excluded. For example: Temporary disabled test for mssql until CASTOR-2221 is resolved.
@TEST 2527: Can you try to use 'DATETIME' instead of 'TIMESTAMP' datatype?
@TEST 2550: Add to comment that mssql is also excluded because it does not support sequence.
@TEST 30, 31, 87: Create a new issue 'Use ANSI outer join operators for Microsoft SQL engine'. Add comment to the new issue that this 3 tests relate to the issue. Exclude the issues from test execution for mssql and add comment to the test case that they have been excluded because of the this new issue.
@cpactf-conf.xml: You don't need to keep the old datasource definitions or comment the class change in the file.
@sql scripts: I though a bit about the sql scripts you provided. On one hand it is very good that you added the conditional execution of the scripts but on the other they will make another improvement we have in mind much harder. Let me explain on this. Later on we intend to use DDLGEN to generate create and drop scripts for cpactf. As mysql offers such a simple 'IF EXISTS' clause we use it for mysql but this iscomplicated or imposible for most of the other database engines. I would also prefer if we can omit 'USE' and 'GRANT' statements from the script and only include what's absolutely required. For example the script of TEST 1158:
USE [cpactf]
GO
IF NOT EXISTS (SELECT * FROM sysobjects WHERE id = OBJECT_ID(N'[test1002_prod]') AND OBJECTPROPERTY(id, N'IsUserTable') = 1)
BEGIN
CREATE TABLE [test1002_prod](
[id] [int] NOT NULL,
[name] [varchar](200) NOT NULL
) ON [PRIMARY]
END
GO
INSERT [test1002_prod] ([id], [name]) VALUES (1, CONVERT(TEXT, N'This is the test object.'))
should better look like:
DROP TABLE [test1002_prod]
GO
CREATE TABLE [test1002_prod](
[id] int NOT NULL,
[name] varchar(200) NOT NULL
)
GO
INSERT [test1002_prod] ([id], [name]) VALUES (1, 'This is the test object.')
GO
The reason for this is, that it will be much easier to compare the output of DDLGEN with the simpler scripts. Having said that I am aware that all drop statements will cause errors when tables are not available. Same happens with create statements when there already is a table with the same name. According to the drop statements for constraints you could check if mssql offers something similar to oracle (DROP TABLE <name> CASCADE CONSTRAINTS).
I have to note that I'm aware that my comments are a bit picky and that I could have fixed most of the things mentioned in a shorter time then writing this comment. Background is that I think it will help you more if you know about all the things and fix them yourself. My idea is that it should enable you to provide patches with less of this small things next time, which I hope you will do
.
Thank you for your great work.
Ralf
Initially I configured the tests and ran them against MySQL 5.0 using to check if the behavior of my local repository.
Old tests run without any error/fail.
New tests:- TEST 1355, 1379, 31, 972 failed, TEST 2550 no mysql script, Test 30 OQLPassThrough tests fails, Test 87 fails(7 fails 8 errors)
Now I will setup and run these tests against MS SQL Server 2008 Express