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

Internal error on select by a space id and an index name #42

Closed
rybakit opened this issue May 29, 2015 · 6 comments · Fixed by #142
Closed

Internal error on select by a space id and an index name #42

rybakit opened this issue May 29, 2015 · 6 comments · Fixed by #142
Assignees
Labels
bug Something isn't working

Comments

@rybakit
Copy link
Collaborator

rybakit commented May 29, 2015

const SPACE_INDEX = 288;
(new Tarantool())->select(SPACE_INDEX, [], 'name');

fails with:

PHP Fatal error:  Uncaught exception 'Exception' with message 'Internal Error' ...
@bigbes bigbes added the bug Something isn't working label May 30, 2015
@bigbes bigbes self-assigned this May 30, 2015
@bigbes
Copy link
Contributor

bigbes commented May 30, 2015

Ok, i've found where is error, but until i fix it, i suggest you to authenticate before start sending commands or using '_index'/'_vindex' instead of number. Or using 2 instead of "name"

@bigbes bigbes modified the milestones: 0.0.15, 0.1.0 Jun 2, 2015
@bigbes
Copy link
Contributor

bigbes commented Feb 10, 2016

Sorry for long response. Everything must be OK by now. If it's not - please, reopen this bug and attach more information. Thank you!

@bigbes bigbes closed this as completed Feb 10, 2016
@rybakit
Copy link
Collaborator Author

rybakit commented Mar 2, 2016

@bigbes Looks like it's still not possible to use index id. Now I get

Uncaught exception 'Exception' with message 'Failed parsing schema (index) or memory issues' ...

@rybakit rybakit reopened this Mar 2, 2016
@rybakit
Copy link
Collaborator Author

rybakit commented Mar 2, 2016

Also, in this test case https://github.com/tarantool/tarantool-php/blob/master/test/MockTest.php there is a defined SPACE_INDEX constant which is not used anywhere in the tests.

@bigbes
Copy link
Contributor

bigbes commented Mar 9, 2016

This 'bug' requires major rewriting of schema.
If you're using SPACE_ID as number, please, use INDEX_ID as number too, for now.

@bigbes bigbes assigned bigbes and unassigned bigbes Jun 23, 2016
bigbes added a commit to bigbes/tarantool-php that referenced this issue Jul 20, 2018
bigbes added a commit to bigbes/tarantool-php that referenced this issue Jul 20, 2018
bigbes added a commit to bigbes/tarantool-php that referenced this issue Dec 2, 2018
@Totktonada Totktonada removed this from the 0.1.0 milestone Mar 23, 2020
@Totktonada Totktonada changed the title Internal error Internal error on select by a space id and an index name Mar 23, 2020
@Totktonada Totktonada added this to the PHP 7.* + bugfixes milestone Mar 23, 2020
MariaHajdic added a commit that referenced this issue Jun 16, 2020
@Totktonada
Copy link
Member

Totktonada commented Jul 13, 2020

NB: The problem appears only when a schema is not fetched at the moment of select() (or when it is obsoleted and does not contain relevant space). (I mean, the problem that is present after reopening the issue.)

Totktonada pushed a commit to bigbes/tarantool-php that referenced this issue Jul 13, 2020
When a space exists, but a client side schema does not know about it at
the moment (say, right after connection), and when a user calls select()
with a space id (number) and name of an index (string), the problem
appears: the client raises the following error:

 | TarantoolParsingException: Failed to parse schema (index)

However it should successfully perform the request.

The problem is that when there is no record for a space in client's
schema, it is not possible to save a record for an index of this space.

The idea of the fix is to verify whether we know about a space even when
a numeric ID is already provided. If a client has no record about the
space, it fetches a schema and verify whether the space appears. If so,
there is no more problem described above. Otherwise the client raises an
error that the space with given ID does not exist.

Closes tarantool#42

@Totktonada: polish code, polish test, write the description.
Totktonada pushed a commit to bigbes/tarantool-php that referenced this issue Jul 17, 2020
When a space exists, but a client side schema does not know about it at
the moment (say, right after connection), and when a user calls select()
with a space id (number) and name of an index (string), the problem
appears: the client raises the following error:

 | TarantoolParsingException: Failed to parse schema (index)

However it should successfully perform the request.

The problem is that when there is no record for a space in client's
schema, it is not possible to save a record for an index of this space.

The idea of the fix is to verify whether we know about a space even when
a numeric ID is already provided. If a client has no record about the
space, it fetches a schema and verify whether the space appears. If so,
there is no more problem described above. Otherwise the client raises an
error that the space with given ID does not exist.

While we're here, ensure that return values of tarantool_schema_*() are
checked against -1, not Zend's FAILURE macro (which is only guaranteed
to be less than zero) in the modified code. Also ensure that the FAILURE
macro is returned from the get_spaceno_by_name() function, not -1.

Closes tarantool#42

@Totktonada: polish code, polish test, write the description.
Totktonada pushed a commit that referenced this issue Jul 17, 2020
When a space exists, but a client side schema does not know about it at
the moment (say, right after connection), and when a user calls select()
with a space id (number) and name of an index (string), the problem
appears: the client raises the following error:

 | TarantoolParsingException: Failed to parse schema (index)

However it should successfully perform the request.

The problem is that when there is no record for a space in client's
schema, it is not possible to save a record for an index of this space.

The idea of the fix is to verify whether we know about a space even when
a numeric ID is already provided. If a client has no record about the
space, it fetches a schema and verify whether the space appears. If so,
there is no more problem described above. Otherwise the client raises an
error that the space with given ID does not exist.

While we're here, ensure that return values of tarantool_schema_*() are
checked against -1, not Zend's FAILURE macro (which is only guaranteed
to be less than zero) in the modified code. Also ensure that the FAILURE
macro is returned from the get_spaceno_by_name() function, not -1.

Closes #42

@Totktonada: polish code, polish test, write the description.
Totktonada added a commit that referenced this issue Jul 17, 2020
Technically we should return FAILURE (not -1) from the changed function
in case of a failure. FAILURE is -1 in fact and it unlikely will be
changed, but the only formal guarantee we have is that FAILURE is a
negative value.

Cited from [1]:

 | typedef enum {
 |   SUCCESS =  0,
 |   FAILURE = -1,		/* this MUST stay a negative number, or it may affect functions! */
 | } ZEND_RESULT_CODE;

No behaviour actually changed in this commit.

It is the follow up for 5647d24 ('Fix
select() by space_no and index_name'), where we initially set eyes on
this point.

[1]: https://github.com/php/php-src/blob/4903f7c5fde11a115f659ec54a1d0ede6fd7232c/Zend/zend_types.h#L53-L56

Follows up #42.
Totktonada added a commit that referenced this issue Jul 17, 2020
Technically we should return FAILURE (not -1) from the changed function
in case of a failure. FAILURE is -1 in fact and it unlikely will be
changed, but the only formal guarantee we have is that FAILURE is a
negative value.

Cited from [1]:

 | typedef enum {
 |   SUCCESS =  0,
 |   FAILURE = -1,		/* this MUST stay a negative number, or it may affect functions! */
 | } ZEND_RESULT_CODE;

No behaviour actually changed in this commit.

It is the follow up for 5647d24 ('Fix
select() by space_no and index_name'), where we initially set eyes on
this point.

[1]: https://github.com/php/php-src/blob/4903f7c5fde11a115f659ec54a1d0ede6fd7232c/Zend/zend_types.h#L53-L56

Follows up #42.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants