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

AVRO-3051: C++ Reader and writers modernized #1153

Merged
merged 2 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions lang/c++/api/AvroTraits.hh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

#include "Config.hh"
#include "Types.hh"
#include <stdint.h>
#include <cstdint>
#include <type_traits>

/** @file
Expand Down Expand Up @@ -59,9 +59,11 @@ struct is_defined {

typedef char no[2];

template <class U> static yes& test(char(*)[sizeof(U)]) { throw 0; };
template<class U>
static yes &test(char(*)[sizeof(U)]) { throw 0; };

template <class U> static no& test(...) { throw 0; };
template<class U>
static no &test(...) { throw 0; };

static const bool value = sizeof(test<T>(0)) == sizeof(yes);
};
Expand All @@ -79,9 +81,11 @@ struct is_not_defined {

typedef char no[2];

template <class U> static yes& test(char(*)[sizeof(U)]) { throw 0; };
template<class U>
static yes &test(char(*)[sizeof(U)]) { throw 0; };

template <class U> static no& test(...) { throw 0; };
template<class U>
static no &test(...) { throw 0; };

static const bool value = sizeof(test<T>(0)) == sizeof(no);
};
Expand Down Expand Up @@ -110,7 +114,6 @@ DEFINE_PRIMITIVE(Null, AVRO_NULL)
DEFINE_PRIMITIVE(std::string, AVRO_STRING)
DEFINE_PRIMITIVE(std::vector<uint8_t>, AVRO_BYTES)


} // namespace avro

#endif
2 changes: 1 addition & 1 deletion lang/c++/api/Compiler.hh
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class AVRO_DECL InputStream;

class AVRO_DECL ValidSchema;

/// Given a stream comtaining a JSON schema, compiles the schema to a
/// Given a stream containing a JSON schema, compiles the schema to a
/// ValidSchema object. Throws if the schema cannot be compiled to a valid
/// schema

Expand Down
2 changes: 1 addition & 1 deletion lang/c++/api/Config.hh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#ifndef avro_Config_hh
#define avro_Config_hh

// Windows DLL suport
// Windows DLL support

#ifdef _WIN32
#pragma warning (disable: 4275 4251)
Expand Down
21 changes: 10 additions & 11 deletions lang/c++/api/DataFile.hh
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public:
/**
* Returns the byte offset (within the current file) of the start of the current block being written.
*/
uint64_t getCurrentBlockStart();
uint64_t getCurrentBlockStart() const;

/**
* Increments the object count.
Expand Down Expand Up @@ -172,7 +172,6 @@ public:
*/
uint64_t getCurrentBlockStart() { return base_->getCurrentBlockStart(); }


/**
* Closes the current file. Once closed this datafile object cannot be
* used for writing any more.
Expand Down Expand Up @@ -200,8 +199,8 @@ class AVRO_DECL DataFileReaderBase : boost::noncopyable {
int64_t objectCount_;
bool eof_;
Codec codec_;
int64_t blockStart_;
int64_t blockEnd_;
int64_t blockStart_{};
int64_t blockEnd_{};

ValidSchema readerSchema_;
ValidSchema dataSchema_;
Expand All @@ -210,7 +209,7 @@ class AVRO_DECL DataFileReaderBase : boost::noncopyable {
typedef std::map<std::string, std::vector<uint8_t> > Metadata;

Metadata metadata_;
DataFileSync sync_;
DataFileSync sync_{};

// for compressed buffer
std::unique_ptr<boost::iostreams::filtering_istream> os_;
Expand Down Expand Up @@ -242,9 +241,9 @@ public:
* This function should be called exactly once after constructing
* the DataFileReaderBase object.
*/
DataFileReaderBase(const char* filename);
explicit DataFileReaderBase(const char *filename);

DataFileReaderBase(std::unique_ptr<InputStream> inputStream);
explicit DataFileReaderBase(std::unique_ptr<InputStream> inputStream);

/**
* Initializes the reader so that the reader and writer schemas
Expand Down Expand Up @@ -297,7 +296,7 @@ public:
/**
* Return the last synchronization point before our current position.
*/
int64_t previousSync();
int64_t previousSync() const;
};

/**
Expand Down Expand Up @@ -325,12 +324,12 @@ public:
* Constructs the reader for the given file and the reader is
* expected to use the schema that is used with data.
*/
DataFileReader(const char* filename) :
explicit DataFileReader(const char *filename) :
base_(new DataFileReaderBase(filename)) {
base_->init();
}

