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

ARROW-6772: [C++] Add operator== for interfaces with an Equals() method #14038

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

benibus
Copy link
Collaborator

@benibus benibus commented Sep 2, 2022

Addresses ARROW-6772.

Adds util::EqualityComparable mixin to relevant types, replacing prior operator==() overloads if applicable.

Adds util::EqualityComparable mixin to relevant types, replacing prior
overloads if applicable.
@github-actions
Copy link

github-actions bot commented Sep 2, 2022

@github-actions
Copy link

github-actions bot commented Sep 2, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@pitrou
Copy link
Member

pitrou commented Sep 6, 2022

The RTools 40 mingw32 CI failure looked unrelated, so I restarted the job.

@benibus
Copy link
Collaborator Author

benibus commented Sep 6, 2022

For reference, I've hacked together a list of potentially similar classes that may be worth inspecting - i.e. they declare Equals() but don't appear to inherit the mixin.

  • arrow::Array
  • arrow::Buffer
  • arrow::ChunkedArray
  • arrow::Datum
  • arrow::KeyValueMetadata
  • arrow::RecordBatch
  • arrow::SparseCOOIndex
  • arrow::SparseCSFIndex
  • arrow::Table
  • arrow::Tensor
  • arrow::compute::ExecBatch
  • arrow::compute::Expression
  • arrow::compute::Forest
  • arrow::compute::KernelSignature
  • arrow::compute::TypeMatcher
  • arrow::dataset::FileFormat
  • arrow::flight::ActionType
  • arrow::flight::FlightDescriptor
  • arrow::flight::FlightEndpoint
  • arrow::flight::Location
  • arrow::flight::Ticket
  • arrow::fs::FileSystem
  • arrow::fs::GcsCredentials
  • arrow::fs::GcsOptions
  • arrow::fs::HdfsOptions
  • arrow::fs::LocalFileSystemOptions
  • arrow::ipc::Message

@pitrou
Copy link
Member

pitrou commented Sep 7, 2022

For reference, I've hacked together a list of potentially similar classes that may be worth inspecting - i.e. they declare Equals() but don't appear to inherit the mixin.

Thanks @benibus . Looking at this list, many of these classes can have quite expensive equality (for example Array::Equals would compare the entire array contents), so I'm not sure it would be a good idea to add an implicit operator==. @bkietz What do you think?

@pitrou
Copy link
Member

pitrou commented Sep 14, 2022

I'm gonna merge here since all is good. Thank you @benibus !

@pitrou pitrou merged commit bda98aa into apache:master Sep 14, 2022
@ursabot
Copy link

ursabot commented Sep 15, 2022

Benchmark runs are scheduled for baseline = bc8a608 and contender = bda98aa. bda98aa is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.37% ⬆️0.07%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.35% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] bda98aab ec2-t3-xlarge-us-east-2
[Finished] bda98aab test-mac-arm
[Failed] bda98aab ursa-i9-9960x
[Finished] bda98aab ursa-thinkcentre-m75q
[Finished] bc8a6082 ec2-t3-xlarge-us-east-2
[Finished] bc8a6082 test-mac-arm
[Failed] bc8a6082 ursa-i9-9960x
[Finished] bc8a6082 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
…od (apache#14038)

Addresses [ARROW-6772](https://issues.apache.org/jira/browse/ARROW-6772).

Adds util::EqualityComparable mixin to relevant types, replacing prior operator==() overloads if applicable.

Authored-by: benibus <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
fatemehp pushed a commit to fatemehp/arrow that referenced this pull request Oct 17, 2022
…od (apache#14038)

Addresses [ARROW-6772](https://issues.apache.org/jira/browse/ARROW-6772).

Adds util::EqualityComparable mixin to relevant types, replacing prior operator==() overloads if applicable.

Authored-by: benibus <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
@benibus benibus deleted the ARROW-6772-add-equality-operators branch February 17, 2023 22:10
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