pg_fetch_object() with abstract classes #20180
Conversation
Girgias
force-pushed
the
pgsql-abstract-class
branch
3 times, most recently
from
4db2580 to
dc642e8
Compare
|
@devnexen do you know why Windows fails, it says there is an issue with the CLEAN section of the test |
|
I have no real idea honestly. |
| } catch(Throwable $e) { | ||
| echo $e::class, ': ', $e->getMessage(), PHP_EOL; | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: would it be possible to add the closing tag ?
There was a problem hiding this comment.
Might also be the reason windows is confused
| $result = pg_query($db, $sql); | ||
| var_dump(pg_fetch_object($result, NULL, 'E')); | ||
| } catch(Throwable $e) { | ||
| echo $e::class, ': ', $e->getMessage(), PHP_EOL; |
There was a problem hiding this comment.
Maybe try without this last PHP_EOL wdyt ?
Girgias
force-pushed
the
pgsql-abstract-class
branch
from
dc642e8 to
c48266c
Compare
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
I did not test this myself, but the code changes look good.
I don't think you need all those CE variants (i.e. interface, enum, ...) for the test, but it doesn't hurt.
I mainly wanted them to check the error messages, but they indeed can't hurt (especially considering how poorly tested ext/pgsql is) |
This shouldn't really be happening in the first place
| include('config.inc'); | ||
| $db = pg_connect($conn_str); | ||
| pg_query($db, "DROP TABLE IF EXISTS pg_fetch_object_abstract_class cascade"); | ||
| $db = @pg_connect($conn_str); |
There was a problem hiding this comment.
This issue really weirds me out. would it be possible to do the cleaning somewhere in the file section instead just for testing once at least ?
|
So it fails because the CLEAN section somehow outputs " \r\n" and that's not equal to the empty string so it borks. I don't know what produces these whitespace characters. Pushing some more debugging code... |
|
Will continue debugging this later ig. The simple fix is just get rid of the clean section and do the table deletion in the FILE section, that's how the other tests do it. |
|
I was completely confused by the random syntax error that appears when I remove the php end tags. Especially since it doesn't even reproduce in my Windows VM. That should be illegal. That is the thing that adds whitespace characters to the config script which then gets into the output. |
ndossche
force-pushed
the
pgsql-abstract-class
branch
from
2bc7aae to
9a149a7
Compare
|
It's green finally. And the code is adjusted such that we will never run into this again in the future. I'll leave the cleanup to you (i.e. squashing + reverting my CI workflow adaptations that disable other jobs and only run the pgsql tests) |
* PHP-8.3: Update NEWS for recent pgsql bugfix ext/pgsql: Fix segfaults when attempting to fetch row into a non-instantiable class name (#20180)
* PHP-8.4: Update NEWS for recent pgsql bugfix ext/pgsql: Fix segfaults when attempting to fetch row into a non-instantiable class name (#20180)
* PHP-8.5: Update NEWS for recent pgsql bugfix ext/pgsql: Fix segfaults when attempting to fetch row into a non-instantiable class name (#20180)
3 participants
No description provided.