edespino commented on code in PR #627:
URL: https://github.com/apache/madlib/pull/627#discussion_r2454401941
##########
src/madpack/madpack.py:
##########
@@ -1344,14 +1346,14 @@ def set_dynamic_library_path_in_database(dbver_split,
madlib_library_path):
(portid == 'postgres' and is_rev_gte(dbver_split,
get_rev_num('13.0')))):
libdir = libdir.decode()
- libdir = libdir.strip()+'/postgresql'
+ libdir = str(libdir.strip(), encoding='utf-8')+'/postgresql'
Review Comment:
**Testing Note:** Encountering `TypeError: decoding str is not supported`
when installing on PostgreSQL 14.19.
**Root Cause:**
For Postgres 13+ (line 1347), `libdir` is already decoded to a string via
`.decode()`, but line 1349 attempts to decode it again with
`str(libdir.strip(), encoding='utf-8')`, which fails because you cannot
decode a string that's already been decoded.
**Recommended Solution:**
Ensure `libdir` is always decoded to a string before line 1349, then simply
strip and append the path:
```python
libdir = subprocess.check_output(['pg_config','--libdir'])
if ((portid == 'greenplum' and is_rev_gte(dbver_split, get_rev_num('7.0')))
or
(portid == 'postgres' and is_rev_gte(dbver_split, get_rev_num('13.0')))):
libdir = libdir.decode()
else:
libdir = libdir.decode('utf-8')
libdir = libdir.strip() + '/postgresql'
This ensures libdir is consistently a string for all code paths (older and
newer versions), eliminating the type inconsistency that causes the
error.
Request for Review: Please validate this fix works correctly for both:
- Older versions (Postgres <13, Greenplum <7) where
subprocess.check_output() returns bytes
- Newer versions (Postgres 13+, Greenplum 7+) where explicit decoding is
needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
