cap max column sizes for mssql and oracle#10297
cap max column sizes for mssql and oracle#10297willus10245 wants to merge 2 commits intoerlang:masterfrom
Conversation
|
|
CT Test Results 2 files 15 suites 4m 35s ⏱️ Results for commit e96e73d. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
|
what needs to be done to move this forward? I see the failing SBOM task, but I'm not sure there's anything I can do to fix that? We cannot upgrade past OTP 26 on our elixir app without fixing this odbc issue. |
I have a task to review this. I also wanted to give it a good thought and check the documentation regarding those column limits, what does each database doc suggest to do about it. If you override the size, what will happen next, will it trim the data? Why allocating 4 GB is a problem, do you have this much data or does it allocate too much empty memory? Are you using 32-bit OS and Erlang? SBOM is fixed in latest master and maint, so don't worry about it, it will need to be rebased at some point. P.S. There also was another merge request in this line #7000 , merged earlier then reverted as causing too much problems, so i'm asked to act cautiously 😊 |
|
Got it. Thank you for the thoughtful response! I appreciate the thorough consideration. The change in #7000 is what I am attempting to fix. You said that change was reverted? But the change introduced there is still on master. |
I did not check whether it actually was reverted or just blamed and named as a candidate for reversal. Maybe this ticket will become its reversal. Anyway i will be looking at both this and 7000, to get something that works. And also the team was interested in some way to automated test this. |
|
@kvakvs @willus10245 No the change was not reverted, it was a consideration for perhaps putting odbc application in a better state for all backends if no one could come up with a real solution which I think is preferable. And of course it would also be fantastic if @willus10245 with the support of @kvakvs could provide a way testing it maybe with a docker solution, as alas I think our windows set up has become rotten and we no longer have a oracle database either so I believe only our postgreSQL test are actually run. |
IngelaAndin
added
help wanted
|
@willus10245 Alas "kvakvs" stopped working for as and will not have time to look into a better solution. Any way this code is really old and a little "AI- googling" results in the following ... "In SQL Server, varchar(8000) is the maximum size for in-row storage, while varchar(max) allows up to 2GB of data, often requiring specific handling in ODBC when exceeding 8000 characters. Using varchar(max) enables storing over 8000 characters, but data exceeding this typically shifts from in-row to out-of-row storage. " ... which can explain the current max value. Maybe a small improvement could be to make the max value configurable. A little more work could perhaps make the code consider varchar(max). We will of course accept things that are clearly an improvement as long as it remains a supported application. |
Oracle and MSSQL can return column sizes of upwards of 4GB, which can cause memory allocation errors when trying to allocate that much buffer space. This change keeps the prior fix for Postgres compatibility, but caps large column sizes for Oracle and MSSQL databases.
willus10245
force-pushed
the
fix-odbc-memory-allocation-error
branch
from
c48d8ed to
e96e73d
Compare
|
@willus10245 We will revaluate this PR after next release candidate that has already created a code freez. Although we will deprecate the odbc application we in OTP-29 we want it to be in as good shape as possible and there is an interest from the community to make it an hex-package. |
SQLDescribeCol can report 0 or a very large display size for SQL_LONGVARCHAR and related types (e.g. MSSQL varchar(max)). Using that value directly for SQLBindCol buffers can make safe_malloc fail and terminate the port with memory_allocation_failed (see erlangGH-9302), regressing the intent of erlangGH-7000. Clamp the effective size to a default limit (8001 bytes, preserving prior behavior for drivers that report modest sizes such as PostgreSQL), with a hard upper bound to avoid runaway allocation. Add connect option max_long_column_size and an extended OPEN_CONNECTION wire layout (0xFF 0x01 prefix + 32-bit big-endian size). Legacy open messages still work; ERL_ODBC_MAX_LONG_COLUMN_SIZE applies when the extended layout is not used.
willus10245
force-pushed
the
fix-odbc-memory-allocation-error
branch
from
e96e73d to
2642ba1
Compare
6 participants
fixes #9302
Oracle and MSSQL can return column sizes of upwards of 4GB, which can cause memory allocation errors when trying to allocate that much buffer space. This change keeps the prior fix for Postgres compatibility, but caps large column sizes for Oracle and MSSQL databases.