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

Backport negative string size (or size too big) fix for Rails 7.0 #7

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

douglas
Copy link

@douglas douglas commented Oct 28, 2024

Backports the negative string size (or size too big) fix to Rails 7.0 since we can't use master tip due to it being locked to Rails 7.1, which is not our case yet.

This PR should never be merged since we must either fix this odbc_adapter to work with Rails 7.0 or upgrade Campaigns to Rails 7.1.

@douglas douglas changed the title Backport the fix for Rails 7.0 Backport negative string size (or size too big) fix for Rails 7.0 Oct 28, 2024
schema_name, table_name, table_type = row[1..3]
next if respond_to?(:table_filtered?) && table_filtered?(schema_name, table_type)
table_names << format_case(table_name)
stmt = @connection.prepare("SHOW TABLES IN ACCOUNT")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just mildly curious, executing this for my personal user in Snowflake returned a very large result. Does this work only because our snowflake connection user is more limited access wise so the table count is reasonable?

Copy link
Author

@douglas douglas Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmmm, I didn't tried it yet, but according to the comment on this commit:

use SHOW TABLES query to address "size too big" errors for long table comments
This change fixes an issue with the `tables` call erroring out if attempting to
fetch any tables that have longer than 254 character comments on them. This fix
is based on the approach used by the patterninc fork of the gem, whose PR does
a great job of explaining the issue and idea behind the solution:

> The fetch_all function is using a bound 'remarks' column that is configured to
> have a max_length of 254 characters. That max length is not configurable and the
> column cannot be unbound. So when a table with a comment longer than 254 characters
> exists in the result set, we run into the negative string size (or size too big) error.

- patterninc#9

The primary difference here is that we're using `SHOW TABLES IN ACCOUNT` query,
which should return the same set of tables as `fetch_all` was returning before,
eliminating the need for an opt-in config flag.

Snowflake docs on the SHOW TABLES query:
https://docs.snowflake.com/en/sql-reference/sql/show-tables

So, I suppose @connection.prepare("SHOW TABLES IN ACCOUNT") would return the same as the previous stmt.fetch_all, of course, depending on the account being used.

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.

2 participants