Skip to content

Implement clone() and equals() methods for the nullable_T_vector structs #491

Merged
q10 merged 2 commits intomainfrom
bm-exchange-cpp-2
Feb 9, 2022
Merged

Implement clone() and equals() methods for the nullable_T_vector structs #491
q10 merged 2 commits intomainfrom
bm-exchange-cpp-2

Conversation

@q10
Copy link
Collaborator

@q10 q10 commented Feb 9, 2022

Summary:

  1. Redefine the nullable scalar vectors as instantiations of NullableScalarVec<T> for consistency of implementation
  2. Implement the clone() and equals() methods for all the nullable_T_vectors
  3. Add unit tests to check that these implementations are correct.
  4. Simplify the implementation of GroupingFunction.cloneVecStmt() to call the C++ member functions for struct copy().

See the example generated code for vastly simplfiied implementation of the exchange function for the zero key case.

The primary goal for this PR is to demonstrate that redefining the nullable scalar vectors as template instantiation of a common template class works. Additional work will follow to fold the rest of the code generated GroupingFunction.groupData into common C++ library code.

@q10 q10 force-pushed the bm-exchange-cpp-2 branch from 9f0d6c5 to 1d04ca7 Compare February 9, 2022 09:58
@q10 q10 marked this pull request as ready for review February 9, 2022 10:01
q10 added 2 commits February 9, 2022 10:57
- Add clone() methods to the nullable T vector classes to simplify
the code generation

- Add unit tests to ensure that the clone methods work correctly
- Rewite the clone operation in GroupingFunction to use the C++ clone()
methods to simplify the code generation
@q10 q10 force-pushed the bm-exchange-cpp-2 branch from 1d04ca7 to e055c2d Compare February 9, 2022 18:57
@q10 q10 marked this pull request as draft February 9, 2022 18:57
@q10 q10 marked this pull request as ready for review February 9, 2022 18:58
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@wmeddie wmeddie 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 to me.

@q10 q10 merged commit a6d9433 into main Feb 9, 2022
@q10 q10 deleted the bm-exchange-cpp-2 branch February 9, 2022 20:07
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.

2 participants