Skip to content

Commit

Permalink
Use implicit pointer/optional-bool conversion wherever possible (hyri…
Browse files Browse the repository at this point in the history
  • Loading branch information
Moritz Eyssen authored and mrks committed Mar 28, 2019
1 parent 38aab7f commit 6ef2a54
Show file tree
Hide file tree
Showing 56 changed files with 147 additions and 146 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Avoid exception handling. Because Hyrise is not a product, we do not have to rec
- Use C++11 for loops when possible: `for (const auto& item : items) {...}`
- When creating a vector where you know the size beforehand, use `reserve` to avoid unnecessary resizes and allocations
- Don’t evaluate end() every time through a loop: http://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop
- Use `if (object) {` over `if (object != nullptr) {` or `if (object.has_value()) {`


## Naming Conventions
Expand Down
4 changes: 2 additions & 2 deletions src/benchmarklib/table_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ std::shared_ptr<Table> TableGenerator::generate_table(const ChunkID chunk_size,
table->append_chunk(segments);
}

if (encoding_type.has_value()) {
if (encoding_type) {
ChunkEncoder::encode_all_chunks(table, encoding_type.value());
}

Expand Down Expand Up @@ -189,7 +189,7 @@ std::shared_ptr<Table> TableGenerator::generate_table(
}
}

if (encoding_type.has_value()) {
if (encoding_type) {
ChunkEncoder::encode_all_chunks(table, SegmentEncodingSpec{encoding_type.value()});
}

Expand Down
2 changes: 1 addition & 1 deletion src/benchmarklib/tpch/tpch_table_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ void dbgen_cleanup() {
distribution->permute = nullptr;
}

if (asc_date != nullptr) {
if (asc_date) {
for (size_t idx = 0; idx < TOTDATE; ++idx) {
free(asc_date[idx]); // NOLINT
}
Expand Down
17 changes: 8 additions & 9 deletions src/bin/console/console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ int Console::read() {

// Prompt user for input
buffer = readline(_prompt.c_str());
if (buffer == nullptr) {
if (!buffer) {
return ReturnCode::Quit;
}

Expand Down Expand Up @@ -242,7 +242,7 @@ int Console::_eval_command(const CommandFunction& func, const std::string& comma
bool Console::_initialize_pipeline(const std::string& sql) {
try {
auto builder = SQLPipelineBuilder{sql}.dont_cleanup_temporaries(); // keep tables for debugging and visualization
if (_explicitly_created_transaction_context != nullptr) {
if (_explicitly_created_transaction_context) {
builder.with_transaction_context(_explicitly_created_transaction_context);
}
_sql_pipeline = std::make_unique<SQLPipeline>(builder.create_pipeline());
Expand All @@ -264,8 +264,7 @@ int Console::_eval_sql(const std::string& sql) {
"executed at a time.");
} catch (const InvalidInputException& exception) {
out(std::string(exception.what()) + "\n");
if (_handle_rollback() && _explicitly_created_transaction_context == nullptr &&
_sql_pipeline->statement_count() > 1) {
if (_handle_rollback() && !_explicitly_created_transaction_context && _sql_pipeline->statement_count() > 1) {
out("All previous statements have been committed.\n");
}
return ReturnCode::Error;
Expand Down Expand Up @@ -427,7 +426,7 @@ int Console::_generate_tpcc(const std::string& tablename) {

out("Generating TPCC table: \"" + tablename + "\" ...\n");
auto table = TpccTableGenerator().generate_table(tablename);
if (table == nullptr) {
if (!table) {
out("Error: No TPCC table named \"" + tablename + "\" available.\n");
return ReturnCode::Error;
}
Expand Down Expand Up @@ -841,7 +840,7 @@ void Console::handle_signal(int sig) {
}

int Console::_begin_transaction(const std::string& input) {
if (_explicitly_created_transaction_context != nullptr) {
if (_explicitly_created_transaction_context) {
const auto transaction_id = std::to_string(_explicitly_created_transaction_context->transaction_id());
out("Error: There is already an active transaction (" + transaction_id + "). ");
out("Type `rollback` or `commit` before beginning a new transaction.\n");
Expand All @@ -856,7 +855,7 @@ int Console::_begin_transaction(const std::string& input) {
}

int Console::_rollback_transaction(const std::string& input) {
if (_explicitly_created_transaction_context == nullptr) {
if (!_explicitly_created_transaction_context) {
out("Console is in auto-commit mode. Type `begin` to start a manual transaction.\n");
return ReturnCode::Error;
}
Expand All @@ -871,7 +870,7 @@ int Console::_rollback_transaction(const std::string& input) {
}

int Console::_commit_transaction(const std::string& input) {
if (_explicitly_created_transaction_context == nullptr) {
if (!_explicitly_created_transaction_context) {
out("Console is in auto-commit mode. Type `begin` to start a manual transaction.\n");
return ReturnCode::Error;
}
Expand All @@ -886,7 +885,7 @@ int Console::_commit_transaction(const std::string& input) {
}

int Console::_print_transaction_info(const std::string& input) {
if (_explicitly_created_transaction_context == nullptr) {
if (!_explicitly_created_transaction_context) {
out("Console is in auto-commit mode. Type `begin` to start a manual transaction.\n");
return ReturnCode::Error;
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/concurrency/transaction_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ TransactionID TransactionContext::transaction_id() const { return _transaction_i
CommitID TransactionContext::snapshot_commit_id() const { return _snapshot_commit_id; }

CommitID TransactionContext::commit_id() const {
Assert((_commit_context != nullptr), "TransactionContext cid only available after commit context has been created.");
Assert(_commit_context, "TransactionContext cid only available after commit context has been created.");

return _commit_context->commit_id();
}
Expand Down
5 changes: 2 additions & 3 deletions src/lib/expression/evaluation/expression_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -716,9 +716,8 @@ std::shared_ptr<ExpressionResult<Result>> ExpressionEvaluator::_evaluate_value_o
value = value_expression.value;
} else {
const auto& correlated_parameter_expression = dynamic_cast<const CorrelatedParameterExpression*>(&expression);
Assert(correlated_parameter_expression, "ParameterExpression not a CorrelatedParameterExpression")
Assert(correlated_parameter_expression->value().has_value(),
"CorrelatedParameterExpression: Value not set, cannot evaluate");
Assert(correlated_parameter_expression, "ParameterExpression not a CorrelatedParameterExpression") Assert(
correlated_parameter_expression->value(), "CorrelatedParameterExpression: Value not set, cannot evaluate");
value = *correlated_parameter_expression->value();
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/expression/expression_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ DataType expression_common_type(const DataType lhs, const DataType rhs);
* @return Checks whether the expression can be evaluated by the ExpressionEvaluator on top of a specified LQP (i.e.,
* all required LQPColumnExpressions are available from this LQP).
* To check if an expression is available in a form ready to be used by a scan/join,
* use `Operator*Predicate::from_expression(...) != nullptr`.
* use `Operator*Predicate::from_expression(...)`.
*/
bool expression_evaluable_on_lqp(const std::shared_ptr<AbstractExpression>& expression, const AbstractLQPNode& lqp);

Expand Down
4 changes: 2 additions & 2 deletions src/lib/logical_query_plan/lqp_translator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,13 @@ std::shared_ptr<AbstractOperator> LQPTranslator::_translate_validate_node(

std::shared_ptr<AbstractOperator> LQPTranslator::_translate_show_tables_node(
const std::shared_ptr<AbstractLQPNode>& node) const {
DebugAssert(node->left_input() == nullptr, "ShowTables should not have an input operator.");
DebugAssert(!node->left_input(), "ShowTables should not have an input operator.");
return std::make_shared<ShowTables>();
}

std::shared_ptr<AbstractOperator> LQPTranslator::_translate_show_columns_node(
const std::shared_ptr<AbstractLQPNode>& node) const {
DebugAssert(node->left_input() == nullptr, "ShowColumns should not have an input operator.");
DebugAssert(!node->left_input(), "ShowColumns should not have an input operator.");
const auto show_columns_node = std::dynamic_pointer_cast<ShowColumnsNode>(node);
return std::make_shared<ShowColumns>(show_columns_node->table_name);
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/memory/numa_memory_resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void NUMAMemoryResource::do_deallocate(void* p, std::size_t bytes, std::size_t a

bool NUMAMemoryResource::do_is_equal(const memory_resource& other) const noexcept {
const auto other_numa_resource = dynamic_cast<const NUMAMemoryResource*>(&other);
if (other_numa_resource == nullptr) {
if (!other_numa_resource) {
return false;
} else {
return other_numa_resource->_memory_source == _memory_source;
Expand Down
6 changes: 3 additions & 3 deletions src/lib/operators/abstract_operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void AbstractOperator::execute() {
std::shared_ptr<const Table> AbstractOperator::get_output() const {
DebugAssert(
[&]() {
if (_output == nullptr) return true;
if (!_output) return true;
if (_output->chunk_count() <= ChunkID{1}) return true;
for (auto chunk_id = ChunkID{0}; chunk_id < _output->chunk_count(); ++chunk_id) {
if (_output->get_chunk(chunk_id)->size() < 1) return true;
Expand Down Expand Up @@ -107,8 +107,8 @@ void AbstractOperator::set_transaction_context_recursively(
const std::weak_ptr<TransactionContext>& transaction_context) {
set_transaction_context(transaction_context);

if (_input_left != nullptr) mutable_input_left()->set_transaction_context_recursively(transaction_context);
if (_input_right != nullptr) mutable_input_right()->set_transaction_context_recursively(transaction_context);
if (_input_left) mutable_input_left()->set_transaction_context_recursively(transaction_context);
if (_input_right) mutable_input_right()->set_transaction_context_recursively(transaction_context);
}

std::shared_ptr<AbstractOperator> AbstractOperator::mutable_input_left() const {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/operators/index_scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ PosList IndexScan::_scan_chunk(const ChunkID chunk_id) {
auto matches_out = PosList{};

const auto index = chunk->get_index(_index_type, _left_column_ids);
Assert(index != nullptr, "Index of specified type not found for segment (vector).");
Assert(index, "Index of specified type not found for segment (vector).");

switch (_predicate_condition) {
case PredicateCondition::Equals: {
Expand Down
22 changes: 11 additions & 11 deletions src/lib/operators/jit_operator/jit_operations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ std::optional<bool> jit_not(const JitExpression& left_side, JitRuntimeContext& c
(left_side.result_entry().data_type() == DataType::Bool || left_side.result_entry().data_type() == DataType::Int),
"invalid type for jit operation not");
const auto value = left_side.compute<bool>(context);
if (left_side.result_entry().is_nullable() && !value.has_value()) {
if (left_side.result_entry().is_nullable() && !value) {
return std::nullopt;
} else {
return !value.value();
Expand All @@ -62,25 +62,25 @@ std::optional<bool> jit_and(const JitExpression& left_side, const JitExpression&

const auto left_result = left_side.compute<bool>(context);
// Computation of right hand side can be pruned if left result is false and not null
if (!left_entry.is_nullable() || left_result.has_value()) { // Left result is not null
if (!left_result.value()) { // Left result is false
if (!left_entry.is_nullable() || left_result) { // Left result is not null
if (!left_result.value()) { // Left result is false
return false;
}
}

// Left result is null or true
const auto right_result = right_side.compute<bool>(context);
if (left_entry.is_nullable() && !left_result.has_value()) { // Left result is null
if (left_entry.is_nullable() && !left_result) { // Left result is null
// Right result is null or true
if ((right_entry.is_nullable() && !right_result.has_value()) || right_result.value()) {
if ((right_entry.is_nullable() && !right_result) || right_result.value()) {
return std::nullopt;
} else { // Right result is false
return false;
}
}

// Left result is false and not null
if (right_entry.is_nullable() && !right_result.has_value()) {
if (right_entry.is_nullable() && !right_result) {
return std::nullopt;
} else {
return right_result.value();
Expand All @@ -104,25 +104,25 @@ std::optional<bool> jit_or(const JitExpression& left_side, const JitExpression&

const auto left_result = left_side.compute<bool>(context);
// Computation of right hand side can be pruned if left result is true and not null
if (!left_entry.is_nullable() || left_result.has_value()) { // Left result is not null
if (left_result.value()) { // Left result is true
if (!left_entry.is_nullable() || left_result) { // Left result is not null
if (left_result.value()) { // Left result is true
return true;
}
}

// Left result is null or false
const auto right_result = right_side.compute<bool>(context);
if (left_entry.is_nullable() && !left_result.has_value()) { // Left result is null
if (left_entry.is_nullable() && !left_result) { // Left result is null
// Right result is null or false
if ((right_entry.is_nullable() && !right_result.has_value()) || !right_result.value()) {
if ((right_entry.is_nullable() && !right_result) || !right_result.value()) {
return std::nullopt;
} else { // Right result is true
return true;
}
}

// Left result is false and not null
if (right_entry.is_nullable() && !right_result.has_value()) {
if (right_entry.is_nullable() && !right_result) {
return std::nullopt;
} else {
return right_result.value();
Expand Down
2 changes: 1 addition & 1 deletion src/lib/operators/jit_operator/jit_operations.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ std::optional<ResultValueType> jit_compute(const T& op_func, const JitExpression
const auto store_result_wrapper = [&](const auto& typed_lhs, const auto& typed_rhs)
-> std::optional<decltype(op_func(typed_lhs.value(), typed_rhs.value()), ResultValueType{})> {
// Handle NULL values and return NULL if either input is NULL.
if ((left_entry.is_nullable() && !typed_lhs.has_value()) || (right_entry.is_nullable() && !typed_rhs.has_value())) {
if ((left_entry.is_nullable() && !typed_lhs) || (right_entry.is_nullable() && !typed_rhs)) {
return std::nullopt;
}

Expand Down
4 changes: 2 additions & 2 deletions src/lib/operators/jit_operator/operators/jit_expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ namespace opossum {
#define JIT_EXPRESSION_COMPUTE_CASE(r, types) \
case JIT_GET_ENUM_VALUE(0, types): { \
const auto result = compute<JIT_GET_DATA_TYPE(0, types)>(context); \
if (!_result_entry.is_nullable() || result.has_value()) { \
if (!_result_entry.is_nullable() || result) { \
_result_entry.set<JIT_GET_DATA_TYPE(0, types)>(result.value(), context); \
} \
if (_result_entry.is_nullable()) { \
_result_entry.set_is_null(!result.has_value(), context); \
_result_entry.set_is_null(!result, context); \
} \
break; \
}
Expand Down
2 changes: 2 additions & 0 deletions src/lib/operators/jit_operator/operators/jit_expression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ class JitExpression {

/* The compute<ResultValueType>() function directly returns the result and does not store it in the runtime tuple. The
* ResultValueType function template parameter specifies the returned type of the result.
* If the result entry is nullable, the result value can be a nullopt (i.e. it is null) which requires an is null
* check (has_value()).
*/
template <typename ResultValueType>
std::optional<ResultValueType> compute(JitRuntimeContext& context) const;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/operators/jit_operator/operators/jit_filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ std::shared_ptr<const JitExpression> JitFilter::expression() { return _expressio

void JitFilter::_consume(JitRuntimeContext& context) const {
const auto result = _expression->compute<bool>(context);
if ((!_expression->result_entry().is_nullable() || result.has_value()) && result.value()) {
if ((!_expression->result_entry().is_nullable() || result) && result.value()) {
_emit(context);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ void JitWriteReferences::after_chunk(const std::shared_ptr<const Table>& in_tabl
auto segment_in = chunk_in->get_segment(output_column.referenced_column_id);

auto ref_segment_in = std::dynamic_pointer_cast<const ReferenceSegment>(segment_in);
DebugAssert(ref_segment_in != nullptr, "All segments should be of type ReferenceSegment.");
DebugAssert(ref_segment_in, "All segments should be of type ReferenceSegment.");

const auto pos_list_in = ref_segment_in->pos_list();

Expand Down
2 changes: 1 addition & 1 deletion src/lib/operators/join_hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class JoinHash::JoinHashImpl : public AbstractJoinOperatorImpl {
_predicate_condition(predicate_condition),
_inputs_swapped(inputs_swapped),
_secondary_join_predicates(std::move(secondary_join_predicates)) {
if (radix_bits.has_value()) {
if (radix_bits) {
_radix_bits = radix_bits.value();
} else {
_radix_bits = _calculate_radix_bits();
Expand Down
4 changes: 2 additions & 2 deletions src/lib/operators/join_hash/join_hash_steps.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ void probe(const RadixContainer<RightType>& radix_container,
"Hash join probe called with NULL consideration but inputs do not store any NULL value information");
}

if (hash_tables[current_partition_id].has_value()) {
if (hash_tables[current_partition_id]) {
const auto& hash_table = hash_tables.at(current_partition_id).value();

// simple heuristic to estimate result size: half of the partition's rows will match
Expand Down Expand Up @@ -528,7 +528,7 @@ void probe_semi_anti(const RadixContainer<RightType>& radix_container,

PosList pos_list_local;

if (hash_tables[current_partition_id].has_value()) {
if (hash_tables[current_partition_id]) {
// Valid hashtable found, so there is at least one match in this partition

for (size_t partition_offset = partition_begin; partition_offset < partition_end; ++partition_offset) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/operators/join_index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void JoinIndex::_perform_join() {
}

// Scan all chunks from left input
if (index != nullptr) {
if (index) {
for (ChunkID chunk_id_left = ChunkID{0}; chunk_id_left < input_table_left()->chunk_count(); ++chunk_id_left) {
const auto segment_left =
input_table_left()->get_chunk(chunk_id_left)->get_segment(_primary_predicate.column_ids.first);
Expand Down
4 changes: 2 additions & 2 deletions src/lib/operators/join_mpsm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ JoinMPSM::JoinMPSM(const std::shared_ptr<const AbstractOperator>& left,
: AbstractJoinOperator(OperatorType::JoinMPSM, left, right, mode, primary_predicate, {}) {
// Validate the parameters
DebugAssert(mode != JoinMode::Cross, "This operator does not support cross joins.");
DebugAssert(left != nullptr, "The left input operator is null.");
DebugAssert(right != nullptr, "The right input operator is null.");
DebugAssert(left, "The left input operator is null.");
DebugAssert(right, "The right input operator is null.");
DebugAssert(primary_predicate.predicate_condition == PredicateCondition::Equals,
"Only Equi joins are supported by MPSM join.");
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/operators/join_mpsm/column_materializer_numa.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class ColumnMaterializerNUMA {

// Find out whether we actually are on a NUMA System, if so, remember the numa node
auto numa_res = dynamic_cast<NUMAMemoryResource*>(alloc.resource());
if (numa_res != nullptr) {
if (numa_res) {
numa_node_id = NodeID{static_cast<uint32_t>(numa_res->get_node_id())};
}

Expand Down
4 changes: 2 additions & 2 deletions src/lib/operators/join_mpsm/radix_cluster_sort_numa.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ class RadixClusterSortNUMA {
_materialize_null_right{materialize_null_right} {
DebugAssert(cluster_count > 0, "cluster_count must be > 0");
DebugAssert((cluster_count & (cluster_count - 1)) == 0, "cluster_count must be a power of two, i.e. 1, 2, 4, 8...");
DebugAssert(left != nullptr, "left input operator is null");
DebugAssert(right != nullptr, "right input operator is null");
DebugAssert(left, "left input operator is null");
DebugAssert(right, "right input operator is null");
}

virtual ~RadixClusterSortNUMA() = default;
Expand Down
Loading

0 comments on commit 6ef2a54

Please sign in to comment.