Skip to content

IGNITE-17590 C++ 3.0: Implement RecordBinaryView#1287

Merged
isapego merged 57 commits intoapache:mainfrom
gridgain:ignite-17590
Nov 1, 2022
Merged

IGNITE-17590 C++ 3.0: Implement RecordBinaryView#1287
isapego merged 57 commits intoapache:mainfrom
gridgain:ignite-17590

Conversation

@isapego
Copy link
Contributor

@isapego isapego commented Oct 31, 2022

  • Implemented ignite_tuple;
  • Implemented record_view<ignite_tuple>;
  • Added tests;
  • Added some stubs (e.g. transaction) for future;
  • Fixed some bugs in binary tuple writing/parsing;
  • Used remove instead of delete in method names, as delete is reserved word in C++.

@isapego isapego requested a review from ptupitsyn October 31, 2022 23:17
* @param value Value.
*/
template<typename T>
void set(std::string_view name, T&& value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have ignite_tuple_builder, but the tuple itself is also mutable? I think we should pick one:

  • Builder + immutable Tuple with to_builder method
  • Mutable Tuple without builder (used in Java and .NET clients)

Copy link
Contributor Author

@isapego isapego Nov 1, 2022

Choose a reason for hiding this comment

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

I agree. Fixed.

ignite_callback<std::vector<std::optional<value_type>>> callback)
{
if (keys.empty())
throw ignite_error("At least one key should be supplied");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should return empty vector when keys are empty. At least this is what Java and .NET APIs do. And it may save the user some extra checks.

The same applies to all multi-record operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

DATETIME = 14, /**< A timezone-free datetime encoded into 8 bytes as (date, time). */
TIMESTAMP = 15, /**< Number of microseconds since Jan 1, 1970 00:00:00.000000 (with no
timezone) encoded into 10 bytes. */
NUMBER = 16, /**< Variable-length integer number (optionally bound by n bytes in size). */
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these explicit numbers have any significance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, they are actually used in thin client protocol when we are retrieving table schemas from server.

@isapego isapego requested review from ademakov and ptupitsyn and removed request for ademakov November 1, 2022 10:25
Copy link
Contributor

@ademakov ademakov left a comment

Choose a reason for hiding this comment

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

LGTM

@isapego isapego merged commit dee0292 into apache:main Nov 1, 2022
@isapego isapego deleted the ignite-17590 branch November 1, 2022 10:34
slukyano pushed a commit to slukyano/ignite-3 that referenced this pull request Nov 1, 2022
slukyano pushed a commit to slukyano/ignite-3 that referenced this pull request Nov 1, 2022
ivgag pushed a commit to unisonteam/ignite-3 that referenced this pull request Nov 4, 2022
slukyano pushed a commit to slukyano/ignite-3 that referenced this pull request Nov 7, 2022
lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Mar 18, 2023
lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Apr 19, 2023
isapego pushed a commit to isapego/ignite-3 that referenced this pull request Dec 26, 2024
Bumps `hadoop` from 3.4.0 to 3.4.1.

Updates `org.apache.hadoop:hadoop-common` from 3.4.0 to 3.4.1

Updates `org.apache.hadoop:hadoop-mapreduce-client-core` from 3.4.0 to 3.4.1

Updates `org.apache.hadoop:hadoop-aws` from 3.4.0 to 3.4.1

---
updated-dependencies:
- dependency-name: org.apache.hadoop:hadoop-common
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.hadoop:hadoop-mapreduce-client-core
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.hadoop:hadoop-aws
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: Aleksandr Polovtsev <alex.polovtcev@gmail.com>
Co-authored-by: Aleksandr Polovtsev <alex.polovtcev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants