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

Regression: incorrect calculation of byte-length for view columns [CORE5049] #5336

Closed
firebird-issue-importer opened this issue Dec 16, 2015 · 16 comments

Comments

@firebird-issue-importer

Submitted by: @abzalov

create database 'c:\test.fdb' page_size 16384 user 'SYSDBA' password 'masterkey' default character set UTF8 collation UTF8;

connect 'c:\test.fdb' user 'SYSDBA' password 'masterkey';

CREATE OR ALTER VIEW test
(
column1,
column2,
column3
)
AS
SELECT
cast(rdb$character_set_name as varchar(2000)),
cast(rdb$character_set_name as varchar(2000)),
cast(rdb$character_set_name as varchar(2000))
FROM
rdb$database;

Statement failed, SQLSTATE = 54000
unsuccessful metadata update
-new record size of 96028 bytes is too big
-TABLE TEST

Commits: 80a29fe FirebirdSQL/fbt-repository@b5b9733

====== Test Details ======

Confirmed:
1) FAULT on WI-V3.0.0.32208.
2) SUCCESS on LI-V3.0.0.32233, Rev: 62699.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @dyemanov

I'm not sure the limit can be easily removed for views. But there seems to be a bug - v2.5 calculates the format length as 24KB (2000 * 3 * 4) while v3.0 calculates it as 96KB. So this example works in v2.5 but fails in v3.0.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @abzalov

This check makes sense when selecting from the view and record size is greater than limit.
But this check does not make sense (except protection against the fool) when creating a view, because developer knows what columns will be requested in different situations.

In FB <= 2.5 it worked well, and it was right, IMHO.

By the way, the same applies to checks exceeding 255 context when creating the view. Starting with 2.5, such view can not create. The 1.5 was more correct - it was possible to create, and checking is made at selecting time.

---

Проверка имеет смысл при выборке из представления, на предмет превышения размера записи выборки.
Но она не имеет смысла (кроме защиты от дурака) при создании представления, т.к. разработчик знает какие колонки будут запрошены в тех или иных вариациях.

В FB <= 2.5 это работало так, и это было более правильно, IMHO.

Кстати, тоже самое касается проверки на превышение 255 контекстов при создании представления. Начиная с 2.5 такие представления вообще нельзя создать. В 1.5 было более правильно - создать было можно, а проверка производилась по факту выборки.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @sim1984

It is obvious that the length of the format is evaluated twice
3 * 2000 * 4 = 24K, an error message says that the length is equal to 96K that just 24K * 4 = 96K.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @abzalov

Wrong count size is half woes.
When counting is fixed, it still would be impossible to create a view exceeding 64Kb of columns.
This will be a much larger problem.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @dyemanov

Maybe, however that "larger" problem will not be a regression.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @abzalov

Yes, really, in 2.5 it is also impossible to create a view that excess of 64Kb.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @abzalov

Will it be possible to remove this restriction (or at least check) when creating views, as well as limit (or at least check) associated with the error:
"Too many Contexts of Relation/Procedure/Views Maximum allowed is 256"?
I think I can found funding for this work.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @dyemanov

I suppose views need a pre-defined format because triggers are supported for views and OLD/NEW pseudo-records are limited by 64KB. There may be other hidden reasons as well. So this limit is unlikely to be removed in the near future.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @abzalov

OK...

Сan I expect that a bag with incorrect size counting will be fixed to Release?

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 16, 2015

Commented by: @dyemanov

Sure.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 18, 2015

Modified by: @dyemanov

summary: Remove the checking record size at create view - new record size of NNN bytes is too big => Regression: incorrect calculation of byte-length for view columns

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 18, 2015

Modified by: @dyemanov

assignee: Adriano dos Santos Fernandes [ asfernandes ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 18, 2015

Modified by: @dyemanov

Fix Version: 3.0 RC2 [ 10048 ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 18, 2015

Modified by: @asfernandes

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 18, 2015

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: No test => Done successfully

Test Details: Confirmed:
1) FAULT on WI-V3.0.0.32208.
2) SUCCESS on LI-V3.0.0.32233, Rev: 62699.

@firebird-issue-importer
Copy link
Author

firebird-issue-importer commented Dec 18, 2015

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Closed [ 6 ]

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

No branches or pull requests

2 participants