Fix select() by space_no and index_name #142

bigbes · 2018-12-02T22:05:20Z

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

Looks okay except minor comments.

src/tarantool_schema.c

src/tarantool.c

src/tarantool_schema.c

test/DMLTest.php

src/tarantool.c

test/DMLTest.php

Totktonada · 2020-07-13T22:39:14Z

@LeonidVas Please, look at this briefly if time permits. I think the patch in the good state now, but it would be good if you'll glance.

LeonidVas · 2020-07-15T12:15:33Z

@LeonidVas Please, look at this briefly if time permits. I think the patch in the good state now, but it would be good if you'll glance.

Hi! @bigbes , thank you for the PR. Generally LGTM.
But I'm confused by mixing FAILURE from the ZEND_RESULT_CODE enum and -1.
We don't have any guarantee that it will always be -1 and can't check -1 == FAILURE. From a comment to ZEND_RESULT_CODE: "this MUST stay a negative number", not exactly -1. On the other hand, I agree that such things rarely change. So, Up to you @Totktonada .

@Totktonada

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 · 2020-07-17T09:02:17Z

@LeonidVas I agree. Fixed and added the following paragraph into the commit message:

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.

bigbes mentioned this pull request Dec 2, 2018

Updated PHPUnit to version 7 + gh-24 + gh-42 + gh-85 + gh-83 + gh-71 #134

Closed

Totktonada reviewed Dec 13, 2018

View reviewed changes

src/tarantool_schema.c Outdated Show resolved Hide resolved

src/tarantool_schema.c Outdated Show resolved Hide resolved

MariaHajdic force-pushed the gh-42-fetch-schema-anyway branch 2 times, most recently from 4cb7778 to 8b8d09e Compare June 16, 2020 19:06

Totktonada mentioned this pull request Jun 17, 2020

Bigbes gh 42 fetch schema anyway #158

Closed

MariaHajdic force-pushed the gh-42-fetch-schema-anyway branch from bd6232c to f3ba38f Compare June 19, 2020 12:45

Totktonada changed the base branch from php7-v2 to master June 29, 2020 22:14