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

SQL Server/TinyTDS Error for OCTET_LENGTH #35

Closed
utsw-alex-williams opened this issue Oct 30, 2023 · 6 comments · Fixed by #38
Closed

SQL Server/TinyTDS Error for OCTET_LENGTH #35

utsw-alex-williams opened this issue Oct 30, 2023 · 6 comments · Fixed by #38

Comments

@utsw-alex-williams
Copy link

Hello, and thank you for creating this gem - it has been very useful.

I am using ActiveStorageDB with SQL Server via TinyTDS. I'm aware that ActiveStorageDB currently specifies support for Postgres and MySQL only, but I figured I'd ask if SQL Server could be considered for future support, or if you have any insight into a possible workaround for my specific issue.

I am encountering errors when attempting to use ActiveStorage variants with ActiveStorageDB. I got an error related to AnalyzeJob after attaching images via ActiveStorage, but displaying images was ok. However, the same error occurs when attempting to display variants. The error displayed is:

ActionView::Template::Error (TinyTds::Error: 'OCTET_LENGTH' is not a recognized built-in function name.)

The SQL statement causing this issue is:

ActiveStorageDB::File Load (45.7ms)  EXEC sp_executesql N'SELECT OCTET_LENGTH(data) AS size FROM [active_storage_db_files] WHERE [active_storage_db_files].[ref] = @0 ORDER BY [active_storage_db_files].[id] ASC OFFSET 0 ROWS FETCH NEXT @1 ROWS ONLY', N'@0 nvarchar(4000), @1 int', @0 = N'wbcflekvut7wmxukoxa535n8hwfg', @1 = 1  [["ref", nil], ["LIMIT", nil]]

I think SQL Server has either LEN or DATALENGTH, though without deep diving into the code I'm not sure which would be better to use, if any, or if it could be easily patched in.

I understand that SQL Server is not officially supported but any guidance on how to fix this would be appreciated.

@blocknotes
Copy link
Owner

Hey @utsw-alex-williams,
thanks for your feedback.

I have no experience with MSSQL unfortunately so I can tell you little at the moment.

I have just opened an experimental PR here: #38
If you check the test suite output I get the same error that you reported in a test and a second error that seems a syntax one.

I'll try something when possible 🤷

@blocknotes
Copy link
Owner

After checking the failing specs:

  • DATALENGTH in place of OCTET_LENGTH for MSSQL seems good to me (ref) because it returns the number of bytes also of binary fields;
  • the syntax error regards SUBSTRING

I added a new commit on the PR 38 to apply these changes.

Can you make a try using the branch feat/mssql-support ?

@utsw-alex-williams
Copy link
Author

So far it looks like that worked perfectly. There were no problems displaying a variant with Vips transformations.

Thank you so much!

@blocknotes
Copy link
Owner

blocknotes commented Nov 2, 2023

@utsw-alex-williams: I have just released the version 1.3.0 which includes the experimental MSSQL support, can you make a test and let me know?

@utsw-alex-williams
Copy link
Author

Looks great!

@blocknotes
Copy link
Owner

A special thanks to @utsw-alex-williams for the support! 💪

FYI: I've also updated a test to include variants:
https://github.com/blocknotes/active_storage_db/pull/41/files#diff-0f99c3fbce9abb834c898a75e39473e2ab2a5b9b3db400f38ca458953e060374

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 a pull request may close this issue.

2 participants