select() does not allow null offset parameter in the strict types mode

[Totktonada]

How to reproduce:

(console 1) $ tarantool
tarantool> box.cfg{listen = 3301}
(console 2) $ phpize && ./configure && make
(console 2) $ php -a -d extension=modules/tarantool.so
php > declare(strict_types=1); print_r($c->select('s', [], 0, null, 0, 'ALL'));
Array
(
    [0] => Array
        (
            [0] => 1
        )

    [1] => Array
        (
            [0] => 2
        )

)
php > declare(strict_types=1); print_r($c->select('s', [], 0, null, null, 'ALL'));
PHP Warning:  Uncaught TypeError: Tarantool::select() expects parameter 5 to be int, null given in php shell code:1
Stack trace:
#0 php shell code(1): Tarantool->select()
#1 {main}
  thrown in php shell code on line 1

The documetation states that $offset parameter is 0 and it is quite natural to expect that when null is passed it'll be considered as zero offset.

See the similar #58.

[rybakit]

Well, the current behavior does not contradict the documentation, it is stated that the offset is an integer number, to support both integer and null, it should be declared as ?int $offset = 0:

https://3v4l.org/l1dlE

So I don't know whether this issue is a bug or a feature request. :)

[Totktonada]

Cited from the documentation:

public array Tarantool::select(mixed $space [, mixed $key = array() [, mixed $index = 0 [, int $limit = PHP_INT_MAX [, int $offset = 0 [, $iterator = Tarantool::ITERATOR_EQ ] ] ] ] ] )

Should all parameters except the first one be described as ?<type>? Or we going to be too formal? :)

[rybakit]

Since php doesn't support named arguments, ?<type> might be more convenient when you want to bypass some arguments, so you don't have to worry what is the default value for each skipped argument:

$tarantool->select('foo', null, null, null, null, Tarantool::ITERATOR_EQ);

vs

$tarantool->select('foo', [], 0, PHP_INT_MAX, 0, Tarantool::ITERATOR_EQ);

But I don't have a strong opinion on that, both options look ugly to me :)

[Totktonada]

No-no, I meant whether you suggest to update README with the change of the behaviour and, if so, how exactly?

[rybakit]

public Tarantool::select(
    int|string $space [, 
    int|array|null $key = [] [, 
    int|string|null $index = 0 [,  // does it support named indices yet?
    int|null $limit = PHP_INT_MAX [, 
    int|null $offset = 0 [, 
    int|string|null $iterator = Tarantool::ITERATOR_EQ|Tarantool::ITERATOR_ALL
] ] ] ] ] ) : array

where

• $key = null is equivalent to $key = []
• $index = null is equivalent to $index = 0
• $limit = null is equivalent to $limit = PHP_INT_MAX
• $offset = null is equivalent to $offset = 0
• $iterator = null is equivalent to $iterator = Tarantool::ITERATOR_EQ|Tarantool::ITERATOR_ALL.

WDYT?

[Totktonada]

// does it support named indices yet?

Yep. However only when space is a string too :) See #42.

where

<...>

I would say just that null is equivalent of passing of a default value.

Anyway, it seems I'll postpone README.md updates, because I should finish PR #153 on this week.

[Totktonada]

Fixed in php7-v2 branch in 0.3.0-37-g0a206c4. The branch will be renamed to master in the scope of #137.