Skip to content

Commit 6393513

Browse files
committed
PHP-189 - Map compare doesn't compare values
`php_driver_map_compare()` doesn't compare values so if two maps have the same keys, but different values they will compare as being equal.
1 parent 7094dc7 commit 6393513

File tree

4 files changed

+33
-5
lines changed

4 files changed

+33
-5
lines changed

ext/src/Map.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -513,16 +513,19 @@ php_driver_map_compare(zval *obj1, zval *obj2 TSRMLS_DC)
513513
result = php_driver_type_compare(type1, type2 TSRMLS_CC);
514514
if (result != 0) return result;
515515

516-
if (HASH_COUNT(map1->entries) != HASH_COUNT(map1->entries)) {
517-
return HASH_COUNT(map1->entries) < HASH_COUNT(map1->entries) ? -1 : 1;
516+
if (HASH_COUNT(map1->entries) != HASH_COUNT(map2->entries)) {
517+
return HASH_COUNT(map1->entries) < HASH_COUNT(map2->entries) ? -1 : 1;
518518
}
519519

520520
HASH_ITER(hh, map1->entries, curr, temp) {
521-
php_driver_map_entry *entry;
521+
php_driver_map_entry *entry = NULL;
522522
HASH_FIND_ZVAL(map2->entries, PHP5TO7_ZVAL_MAYBE_P(curr->key), entry);
523523
if (entry == NULL) {
524524
return 1;
525525
}
526+
result = php_driver_value_compare(PHP5TO7_ZVAL_MAYBE_P(curr->value),
527+
PHP5TO7_ZVAL_MAYBE_P(entry->value));
528+
if (result != 0) return result;
526529
}
527530

528531
return 0;

ext/src/Set.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,8 @@ php_driver_set_compare(zval *obj1, zval *obj2 TSRMLS_DC)
351351
result = php_driver_type_compare(type1, type2 TSRMLS_CC);
352352
if (result != 0) return result;
353353

354-
if (HASH_COUNT(set1->entries) != HASH_COUNT(set1->entries)) {
355-
return HASH_COUNT(set1->entries) < HASH_COUNT(set1->entries) ? -1 : 1;
354+
if (HASH_COUNT(set1->entries) != HASH_COUNT(set2->entries)) {
355+
return HASH_COUNT(set1->entries) < HASH_COUNT(set2->entries) ? -1 : 1;
356356
}
357357

358358
HASH_ITER(hh, set1->entries, curr, temp) {

tests/unit/Cassandra/MapTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,29 @@ public function notEqualTypes()
286286
{
287287
$setType = Type::set(Type::int());
288288
return array(
289+
// Different types
289290
array(Type::map(Type::int(), Type::int())->create(),
290291
Type::map(Type::int(), Type::varchar())->create()),
292+
// Different number of keys
291293
array(Type::map(Type::int(), Type::varchar())->create(1, 'a', 2, 'b', 3, 'c'),
292294
Type::map(Type::int(), Type::varchar())->create(1, 'a')),
295+
// Different keys with same values
296+
array(Type::map(Type::int(), Type::varchar())->create(1, 'a', 2, 'b', 3, 'c'),
297+
Type::map(Type::int(), Type::varchar())->create(4, 'a', 5, 'b', 6, 'c')),
298+
// Different values with same keys
299+
array(Type::map(Type::int(), Type::varchar())->create(1, 'a', 2, 'b', 3, 'c'),
300+
Type::map(Type::int(), Type::varchar())->create(1, 'd', 2, 'e', 3, 'f')),
301+
// Composite keys
293302
array(Type::map($setType, Type::varchar())->create($setType->create(4, 5, 6), 'a', $setType->create(7, 8, 9), 'b'),
294303
Type::map($setType, Type::varchar())->create($setType->create(1, 2, 3), 'a', $setType->create(4, 5, 6), 'b'))
295304
);
296305
}
306+
307+
public function testCompareNotEqualDifferentCount()
308+
{
309+
$this->assertTrue(Type::map(Type::int(), Type::varchar())->create(1, 'a') <
310+
Type::map(Type::int(), Type::varchar())->create(1, 'a', 2, 'b'));
311+
$this->assertTrue(Type::map(Type::int(), Type::varchar())->create(1, 'a', 2, 'b') >
312+
Type::map(Type::int(), Type::varchar())->create(1, 'a'));
313+
}
297314
}

tests/unit/Cassandra/SetTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,4 +331,12 @@ public function notEqualTypes()
331331
Type::set($setType)->create($setType->create(4, 5, 6))),
332332
);
333333
}
334+
335+
public function testCompareNotEqualDifferentCount()
336+
{
337+
$this->assertTrue(Type::set(Type::int())->create(1) <
338+
Type::set(Type::int())->create(1, 2));
339+
$this->assertTrue(Type::set(Type::int())->create(1, 2) >
340+
Type::set(Type::int())->create(1));
341+
}
334342
}

0 commit comments

Comments
 (0)