[#10787] - Fix postgresql driver setUtf() method
- Fixed in Code Base
- 16 Jul 2016
- Medium
- Build: staging
- Easy Test
- # 10787
- Diff
- photodude:patch-9
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Pull Request for Issue pg_set_client_encoding method does not exist in some postgresql extensions. HHVM is one example where the postgresql extension deviates from the typical Zend postgresql module php function set.
Summary of Changes
return -1 i.e. a failure when the pg_set_client_encoding method does not exist.
Testing Instructions
This is not easily testable unless you happen to have a postgresql module where the pg_set_client_encoding method does not exist.
Recommend to merge by code review.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
brianteeman
- | Category | ⇒ | Libraries Postgresql |
I have tested this item
Looks good on Code review. Thanks.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10787.
| Easy | No | ⇒ | Yes |
how about:
return !function_exists('pg_set_client_encoding') ? -1 : pg_set_client_encoding($this->connection, 'UTF8');
@dgt41 using a Ternary Operator here is exactly the same. less lines of code, but questionable on if there is improved or reduced readability. I see it as more of a code style preference in this instance.
Either way will work, so it's a question of what will get this PR reviewed and merged.
I have tested this item
On code review!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10787.
@photodude unforunately i'don't have an HipHop VM with a postgresql installation,so i can only do a code review ....
p.s.
i'm not familiar with tests..... searching....
https://github.com/joomla/joomla-cms/blob/staging/tests/unit/suites/database/driver/postgresql/JDatabaseDriverPostgresqlTest.php#L1046
@alikon because this is not easily testable unless you happen to have a postgresql module where the pg_set_client_encoding method does not exist. I recommend to merge/test by code review. (well I guess you could test that it still functions normally with a standard postgresql module that does have the pg_set_client_encoding method, but that is done by travis in the automated testing)
I could try to find the two hhvm tests in my patch-8 branch that showed this failure and the fix resolving the issue.
We now have 2 successful code reviews needed for RTC
| Milestone |
Added: |
||
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-07-16 17:44:37 |
| Closed_By | ⇒ | roland-d |
Merged on review because this function isn't used in the code, only used in a test class.
@wilsonge please consider this for 3.6.1