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

ext/dba/tests/dba_gdbm.phpt: test failure due to key ordering #14786

Closed
orlitzky opened this issue Jul 3, 2024 · 4 comments · Fixed by #14962
Closed

ext/dba/tests/dba_gdbm.phpt: test failure due to key ordering #14786

orlitzky opened this issue Jul 3, 2024 · 4 comments · Fixed by #14962

Comments

@orlitzky
Copy link
Contributor

orlitzky commented Jul 3, 2024

Description

Original report: https://bugs.gentoo.org/935379

The diff for this failed tests contains e.g.

-key4: Another Content String
 key2: Content String 2
 key5: The last content string
-[key10]name10: Content String 10
 name9: Content String 9
+[key10]name10: Content String 10
+key4: Another Content String
 [key30]name30: Content String 30
 Total keys: 6

The corresponding test code is,

// Fetch data                                                               
$key = dba_firstkey($db_writer);
$total_keys = 0;
while ($key) {
    echo $key, ': ', dba_fetch($key, $db_writer), \PHP_EOL;
    $key = dba_nextkey($db_writer);
    $total_keys++;
}
echo 'Total keys: ', $total_keys, \PHP_EOL;

So to me it looks like the insert order is not guaranteed to be the iteration order according to dba_firstkey() and dba_nextkey(). Since there are only six keys, I guess we could sort the output (in a locale-independent way) before printing it?

PHP Version

PHP-8.2.20

Operating System

Gentoo Linux

@Girgias
Copy link
Member

Girgias commented Jul 3, 2024

This might actually something that changed in the GDBM driver/lib? As I don't see why it would change randomly, I had consistent behaviour when I was actively refactoring this a while back

@orlitzky
Copy link
Contributor Author

orlitzky commented Jul 3, 2024

It's possible but my first guess would be that it's on x86. The same version with the same build flags passes for me on amd64.

@orlitzky
Copy link
Contributor Author

orlitzky commented Jul 4, 2024

The iteration order is apparently based on gdbm's internal hashes and FWIW gdbm's own test suite sorts the output, see e.g. https://git.savannah.gnu.org/cgit/gdbm.git/tree/tests/create00.at

The gtdump utility is a little CLI program that uses firstkey() and nextkey().

@Girgias
Copy link
Member

Girgias commented Jul 6, 2024

Right, then adding a sort and referencing the upstream test makes sense.

orlitzky added a commit to orlitzky/php-src that referenced this issue Jul 15, 2024
The actual output is now sorted for consistency, so we need to update
the expected output as well. As a nice side effect, some differences
in the expected outputs for the various engines have been eliminated.

Closes phpGH-14786
Girgias pushed a commit that referenced this issue Jul 26, 2024
* ext/dba/tests/setup/setup_dba_tests.inc: sort test output

Iterating through a database with firstkey() and nextkey() is
guaranteed to retrieve all rows, but apparently not in any particular
order. This is causing a test failure for at least one user, so we
steal the sort() approach from GDBM to ensure that the output is
predictable.

* ext/dba/tests/dba_*.phpt: sort expected test output

The actual output is now sorted for consistency, so we need to update
the expected output as well. As a nice side effect, some differences
in the expected outputs for the various engines have been eliminated.

Closes GH-14786

* ext/pgsql/tests/80_bug14383.phpt: sort expected test output

This test uses a routine from ext/dba that now sorts its (actual)
output, so we have to sort the expected output here as well.

* ext/dba/tests/setup/setup_dba_tests.inc: update comment

After doing some more digging, it looks like GDBM isn't the only
engine where the iteration order with firstkey() and nextkey()
might change unexpectedly.
devnexen pushed a commit to devnexen/php-src that referenced this issue Jul 27, 2024
* ext/dba/tests/setup/setup_dba_tests.inc: sort test output

Iterating through a database with firstkey() and nextkey() is
guaranteed to retrieve all rows, but apparently not in any particular
order. This is causing a test failure for at least one user, so we
steal the sort() approach from GDBM to ensure that the output is
predictable.

* ext/dba/tests/dba_*.phpt: sort expected test output

The actual output is now sorted for consistency, so we need to update
the expected output as well. As a nice side effect, some differences
in the expected outputs for the various engines have been eliminated.

Closes phpGH-14786

* ext/pgsql/tests/80_bug14383.phpt: sort expected test output

This test uses a routine from ext/dba that now sorts its (actual)
output, so we have to sort the expected output here as well.

* ext/dba/tests/setup/setup_dba_tests.inc: update comment

After doing some more digging, it looks like GDBM isn't the only
engine where the iteration order with firstkey() and nextkey()
might change unexpectedly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants