Conversation
…lumnMeta() - each call queried the DB to know the name associated with the table's OID: cache the result between two calls - make pdo_pgsql_translate_oid_to_table higher-level, with the last parameter being the handle instead of the raw connection; thus the statement is cleaner, letting the handle do all memory handling on the table oid-to-name translation cache (which by the way is a driver feature more than a statement one)
KentarouTakeda
left a comment
KentarouTakeda
left a comment
There was a problem hiding this comment.
With persistent connections, cached_table_name is allocated with estrdup() but kept on the persistent handle, so it is referenced again on the next request.
$pdo = new Pdo\Pgsql('pgsql:', options: [
PDO::ATTR_PERSISTENT => true,
]);
$pdo->exec('create temp table t(c int)');
$pdo->query('select c from t')->getColumnMeta(0);ext/pdo_pgsql/pgsql_statement.c(857) : Freeing 0x0000ffffb3e03180 (16 bytes), script=...
=== Total 1 memory leaks detected ===
zend_mm_heap corrupted
Segmentation fault
IMO, it might be better to store the cache on each statement. Then an application that runs ALTER TABLE also works correctly.
| return NULL; | ||
| } | ||
| if (oid == H->cached_table_oid) { | ||
| return H->cached_table_name ? estrdup(H->cached_table_name) : NULL; |
There was a problem hiding this comment.
I think you can optimise it by not copying it just returning as const char * and returning cached_table_name instead since add_assoc_string copies it anyway wdyt ?
There was a problem hiding this comment.
I'll look at it but I think my very first version precisely worked like that, and it looked uncomfortable to me as I thought I was breaking the (implicit) contract of pdo_pgsql_translate_oid_to_table made to return a persistent string.
So if I use this shortcut, I'll add a big comment at its top telling that it is the caller's responsibility to persist that string (it can get overwritten or even freed in case the statement is close).
|
and yes @KentarouTakeda comment is correct too. |
3 participants
pdo_pgsql_translate_oid_to_tablehigher-level, with the last parameter being the handle instead of the raw connection; thus the statement is cleaner, letting the handle do all memory handling on the table oid-to-name translation cache (which by the way is a driver feature more than a statement one)(this is part 4. of #23892 (comment))