You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/06/15 00:53:45 UTC
[1/2] impala git commit: IMPALA-4908: NULL floats don't compare equal
to other NULL floats
Repository: impala
Updated Branches:
refs/heads/2.x f9f2a0127 -> 329979d6f
IMPALA-4908: NULL floats don't compare equal to other NULL floats
This change ensures that comparing two NULL floats with different
val fields returns true. Although this is undefined behavior, it
is now consistent with other types.
Along with the change, a unit test was added to ensure that
equality checking of floats returns results as expected.
Change-Id: Ie7310645e5752d8203be5abc22a6562a59b6e975
Reviewed-on: http://gerrit.cloudera.org:8080/10707
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/329979d6
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/329979d6
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/329979d6
Branch: refs/heads/2.x
Commit: 329979d6fb0caa0dc449d7e0aa75460c30e868f0
Parents: 08d3e10
Author: poojanilangekar <po...@cloudera.com>
Authored: Wed Jun 13 13:29:42 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Jun 14 21:10:19 2018 +0000
----------------------------------------------------------------------
be/src/udf/udf-test.cc | 25 +++++++++++++++++++++++++
be/src/udf/udf.h | 4 +++-
2 files changed, 28 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/329979d6/be/src/udf/udf-test.cc
----------------------------------------------------------------------
diff --git a/be/src/udf/udf-test.cc b/be/src/udf/udf-test.cc
index d16fb08..aac4a7f 100644
--- a/be/src/udf/udf-test.cc
+++ b/be/src/udf/udf-test.cc
@@ -250,6 +250,31 @@ TEST(UdfTest, TestDecimalVal) {
EXPECT_NE(null1.is_null, d1.is_null);
}
+TEST(UdfTest, TestFloatVal) {
+ FloatVal f1(1.0);
+ FloatVal f2(1.0);
+
+ // 1.0 == 1.0
+ EXPECT_EQ(f1, f2);
+
+ // convert to nulls
+ f1.is_null = true;
+ f2.is_null = true;
+ // nulls
+ EXPECT_EQ(f1, f2);
+
+ // change the value contained in one of the nulls
+ f1.val = 0.0;
+ // nulls
+ EXPECT_EQ(f1, f2);
+
+ // convert to non-nulls
+ f1.is_null = false;
+ f2.is_null = false;
+ // 0.0 != 1.0
+ EXPECT_NE(f1, f2);
+}
+
TEST(UdfTest, TestVarArgs) {
vector<StringVal> input;
input.push_back(StringVal("Hello"));
http://git-wip-us.apache.org/repos/asf/impala/blob/329979d6/be/src/udf/udf.h
----------------------------------------------------------------------
diff --git a/be/src/udf/udf.h b/be/src/udf/udf.h
index 26e1f45..62a1576 100644
--- a/be/src/udf/udf.h
+++ b/be/src/udf/udf.h
@@ -524,7 +524,9 @@ struct FloatVal : public AnyVal {
}
bool operator==(const FloatVal& other) const {
- return is_null == other.is_null && val == other.val;
+ if (is_null && other.is_null) return true;
+ if (is_null || other.is_null) return false;
+ return val == other.val;
}
bool operator!=(const FloatVal& other) const { return !(*this == other); }
};
[2/2] impala git commit: IMPALA-6835: Add table name and node id to
Kudu scanner errors
Posted by ta...@apache.org.
IMPALA-6835: Add table name and node id to Kudu scanner errors
Previously, the error messages in KuduScanner only contained the
reason for failure. They did not contain the KuduTable name or the
TPlanNode id which made it inconveient to debug. This change adds
the TPlanNode id to all error messages and the KuduTable name
whenever applicable.
This change was manually tested by explicitly returning failure
while scanning kudu tables.
Change-Id: I0377fc8591738dc45092d228fcf292ddbb367825
Reviewed-on: http://gerrit.cloudera.org:8080/10671
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/08d3e10a
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/08d3e10a
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/08d3e10a
Branch: refs/heads/2.x
Commit: 08d3e10a1b7ed7ed1ee6c10839a8e967559285d7
Parents: f9f2a01
Author: poojanilangekar <po...@cloudera.com>
Authored: Fri Jun 8 17:05:37 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Jun 14 21:10:19 2018 +0000
----------------------------------------------------------------------
be/src/exec/kudu-scanner.cc | 37 +++++++++++++++++++++++--------------
be/src/exec/kudu-scanner.h | 3 +++
be/src/exec/kudu-util.h | 1 +
3 files changed, 27 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/08d3e10a/be/src/exec/kudu-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-scanner.cc b/be/src/exec/kudu-scanner.cc
index 9e90bdb..e63a6fe 100644
--- a/be/src/exec/kudu-scanner.cc
+++ b/be/src/exec/kudu-scanner.cc
@@ -17,11 +17,11 @@
#include "exec/kudu-scanner.h"
+#include <string>
+#include <vector>
#include <kudu/client/row_result.h>
#include <kudu/client/value.h>
#include <thrift/protocol/TDebugProtocol.h>
-#include <vector>
-#include <string>
#include "exec/kudu-util.h"
#include "exprs/scalar-expr.h"
@@ -146,23 +146,26 @@ Status KuduScanner::OpenNextScanToken(const string& scan_token, bool* eos) {
DCHECK(scanner_ == NULL);
kudu::client::KuduScanner* scanner;
KUDU_RETURN_IF_ERROR(kudu::client::KuduScanToken::DeserializeIntoScanner(
- scan_node_->kudu_client(), scan_token, &scanner),
- "Unable to deserialize scan token");
+ scan_node_->kudu_client(), scan_token, &scanner),
+ BuildErrorString("Unable to deserialize scan token"));
scanner_.reset(scanner);
if (UNLIKELY(FLAGS_pick_only_leaders_for_tests)) {
KUDU_RETURN_IF_ERROR(scanner_->SetSelection(kudu::client::KuduClient::LEADER_ONLY),
- "Could not set replica selection.");
+ BuildErrorString("Could not set replica selection"));
}
kudu::client::KuduScanner::ReadMode mode =
MODE_READ_AT_SNAPSHOT == FLAGS_kudu_read_mode ?
kudu::client::KuduScanner::READ_AT_SNAPSHOT :
kudu::client::KuduScanner::READ_LATEST;
- KUDU_RETURN_IF_ERROR(scanner_->SetReadMode(mode), "Could not set scanner ReadMode");
+ KUDU_RETURN_IF_ERROR(
+ scanner_->SetReadMode(mode), BuildErrorString("Could not set scanner ReadMode"));
KUDU_RETURN_IF_ERROR(scanner_->SetTimeoutMillis(FLAGS_kudu_operation_timeout_ms),
- "Could not set scanner timeout");
- VLOG_ROW << "Starting KuduScanner with ReadMode=" << mode << " timeout=" <<
- FLAGS_kudu_operation_timeout_ms;
+ BuildErrorString("Could not set scanner timeout"));
+ VLOG_ROW << "Starting KuduScanner with ReadMode=" << mode
+ << " timeout=" << FLAGS_kudu_operation_timeout_ms
+ << " node with id=" << scan_node_->id()
+ << " Kudu table=" << scan_node_->table_->name();
if (!timestamp_slots_.empty()) {
uint64_t row_format_flags =
@@ -213,14 +216,14 @@ Status KuduScanner::OpenNextScanToken(const string& scan_token, bool* eos) {
KUDU_RETURN_IF_ERROR(
scanner_->AddConjunctPredicate(scan_node_->table_->NewComparisonPredicate(
col_name, KuduPredicate::ComparisonOp::GREATER_EQUAL, min_value)),
- "Failed to add min predicate");
+ BuildErrorString("Failed to add min predicate"));
KuduValue* max_value;
RETURN_IF_ERROR(CreateKuduValue(col_type, max, &max_value));
KUDU_RETURN_IF_ERROR(
scanner_->AddConjunctPredicate(scan_node_->table_->NewComparisonPredicate(
col_name, KuduPredicate::ComparisonOp::LESS_EQUAL, max_value)),
- "Failed to add max predicate");
+ BuildErrorString("Failed to add max predicate"));
}
}
}
@@ -228,12 +231,12 @@ Status KuduScanner::OpenNextScanToken(const string& scan_token, bool* eos) {
if (scan_node_->limit() != -1 && conjunct_evals_.empty()) {
KUDU_RETURN_IF_ERROR(scanner_->SetLimit(scan_node_->limit()),
- "Failed to set limit on scan.");
+ BuildErrorString("Failed to set limit on scan"));
}
{
SCOPED_TIMER(state_->total_storage_wait_timer());
- KUDU_RETURN_IF_ERROR(scanner_->Open(), "Unable to open scanner");
+ KUDU_RETURN_IF_ERROR(scanner_->Open(), BuildErrorString("Unable to open scanner"));
}
*eos = false;
return Status::OK();
@@ -340,7 +343,8 @@ Status KuduScanner::DecodeRowsIntoRowBatch(RowBatch* row_batch, Tuple** tuple_me
Status KuduScanner::GetNextScannerBatch() {
SCOPED_TIMER(state_->total_storage_wait_timer());
int64_t now = MonotonicMicros();
- KUDU_RETURN_IF_ERROR(scanner_->NextBatch(&cur_kudu_batch_), "Unable to advance iterator");
+ KUDU_RETURN_IF_ERROR(scanner_->NextBatch(&cur_kudu_batch_),
+ BuildErrorString("Unable to advance iterator"));
COUNTER_ADD(scan_node_->kudu_round_trips(), 1);
cur_kudu_batch_num_read_ = 0;
COUNTER_ADD(scan_node_->rows_read_counter(), cur_kudu_batch_.NumRows());
@@ -348,4 +352,9 @@ Status KuduScanner::GetNextScannerBatch() {
return Status::OK();
}
+string KuduScanner::BuildErrorString(const char* msg) {
+ return Substitute("$0 for node with id '$1' for Kudu table '$2'", msg, scan_node_->id(),
+ scan_node_->table_->name());
+}
+
} // namespace impala
http://git-wip-us.apache.org/repos/asf/impala/blob/08d3e10a/be/src/exec/kudu-scanner.h
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-scanner.h b/be/src/exec/kudu-scanner.h
index 5617847..2a44725 100644
--- a/be/src/exec/kudu-scanner.h
+++ b/be/src/exec/kudu-scanner.h
@@ -88,6 +88,9 @@ class KuduScanner {
return reinterpret_cast<Tuple*>(mem + scan_node_->tuple_desc()->byte_size());
}
+ /// Builds the error string by adding the PlanNode id and KuduTable to the message.
+ string BuildErrorString(const char* msg);
+
KuduScanNodeBase* scan_node_;
RuntimeState* state_;
http://git-wip-us.apache.org/repos/asf/impala/blob/08d3e10a/be/src/exec/kudu-util.h
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-util.h b/be/src/exec/kudu-util.h
index 9cf2891..8fe4d01 100644
--- a/be/src/exec/kudu-util.h
+++ b/be/src/exec/kudu-util.h
@@ -32,6 +32,7 @@ struct tm;
namespace impala {
/// Takes a Kudu status and returns an impala one, if it's not OK.
+/// Evaluates the prepend argument only if the status is not OK.
#define KUDU_RETURN_IF_ERROR(expr, prepend) \
do { \
kudu::Status _s = (expr); \