Fix migration with pgsql do not work correctly

[aconits]

Fix

Meet probleme with llx_cronjob and the column params
In Dolibarr 3.7 the structure was declare as :

dolibarr/htdocs/install/mysql/tables/llx_cronjob.sql

Line 32 in e61bcd0

params text NOT NULL,

In the next version (3.8) we had this :

dolibarr/htdocs/install/mysql/migration/3.7.0-3.8.0.sql

Line 37 in 71818da

ALTER TABLE llx_cronjob MODIFY COLUMN params text NULL;

But the conversion give us a bad request : ALTER TABLE llx_cronjob ALTER COLUMN params TYPE text DROP NOT NULL;

Here examples of conversion :

-- ALTER TABLE llx_cronjob MODIFY COLUMN params text NULL; replaced by --
ALTER TABLE llx_cronjob ALTER COLUMN params TYPE text;
ALTER TABLE llx_cronjob ALTER COLUMN params DROP NOT NULL;

-- ALTER TABLE llx_cronjob MODIFY COLUMN params VARCHAR(200) DEFAULT NULL; replaced by --
ALTER TABLE llx_cronjob ALTER COLUMN params TYPE VARCHAR(200);
ALTER TABLE llx_cronjob ALTER COLUMN params SET DEFAULT NULL;

-- ALTER TABLE llx_cronjob MODIFY COLUMN params VARCHAR(200) DEFAULT 'test'; replaced by --
ALTER TABLE llx_cronjob ALTER COLUMN params TYPE VARCHAR(200);
ALTER TABLE llx_cronjob ALTER COLUMN params SET DEFAULT 'test';

[aconits]

@eldy
Do you think it's appropriate to write again this request ALTER TABLE llx_cronjob MODIFY COLUMN params text NULL; in the migration script between 6.0 and 7.0 in branch 7.0 ?
Or use the repair.sql file ?

[aconits]

I pushed one more commit to match request like this :

-- ALTER TABLE llx_cronjob MODIFY COLUMN params VARCHAR(255) replaced by --
ALTER TABLE llx_cronjob ALTER COLUMN params TYPE VARCHAR(255);

[aconits]

New commit (99a3269) to convert sql requests like this :

-- ALTER TABLE llx_opensurvey_sondage ADD COLUMN fk_user_creat integer NOT NULL DEFAULT 0; replaced by --
ALTER TABLE llx_opensurvey_sondage ADD COLUMN fk_user_creat integer NOT NULL;
ALTER TABLE llx_opensurvey_sondage ALTER COLUMN fk_user_creat SET DEFAULT 0;

or

-- ALTER TABLE llx_opensurvey_sondage ADD COLUMN status integer DEFAULT 1; replaced by --
ALTER TABLE llx_opensurvey_sondage ADD COLUMN status integer;
ALTER TABLE llx_opensurvey_sondage ALTER COLUMN status SET DEFAULT 1;

I took examples from migration script 5.0.0-6.0.0.sql

[aconits]

00f1027 to match this:

-- ALTER TABLE llx_product_price ADD COLUMN multicurrency_tx numeric(24,8) DEFAULT 1; replaced by --
ALTER TABLE llx_product_price ADD COLUMN multicurrency_tx numeric(24,8);
ALTER TABLE llx_product_price ALTER COLUMN multicurrency_tx SET DEFAULT 1;

request found in migration script 6.0.0-7.0.0.sql in branch 7.0

[aconits]

82eb0d0

-- ALTER TABLE llx_website_page ADD COLUMN type_container varchar(16) NOT NULL DEFAULT 'page'; replaced by --
ALTER TABLE llx_website_page ADD COLUMN type_container varchar(16) NOT NULL;
ALTER TABLE llx_website_page ALTER COLUMN type_container SET DEFAULT 'page';

-- ALTER TABLE llx_expensereport_ik ADD COLUMN ikoffset numeric DEFAULT 0 NOT NULL; replaced by --
ALTER TABLE llx_expensereport_ik ADD COLUMN ikoffset numeric DEFAULT 0;
ALTER TABLE llx_expensereport_ik ALTER COLUMN ikoffset SET NOT NULL;

request found in migration script 6.0.0-7.0.0.sql in branch 7.0

[eldy]

The trouble was into the migration file. The line
ALTER TABLE llx_cronjob MODIFY COLUMN params text NULL;
is not a line that match a syntax allowed in top of migration files. It must be split into 2 requests:
One to change type of column and the other to remove the not null.
I don't want to change the convert function because i want to keep the number of allowed syntax in migration script low with only one way to do something. That's why there is some example at the top of the migration-xxxx.sql files. And we must just follow examples and no use other syntax.

We can add the correct syntax into script migration script 6.0.0-7.0.0.sql in branch 7.0 if you want but by using the suggested syntaxes only.

So it hsould be something like this :

ALTER TABLE llx_cronjob MODIFY COLUMN params text;
-- VMYSQL4.3 ALTER TABLE llx_cronjob MODIFY COLUMN params text NULL;
-- VPGSQL8.2 ALTER TABLE llx_cronjob ALTER COLUMN params DROP NOT NULL;

[aconits]

Do you mean this PR need to be closed and open another one to rewrite all sql requests I had found ?

ALTER TABLE llx_cronjob MODIFY COLUMN params text NULL;
ALTER TABLE llx_cronjob MODIFY COLUMN params VARCHAR(255);
ALTER TABLE llx_opensurvey_sondage ADD COLUMN fk_user_creat integer NOT NULL DEFAULT 0;
ALTER TABLE llx_opensurvey_sondage ADD COLUMN status integer DEFAULT 1;
ALTER TABLE llx_product_price ADD COLUMN multicurrency_tx numeric(24,8) DEFAULT 1;
ALTER TABLE llx_website_page ADD COLUMN type_container varchar(16) NOT NULL DEFAULT 'page';
ALTER TABLE llx_expensereport_ik ADD COLUMN ikoffset numeric DEFAULT 0 NOT NULL;

Limit the syntax is a good practice, I'm sure, but this mean you should check each PR adding sql request

[eldy]

"Limit the syntax is a good practice, I'm sure, but this mean you should check each PR adding sql request"
This is true. This is why I also introduced a PHPUnit test (CodingSQLTest) that check the sql syntax of sql requests, but some undesirable syntax can still pass the test.

We don' need to rewrite all sql requests. A lot of one are good. For example
ALTER TABLE llx_cronjob MODIFY COLUMN params VARCHAR(255);
is allowed.
Only requests that does not follow the examples = requets that generate errors in migration must be rewrite to have the migration not failed anymore.