Skip to content

Conversation

@mdumandag
Copy link
Contributor

Due to lazy deserialization, Compact serialized objects were
not working nicely with SQL. It was possible to get deserialization
errors that cannot be easily understood by the user.

Similar to what we did for other APIs, we have removed lazy
deserialization from this API as well.

To do that efficiently, I have modified the protocol code generator
so that we can pass to_object function to the SqlPageCodec where
we can do deserialization while filling rows and columns, so that
we will not iterate over the result set once more.

protocol pr: hazelcast/hazelcast-client-protocol#461

@mdumandag mdumandag added this to the 5.2.0 milestone Mar 20, 2023
@mdumandag mdumandag requested a review from yuce March 20, 2023 15:36
@mdumandag mdumandag self-assigned this Mar 20, 2023
Due to lazy deserialization, Compact serialized objects were
not working nicely with SQL. It was possible to get deserialization
errors that cannot be easily understood by the user.

Similar to what we did for other APIs, we have removed lazy
deserialization from this API as well.

To do that efficiently, I have modified the protocol code generator
so that we can pass to_object function to the SqlPageCodec where
we can do deserialization while filling rows and columns, so that
we will not iterate over the result set once more.
@mdumandag mdumandag force-pushed the sql-eager-deserialization branch from 31f313a to 47df7c3 Compare March 23, 2023 15:07
@codecov-commenter
Copy link

codecov-commenter commented Mar 23, 2023

Codecov Report

Merging #619 (83eeea0) into master (8125a2d) will increase coverage by 0.03%.
The diff coverage is 97.82%.

@@            Coverage Diff             @@
##           master     #619      +/-   ##
==========================================
+ Coverage   96.47%   96.50%   +0.03%     
==========================================
  Files         357      357              
  Lines       20529    20632     +103     
==========================================
+ Hits        19805    19911     +106     
+ Misses        724      721       -3     
Impacted Files Coverage Δ
hazelcast/proxy/reliable_topic.py 94.60% <ø> (ø)
hazelcast/proxy/ringbuffer.py 97.94% <87.50%> (-1.32%) ⬇️
hazelcast/util.py 92.41% <91.66%> (+0.60%) ⬆️
hazelcast/protocol/builtin.py 94.42% <100.00%> (+0.07%) ⬆️
hazelcast/protocol/codec/sql_execute_codec.py 100.00% <100.00%> (ø)
hazelcast/protocol/codec/sql_fetch_codec.py 100.00% <100.00%> (ø)
hazelcast/proxy/list.py 99.57% <100.00%> (+<0.01%) ⬆️
hazelcast/proxy/map.py 98.10% <100.00%> (+0.27%) ⬆️
hazelcast/proxy/multi_map.py 99.23% <100.00%> (+0.01%) ⬆️
hazelcast/proxy/queue.py 99.50% <100.00%> (+<0.01%) ⬆️
... and 8 more

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@yuce yuce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, two minor comments.

@mdumandag mdumandag merged commit dbe73f5 into hazelcast:master Mar 24, 2023
@mdumandag mdumandag deleted the sql-eager-deserialization branch March 24, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants