search

FirebirdSQL / firebird

Firebird server, client and tools
https://www.firebirdsql.org/
1.21k stars 209 forks source link

Cannot update column type from numeric(15,2) to numeric(15,4) #6867

Open MarvinKlein1508 opened 3 years ago

MarvinKlein1508 commented 3 years ago

I want to change a column type from type TMONEYto TMONEYLONG. This are custom defined domains.

CREATE DOMAIN TMONEY AS 
 DECIMAL(15,2)
 DEFAULT 0
;
CREATE DOMAIN TMONEYLONG AS 
 NUMERIC(15,4)
 DEFAULT 0
;

However the update always fails with this error message:

unsuccessful metadata update
ALTER TABLE ARTIKEL failed
New scale specified for column ARTI_N_STAFFELR1 must be at most 2.
while executing:
ALTER TABLE ARTIKEL 
  ALTER ARTI_N_STAFFELR1 TYPE TMONEYLONG

When I create a new column of type TMONEYLONG I can insert all data from the other column without any issues. I tried to set the old column to NULL then and then change the column type again but this throws me the same error message as before.

dyemanov commented 3 years ago

Firebird neither changes nor even checks the stored values when some column is altered. The benefit is that ALTERs are instant and non-blocking. The downside is that only safe conversions are allowed. Changing from NUMERIC(15, 2) to NUMERIC(15, 4) reduces the integral part from 13 to 11 decimal digits and thus the existing values may potentially not fit into the new definition, hence you see an error.

A possible workaround could be to define the column as NUMERIC(17, 4) to compensate the precision loss. Or just create a different column as you already tried.

MarvinKlein1508 commented 3 years ago

@dyemanov changing TMONEYLONG to NUMERIC(17,4) works and allows me to change the column type. However, all columns needs more storage now which is not necessary in our use case.

Creating a different column doesn't work in our case because the original column as dependencies in many procedures and views.

I think you should allow people to always exchange the datatype of a column when all data within the column is set to NULL. This makes it so much easier for people to change existing columns.

aafemt commented 3 years ago

However, all columns needs more storage now which is not necessary in our use case.

No, they doesn't because of records compression.

This makes it so much easier for people to change existing columns.

I personally don't see support for bad database designers as "a good thing".

dyemanov commented 3 years ago

In both cases storage is the same - 8 bytes per value.

But generally I agree it's worth checking all values for being compatible with the new stricter definition, instead of rejecting this unconditionally. It has a cost -- the whole table must be read -- but perhaps it's acceptable.

aafemt commented 3 years ago

the whole table must be read...

"...in exclusive mode" which makes things worse.

hvlad commented 3 years ago

But generally I agree it's worth checking all values for being compatible with the new stricter definition, instead of rejecting this unconditionally. It has a cost -- the whole table must be read -- but perhaps it's acceptable.

Only if explicitly required by the user, please. For ex. with extension to the ALTER TABLE syntax to force such a check.

hvlad commented 3 years ago

the whole table must be read... "...in exclusive mode" which makes things worse.

PR is enough in this case. Same as when build an index.

MarvinKlein1508 commented 3 years ago

I totally agree with you. There should be a method to enable this kind of feature.

In our case we need to update an old Firebird 2.5 database to fit our needs which we didn't know of back in the past.

Our most common changes are the following:

Doing this changes manually is hard for us. Because as mentioned previously, a column might have around 100 dependencies from different procedures, triggers and views. And I don't want to create a new database everytime and migrate all data.

reevespaul commented 3 years ago

On Wed, 23 Jun 2021 02:22:14 -0700 Dimitry Sibiryakov @.***> wrote:

However, all columns needs more storage now which is not necessary in our use case.

No, they doesn't because of records compression.

This makes it so much easier for people to change existing columns.

I personally don't see support for bad database designers as "a good thing".

We certainly do not want to support bad database design but I suspect in most cases the design was adequate for the requirements of the day and the options available. However times and requirements change. I think all of us must look back and sometimes think 'If I was to do that today I would do things differently.'

Increasing firebird's ability to allow a database design to evolve has to be a good thing for firebird - both to keep existing users and add new ones.

Paul --

Paul Reeves http://www.ibphoenix.com Supporting users of Firebird

aafemt commented 3 years ago

I suspect in most cases the design was adequate for the requirements of the day and the options available. However times and requirements change.

Yes, it is natural process but in such cases creation of a new database from scratch for new requirements and pumping data from old one is the best approach that guarantee no problem appear afterward with missed dependencies like variables based on domain or column.

Pumping data with ESonEDS is quite effective as you know.

MarvinKlein1508 commented 3 years ago

@aafemt

I don't think creating an enitre new database everytime is super easy to manage. I migrated our last database from FB2.5 to Firebird 4 a few weeks ago. While I did this I changed the encoding from ISO88591 to UTF8. The backup tools etc. were really painful to use for this process so I ended up creating all tables, views, triggers and procedures by hand which took me a around a week. Not to mention that all import tools have failed so I needed to write a custom tool for our data migration.

I don't want to go through this entire process again just to change the type of a column which is compatible by default.

aafemt commented 3 years ago

A week? It doesn't sound right. If you have well-organized development, then you have a "master script" for database creation in your version control system. The migration is actually a fork of a new branch in VCS and fixing incompatibilities found during the script's run on new server version. There must be something really wrong if you wasted a whole week on that.

Data migration through Execute Statement on External Data Source is also a script that simply feeds data from old server and inserts into new tables. Its development may be long if you have thousands of tables but as soon as it is complete, running it is a matter of one double-click and can be performed as many times as much errors it encounter without affecting current production server (because test runs are performed on a copy).

PS: I intentionally doesn't mention "smooth migration" with replication usage.

arvanus commented 3 years ago

I would relate this to https://groups.google.com/g/firebird-support/c/7YOx9MOw_ts/m/ROIki33IBQAJ "Conversion from base type XXX to YYY is not supported." When Firebird can't convert safely the data, it could allow the user to specify how to do the conversion

mrotteveel commented 3 years ago

To make the previous comment more explicit: Maybe Firebird can add something similar to PostgreSQL's USING option of their alter column clause (see https://www.postgresql.org/docs/13/sql-altertable.html). This allows you to specify an expression to apply for conversion to the new data type.

dyemanov commented 3 years ago

This does not fit our ALTER TABLE architecture. USING implies the conversion happens during ALTER TABLE which is true for PGSQL but wrong for Firebird, as we convert the old data type to the new one while reading the field (old record format is converted to the latest one) -- this is why they should be compatible. We may end with three options to consider:

The last option generally kills the major DDL benefit we have as compared to other databases, but perhaps it will find its users.

mrotteveel commented 3 years ago

An alternative would be to enhance the current record conversion system to also be able to apply expressions when migrating from one version to another, but then the next problem is: what if the conversion is invalid.

aafemt commented 3 years ago

Why consider these as options and not execution plans? If USING expression is used - UPDATE strategy is forced, if change may be dangerous - CHECK is performed (if confirmation verb used as Vlad suggested), otherwise it works as now.

hvlad commented 3 years ago

Don't forget about indices on such column - it should be disabled before explicit conversion and rebuild after. Same about constraints. It is not as easy task as it seems.

grzegorzzyla commented 2 years ago

https://github.com/FirebirdSQL/firebird/issues/1627#issuecomment-826186202