DataFileReader(std::unique_ptr<InputStream> inputStream) :
explicit DataFileReader(std::unique_ptr<InputStream> inputStream) :
base_(new DataFileReaderBase(std::move(inputStream))) {
base_->init();
}
Expand All @@ -344,7 +343,7 @@ public:
* The schema present in the data file will be used for reading
* from this reader.
*/
DataFileReader(std::unique_ptr<DataFileReaderBase> base) : base_(std::move(base)) {
explicit DataFileReader(std::unique_ptr<DataFileReaderBase> base) : base_(std::move(base)) {
base_->init();
}

Expand Down
14 changes: 8 additions & 6 deletions lang/c++/api/Generic.hh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public:
/**
* Constructs a reader for the given schema using the given decoder.
*/
GenericReader(const ValidSchema& s, const DecoderPtr& decoder);
GenericReader(ValidSchema s, const DecoderPtr &decoder);

/**
* Constructs a reader for the given reader's schema \c readerSchema
Expand Down Expand Up @@ -75,7 +75,6 @@ public:
static void read(Decoder &d, GenericDatum &g, const ValidSchema &s);
};


/**
* A utility class to write generic datum to encoders.
*/
Expand All @@ -88,7 +87,7 @@ public:
/**
* Constructs a writer for the given schema using the given encoder.
*/
GenericWriter(const ValidSchema& s, const EncoderPtr& encoder);
GenericWriter(ValidSchema s, EncoderPtr encoder);

/**
* Writes a value onto the encoder.
Expand All @@ -109,14 +108,16 @@ public:
}
};

template <typename T> struct codec_traits;
template<typename T>
struct codec_traits;

/**
* Specialization of codec_traits for Generic datum along with its schema.
* This is maintained for compatibility with old code. Please use the
* cleaner codec_traits<GenericDatum> instead.
*/
template <> struct codec_traits<std::pair<ValidSchema, GenericDatum> > {
template<>
struct codec_traits<std::pair<ValidSchema, GenericDatum> > {
/** Encodes */
static void encode(Encoder &e,
const std::pair<ValidSchema, GenericDatum> &p) {
Expand All @@ -132,7 +133,8 @@ template <> struct codec_traits<std::pair<ValidSchema, GenericDatum> > {
/**
* Specialization of codec_traits for GenericDatum.
*/
template <> struct codec_traits<GenericDatum> {
template<>
struct codec_traits<GenericDatum> {
/** Encodes */
static void encode(Encoder &e, const GenericDatum &g) {
GenericWriter::write(e, g);
Expand Down
51 changes: 35 additions & 16 deletions lang/c++/api/GenericDatum.hh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#ifndef avro_GenericDatum_hh__
#define avro_GenericDatum_hh__

#include <stdint.h>
#include <cstdint>
#include <vector>
#include <map>
#include <string>
Expand Down Expand Up @@ -54,7 +54,7 @@ namespace avro {
* \li Avro <tt>array</tt> maps to C++ class <tt>GenericArray</tt>.
* \li Avro <tt>map</tt> maps to C++ class <tt>GenericMap</tt>.
* \li There is no C++ type corresponding to Avro <tt>union</tt>. The
* object should have the C++ type corresponing to one of the constituent
* object should have the C++ type corresponding to one of the constituent
* types of the union.
*
*/
Expand All @@ -68,7 +68,7 @@ protected:
boost::any value_;
#endif

GenericDatum(Type t)
explicit GenericDatum(Type t)
: type_(t), logicalType_(LogicalType::NONE) {}

GenericDatum(Type t, LogicalType logicalType)
Expand All @@ -95,7 +95,8 @@ public:
* T The type for the value. This must correspond to the
* avro type returned by type().
*/
template<typename T> const T& value() const;
template<typename T>
const T &value() const;

/**
* Returns the reference to the value held by this datum, which
Expand All @@ -106,7 +107,8 @@ public:
* T The type for the value. This must correspond to the
* avro type returned by type().
*/
template<typename T> T& value();
template<typename T>
T &value();

/**
* Returns true if and only if this datum is a union.
Expand All @@ -129,31 +131,45 @@ public:
GenericDatum() : type_(AVRO_NULL), logicalType_(LogicalType::NONE) {}

/// Makes a new AVRO_BOOL datum whose value is of type bool.
thiru-mg marked this conversation as resolved.
Show resolved Hide resolved
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(bool v)
: type_(AVRO_BOOL), logicalType_(LogicalType::NONE), value_(v) {}

/// Makes a new AVRO_INT datum whose value is of type int32_t.
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(int32_t v)
: type_(AVRO_INT), logicalType_(LogicalType::NONE), value_(v) {}

/// Makes a new AVRO_LONG datum whose value is of type int64_t.
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(int64_t v)
: type_(AVRO_LONG), logicalType_(LogicalType::NONE), value_(v) {}

/// Makes a new AVRO_FLOAT datum whose value is of type float.
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(float v)
: type_(AVRO_FLOAT), logicalType_(LogicalType::NONE), value_(v) {}

/// Makes a new AVRO_DOUBLE datum whose value is of type double.
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(double v)
: type_(AVRO_DOUBLE), logicalType_(LogicalType::NONE), value_(v) {}

/// Makes a new AVRO_STRING datum whose value is of type std::string.
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(const std::string &v)
: type_(AVRO_STRING), logicalType_(LogicalType::NONE), value_(v) {}

/// Makes a new AVRO_BYTES datum whose value is of type
/// std::vector<uint8_t>.
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(const std::vector<uint8_t> &v) :
type_(AVRO_BYTES), logicalType_(LogicalType::NONE), value_(v) {}

Expand All @@ -163,6 +179,8 @@ public:
* data type.
* \param schema The schema that defines the avro type.
*/
/// We don't make this explicit constructor because we want to allow automatic conversion
// NOLINTNEXTLINE(google-explicit-constructor)
GenericDatum(const NodePtr &schema);

/**
Expand All @@ -188,7 +206,7 @@ public:
* data type.
* \param schema The schema that defines the avro type.
*/
GenericDatum(const ValidSchema& schema);
explicit GenericDatum(const ValidSchema &schema);
};

/**
Expand Down Expand Up @@ -225,7 +243,7 @@ public:
* and the given value. The schema should be of Avro type union
* and the value should correspond to one of the branches of the union.
*/
GenericUnion(const NodePtr& schema) :
explicit GenericUnion(const NodePtr &schema) :
GenericContainer(AVRO_UNION, schema), curBranch_(schema->leaves()) {
selectBranch(0);
}
Expand Down Expand Up @@ -273,7 +291,7 @@ public:
* Constructs a generic record corresponding to the given schema \p schema,
* which should be of Avro type record.
*/
GenericRecord(const NodePtr& schema);
explicit GenericRecord(const NodePtr &schema);

/**
* Returns the number of fields in the current record.
Expand Down Expand Up @@ -355,7 +373,7 @@ public:
* Constructs a generic array corresponding to the given schema \p schema,
* which should be of Avro type array.
*/
GenericArray(const NodePtr& schema) : GenericContainer(AVRO_ARRAY, schema) {
explicit GenericArray(const NodePtr &schema) : GenericContainer(AVRO_ARRAY, schema) {
}

/**
Expand Down Expand Up @@ -389,7 +407,7 @@ public:
* Constructs a generic map corresponding to the given schema \p schema,
* which should be of Avro type map.
*/
GenericMap(const NodePtr& schema) : GenericContainer(AVRO_MAP, schema) {
explicit GenericMap(const NodePtr &schema) : GenericContainer(AVRO_MAP, schema) {
}

/**
Expand Down Expand Up @@ -428,7 +446,7 @@ public:
* Constructs a generic enum corresponding to the given schema \p schema,
* which should be of Avro type enum.
*/
GenericEnum(const NodePtr& schema) :
explicit GenericEnum(const NodePtr &schema) :
GenericContainer(AVRO_ENUM, schema), value_(0) {
}

Expand Down Expand Up @@ -498,12 +516,11 @@ public:
* Constructs a generic enum corresponding to the given schema \p schema,
* which should be of Avro type fixed.
*/
GenericFixed(const NodePtr& schema) : GenericContainer(AVRO_FIXED, schema) {
explicit GenericFixed(const NodePtr &schema) : GenericContainer(AVRO_FIXED, schema) {
value_.resize(schema->fixedSize());
}

GenericFixed(const NodePtr& schema, const std::vector<uint8_t>& v) :
GenericContainer(AVRO_FIXED, schema), value_(v) { }
GenericFixed(const NodePtr &schema, const std::vector<uint8_t> &v);

/**
* Returns the contents of this fixed.
Expand Down Expand Up @@ -534,7 +551,8 @@ inline LogicalType GenericDatum::logicalType() const {
return logicalType_;
}

template<typename T> T& GenericDatum::value() {
template<typename T>
T &GenericDatum::value() {
return (type_ == AVRO_UNION) ?
#if __cplusplus >= 201703L
std::any_cast<GenericUnion>(&value_)->datum().value<T>() :
Expand All @@ -545,7 +563,8 @@ template<typename T> T& GenericDatum::value() {
#endif
}

template<typename T> const T& GenericDatum::value() const {
template<typename T>
const T &GenericDatum::value() const {
return (type_ == AVRO_UNION) ?
#if __cplusplus >= 201703L
std::any_cast<GenericUnion>(&value_)->datum().value<T>() :
Expand Down