Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ticket 8848: Fix various typos causing issues on db migration #247

Open
wants to merge 3 commits into
base: testlink_1_9_20_fixed
Choose a base branch
from

Conversation

heuj
Copy link
Contributor

@heuj heuj commented Jan 20, 2020

No description provided.

@squash-labs
Copy link

squash-labs bot commented Jan 20, 2020

Manage this branch in Squash

Test this branch here: https://heujpatch-6-6zbko.squash.io

@atisne
Copy link
Contributor

atisne commented Jan 21, 2020

being_exec_ts (read begin_exec_ts) and end_exec_ts are NOT NULL, so they must have a default value.
Not sure current_timestamp is the best option.
Moreover, zero dates are frequently forbidden (NO_ZERO_IN_DATE,NO_ZERO_DATE).

"Virtual" zero date '1970-01-01 00:00:01' may work.

@fmancardi
Copy link
Contributor

This is a new table, do not think the default value is Mandatory for the NOT NULL command, the value will be handled by the application, not by the DBMS.
Using virtual zero date ad UNIX EPOCH is something I try to avoid.

@atisne
Copy link
Contributor

atisne commented Jan 21, 2020

The creation fails for me:

mysql> CREATE TABLE /*prefix*/baseline_l1l2_context (
    ->   id int(10) unsigned NOT NULL AUTO_INCREMENT,
    ->   testplan_id int(10) unsigned NOT NULL DEFAULT '0',
    ->   platform_id int(10) unsigned NOT NULL DEFAULT '0',
    ->   being_exec_ts timestamp NOT NULL,
    ->   end_exec_ts timestamp NOT NULL,
    ->   creation_ts timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
    ->   PRIMARY KEY (id),
    ->   UNIQUE KEY udx1 (testplan_id,platform_id,creation_ts)
    -> ) DEFAULT CHARSET=utf8;
ERROR 1067 (42000): Invalid default value for 'end_exec_ts'

May it be something else?
No problem on your side?

@fmancardi
Copy link
Contributor

As usual is an issue regarding different MySQL config.
I'm using MySQL 5.7.26 on MAMP PRO and ... obviously all worked ok, dammit :(
Going to rethink the definition.
I'm sorry

@atisne
Copy link
Contributor

atisne commented Jan 21, 2020

FYI, I tested using the Dockerfile provided in the sources.

@heuj
Copy link
Contributor Author

heuj commented Jan 21, 2020

Hi,
there are two issues addressed with this pull request, sry for the confusion, I did not explain carefully enough:

  1. the default NULL problem
    On my database there are two options which work on create:

Either allow NULL and have NULL as default:
--> begin_exec_ts timestamp NULL DEFAULT NULL
or do not allow null and use current time as default
--> begin_exec_ts timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP

both work for me, in the pull request I chose the second option to keep consistency with all other timestamps in the sql file
[edit: using MySQL version: 5.7.28-0 ubuntu0.18.04.4 ]

  1. I think there is a typo in begin_exec_ts (was being_exec_ts): this leads to db access error on my side when using testlink later on, as in the php code it's called begin_exec_ts

@fmancardi
Copy link
Contributor

The column definitions will be:
begin_exec_ts timestamp NOT NULL,
end_exec_ts timestamp NOT NULL,

without default, because this is valid (I've checked with one DB Admin).
This is valid when creating a table, but not when altering a table.

@heuj
Copy link
Contributor Author

heuj commented Jan 21, 2020

Understood.
Maybe its specific to mysql version 5.7.28-0

Creating a table with more than one timestamp NOT NULL and without DEFAULT, like e.g.

CREATE TABLE mytesttable (
  ts_1 timestamp NOT NULL,
  ts_2 timestamp NOT NULL
);

leads to
Error in query (1067): Invalid default value for 'ts_2'

in case db is in strict mode which it is by default on my system. This can be changed by adding
sql_mode=NO_ENGINE_SUBSTITUTION to the mysql config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants