andishgar commented on code in PR #47204:
URL: https://github.com/apache/arrow/pull/47204#discussion_r2311965810
##########
cpp/src/arrow/record_batch.h:
##########
@@ -118,22 +118,32 @@ class ARROW_EXPORT RecordBatch {
static Result<std::shared_ptr<RecordBatch>> FromStructArray(
const std::shared_ptr<Array>& array, MemoryPool* pool =
default_memory_pool());
- /// \brief Determine if two record batches are exactly equal
+ /// \brief Determine if two record batches are equal.
///
/// \param[in] other the RecordBatch to compare with
- /// \param[in] check_metadata if true, check that Schema metadata is the same
+ /// \param[in] check_metadata If true, the schema metadata will be compared,
+ /// regardless of the value set in \ref
EqualOptions::use_metadata_.
/// \param[in] opts the options for equality comparisons
/// \return true if batches are equal
bool Equals(const RecordBatch& other, bool check_metadata = false,
const EqualOptions& opts = EqualOptions::Defaults()) const;
+ /// \brief Determine if two record batches are equal.
+ ///
+ /// \param[in] other the RecordBatch to compare with
+ /// \param[in] opts the options for equality comparisons
+ /// \return true if batches are equal
+ bool Equals(const RecordBatch& other, const EqualOptions& opts) const;
+
/// \brief Determine if two record batches are approximately equal
///
/// \param[in] other the RecordBatch to compare with
/// \param[in] opts the options for equality comparisons
/// \return true if batches are approximately equal
bool ApproxEquals(const RecordBatch& other,
- const EqualOptions& opts = EqualOptions::Defaults()) const;
+ const EqualOptions& opts = EqualOptions::Defaults()) const
{
+ return Equals(other, opts.use_schema(false).use_atol(true));
+ }
Review Comment:
>Moving ApproxEquals to be inline changes the API structure and could affect
binary compatibility.
1-My general assumption is that, since there is a major release every three
months, this should not be a problem. Am I right, or should we apply the
suggestion?
> The inline implementation also hardcodes use_schema(false) which may not
align with user expectations - users might want to compare schemas in
approximate equality checks.
2-should we ignore this, given that the default behavior of
arrow::RecordBatch::ApproxEquals already ignores the schema?
https://github.com/apache/arrow/blob/ed77d25149569eb9a48f61f3694fd8ea4b9a411d/cpp/src/arrow/record_batch.cc#L375-L395
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
