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

Add support for binary data #149

Merged
merged 4 commits into from
Dec 27, 2018
Merged

Conversation

csthomas
Copy link
Contributor

@csthomas csthomas commented Dec 22, 2018

Summary of Changes

Add two new methods:

  • $db->quoteBinary() - required by ms sql server (to explicitly cast a value on INSERT query)
  • $db->decodeBinary() - required by Postgresql/Pgsql drivers to get the original binary string.

Add a missing class DatabaseSqliteCase in order to load ./Stubs/ddl.sql file and the same ./Stubs/database.xml file as on other drivers.
Previously, phpunit loaded SQL/XML data from the additional package 'Joomla\Test'.

Testing Instructions

Code review.
Unit tests should pass

Documentation Changes Required

To work properly with a binary string, we have to quote it using quoteBinary(). This method replaces the binary string with hexadecimal text wrapped in the specified format depending on the database driver.

When we want to get value of BLOB/VARBINARY then Postgresql/Pgsql database API returns an equivalent of binary string.

Postgresql (>= 9.0) by default returns hexdecimal text or (<9.0) escaped string.
I changed SQL variable buffer_output to always return escaped string and then use pg_unescape_string to get the original binary string.

PgSQL returns a stream instead of string. To get the original binary string we have to use stream_get_contents().

Other databases do not require this additional conversion. For the code to work properly regardless of the database driver, we will use $db->decodeBinary($value).

@mbabker
Copy link
Contributor

mbabker commented Dec 24, 2018

The new methods should be added to the interface as well.

@mbabker
Copy link
Contributor

mbabker commented Dec 27, 2018

I shouldn't review PRs on mobile. I thought this was targeted to the 2.0 branch, so 889c2f2 should be reverted if it's going to master. My bad.

@csthomas
Copy link
Contributor Author

OK. No problem.

@mbabker mbabker merged commit 3b2ef25 into joomla-framework:master Dec 27, 2018
@csthomas csthomas deleted the quoteBinary branch December 27, 2018 21:21
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