IGNITE-6692 Select query on a client with unused field throws an exception.

[Nikita Amelchev]

Hello, Igniters!

I’ve fixed the issue where SQL query with unused fields throws an exception
[1].

Description of the problem: We can build a binary object without fields in
the binary object builder. If we don't add a field, SchemaId will be set to
zero. When we try to read field value, marshaller will try to find this
schema and throw an exception.

It was fixed by introducing of a new check on schemaId and when it equals
to zero then the field value will return null.

Please, review the changes [2]. Tests look good [3].

1. https://issues.apache.org/jira/browse/IGNITE-6692

2. https://reviews.ignite.apache.org/ignite/review/IGNT-CR-381

3.
https://ci.ignite.apache.org/project.html?projectId=Ignite20Tests&tab=projectOverview&branch_Ignite20Tests=pull%2F2918%2Fhead


--
Best wishes,
Amelchev Nikita

[Vladimir Ozerov]

Nikita,

I am afraid this is incorrect fix, since zero is valid hash for schema. We
control whether the schema is est through the flag
BinaryUtils.FLAG_HAS_SCHEMA. Please try using it instead.

On Fri, Oct 27, 2017 at 6:38 PM, Nikita Amelchev <[hidden email]>
wrote:

> Hello, Igniters!
>
> I’ve fixed the issue where SQL query with unused fields throws an exception
> [1].
>
> Description of the problem: We can build a binary object without fields in
> the binary object builder. If we don't add a field, SchemaId will be set to
> zero. When we try to read field value, marshaller will try to find this
> schema and throw an exception.
>
> It was fixed by introducing of a new check on schemaId and when it equals
> to zero then the field value will return null.
>
> Please, review the changes [2]. Tests look good [3].
>
> 1. https://issues.apache.org/jira/browse/IGNITE-6692
>
> 2. https://reviews.ignite.apache.org/ignite/review/IGNT-CR-381
>
> 3.
> https://ci.ignite.apache.org/project.html?projectId=Ignite20Tests&tab=
> projectOverview&branch_Ignite20Tests=pull%2F2918%2Fhead
>
>
> --
> Best wishes,
> Amelchev Nikita
>

[dsetrakyan]

On Fri, Oct 27, 2017 at 11:11 AM, Vladimir Ozerov <[hidden email]>
wrote:

> Nikita,
>
> I am afraid this is incorrect fix, since zero is valid hash for schema. We
> control whether the schema is est through the flag
> BinaryUtils.FLAG_HAS_SCHEMA. Please try using it instead.
>

Vladimir, in that case how come the tests passed? Do we need to add a new
test to enforce this scenario?

[Nikita Amelchev]

Thanks for the comments.

I have done check to the existing schema through the
BinaryUtils.FLAG_HAS_SCHEMA flag [1].

Also, I have checked other places where we can try to read not initialized
schema and found that BinaryObjectBuilderImpl.ensureReadCacheInit() method
can do it. If we try to get from the BinaryObject, which contains no
fields, any field through BinaryObjectBuilder, it will throw an exception.
I have fixed this bug and suggest to include it in this issue. Please,
review [2]. Tests look good [3].

1. https://issues.apache.org/jira/browse/IGNITE-6692
2. https://reviews.ignite.apache.org/ignite/review/IGNT-CR-381
3.
https://ci.ignite.apache.org/project.html?projectId=Ignite20Tests&tab=projectOverview&branch_Ignite20Tests=pull%2F2918%2Fhead


2017-10-28 18:29 GMT+03:00 Dmitriy Setrakyan <[hidden email]>:

> On Fri, Oct 27, 2017 at 11:11 AM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > Nikita,
> >
> > I am afraid this is incorrect fix, since zero is valid hash for schema.
> We
> > control whether the schema is est through the flag
> > BinaryUtils.FLAG_HAS_SCHEMA. Please try using it instead.
> >
>
> Vladimir, in that case how come the tests passed? Do we need to add a new
> test to enforce this scenario?
>

--
Best wishes,
Amelchev Nikita