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

Allow larger column sizes than 8001 in case DB supports it #7000

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

ignatenkobrain
Copy link
Contributor

In PostgreSQL, the TEXT field can easily have more than 8001 characters and it does return the actual number. The size == 0 would happen in case of some DBs won't return valid value where we would keep the same behavior as of now.

@CLAassistant
Copy link

CLAassistant commented Mar 9, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

CT Test Results

    2 files    15 suites   3m 45s ⏱️
199 tests   92 ✔️ 107 💤 0
214 runs  107 ✔️ 107 💤 0

Results for commit 53ef5df.

♻️ 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

In PostgreSQL, the TEXT field can easily have more than 8001 characters
and it does return the actual number. The size == 0 would happen in case
of some DBs won't return valid value where we would keep the same
behavior as of now.

Signed-off-by: Igor Raits <igor@gooddata.com>
@rickard-green rickard-green added the team:PS Assigned to OTP team PS label Mar 13, 2023
@IngelaAndin IngelaAndin self-assigned this Mar 15, 2023
@IngelaAndin IngelaAndin added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Mar 29, 2023
@IngelaAndin IngelaAndin changed the base branch from maint to master March 29, 2023 09:26
@IngelaAndin IngelaAndin added the testing currently being tested, tag is used by OTP internal CI label Mar 29, 2023
@IngelaAndin
Copy link
Contributor

Seems reasonable, I have enabled it in our tests to see that it does not break anything. I guess you have tested that it works?
We do have Postgres as backend for Linux odbc tests but I see upper limit test of text are skipped due to they being considered to take up to much resources (maybe that is not absolutely true anymore, this code is old). Do you possible know if there exist some other easy way to test what the limit is ?

@ignatenkobrain
Copy link
Contributor Author

We are using this patch in production for like 5 years :) sadly no, I'm not aware of simple way how to test it.

@IngelaAndin
Copy link
Contributor

As it does not break anything and is not prioritized by Ericsson, we have decided to see this as an improvement worth including even if the test are not improved.

@gionea
Copy link
Contributor

gionea commented Nov 15, 2023

This change has introduced a problem where the ODBC port crashes (port_exit, memory_allocation_failed) when used in conjunction with the Oracle driver.

I have been able to confirm this change as being the culprit by applying a patch that reverts the change from the 26.1 release.

I can open a separate issue for this problem, if you prefer, but it would be helpful if you could give me some pointers to how to collect more information about the crash. For example, it would be useful to know what SQL query it was executing while it crashed, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants