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

query_deserialize() transaction #1437

Merged
merged 1 commit into from
Nov 19, 2019
Merged

Conversation

joe-maley
Copy link
Contributor

This change ensures that if query_deserialize() returns
a non-OK status, both 'copy_state' and 'query' args are
in the same state that they entered the routine in.

Changes:

  1. The existing query_deserialize() implementation has
    been moved to do_query_deserialize().
  2. query_deserialize() maintains the same interface.
  3. query_deserialize() makes a copy of the
    'copy_state'.
  4. query_deserialize() serializes 'query' into a backup
    buffer.
  5. query_deserialize() invokes do_query_deserialize(). If
    it fails, query_deserialize() will reset 'copy_state' to
    the copy in change Added new attribute and coordinate types #3 and it will deserialize the backup
    buffer from change Fixed bug with real coordinates when computing the tile order. #4 into 'query.
  6. Reverts the logical changes in Allow incomplete query returns when user buffer is too small #1424 because we now handle
    all deserialization failures, not just user buffer overflow.

@joe-maley joe-maley requested a review from tdenniston November 14, 2019 21:18
@joe-maley joe-maley force-pushed the jpm/deserialization-transaction branch 5 times, most recently from 2dca4e9 to 0d4d7ba Compare November 15, 2019 18:41
Copy link
Contributor

@tdenniston tdenniston left a comment

Choose a reason for hiding this comment

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

Minor comments, and some CI failures.

tiledb/sm/misc/logger.h Show resolved Hide resolved
tiledb/sm/rest/rest_client.cc Outdated Show resolved Hide resolved
tiledb/sm/serialization/query.cc Show resolved Hide resolved
tiledb/sm/serialization/query.cc Outdated Show resolved Hide resolved
tiledb/sm/serialization/query.h Show resolved Hide resolved
@joe-maley joe-maley force-pushed the jpm/deserialization-transaction branch 4 times, most recently from 7313c21 to 1ec1a4f Compare November 18, 2019 19:37
@joe-maley joe-maley force-pushed the jpm/deserialization-transaction branch from 1ec1a4f to b989ecc Compare November 18, 2019 20:18
This change ensures that if query_deserialize() returns
a non-OK status, both 'copy_state' and 'query' args are
in the same state that they entered the routine in.

Changes:
1. The existing query_deserialize() implementation has
   been moved to do_query_deserialize().
2. query_deserialize() maintains the same interface.
3. query_deserialize() makes a copy of the
   'copy_state'.
4. query_deserialize() serializes 'query' into a backup
   buffer.
5. query_deserialize() invokes do_query_deserialize(). If
   it fails, query_deserialize() will reset 'copy_state' to
   the copy in change #3 and it will deserialize the backup
   buffer from change #4 into 'query.
6. Reverts the logical changes in #1424 because we now handle
   all deserialization failures, not just user buffer overflow.
@joe-maley joe-maley force-pushed the jpm/deserialization-transaction branch from b989ecc to f41fd6b Compare November 19, 2019 14:11
@joe-maley joe-maley merged commit 22dac98 into dev Nov 19, 2019
@tdenniston tdenniston added backport tags commits to backport to release branch rest serialization labels Nov 19, 2019
@tdenniston tdenniston modified the milestones: 1.7.1, 1.7.2 Nov 19, 2019
ihnorton pushed a commit that referenced this pull request Dec 4, 2019
This change ensures that if query_deserialize() returns
a non-OK status, both 'copy_state' and 'query' args are
in the same state that they entered the routine in.

Changes:
1. The existing query_deserialize() implementation has
   been moved to do_query_deserialize().
2. query_deserialize() maintains the same interface.
3. query_deserialize() makes a copy of the
   'copy_state'.
4. query_deserialize() serializes 'query' into a backup
   buffer.
5. query_deserialize() invokes do_query_deserialize(). If
   it fails, query_deserialize() will reset 'copy_state' to
   the copy in change #3 and it will deserialize the backup
   buffer from change #4 into 'query.
6. Reverts the logical changes in #1424 because we now handle
   all deserialization failures, not just user buffer overflow.

(cherry picked from commit f41fd6b
 #1437)
@Shelnutt2 Shelnutt2 modified the milestones: 1.7.2, 1.7.3 Dec 6, 2019
@ihnorton ihnorton removed the backport tags commits to backport to release branch label Jan 8, 2020
@joe-maley joe-maley deleted the jpm/deserialization-transaction branch January 27, 2020 17: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.

4 participants