You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/03/01 19:12:22 UTC

[1/2] kudu git commit: KUDU-1851 - [python] TableAlterer direct instantiation causes SIGSEGV

Repository: kudu
Updated Branches:
  refs/heads/branch-1.2.x 32d4d5f3c -> 1af814b09


KUDU-1851 - [python] TableAlterer direct instantiation causes SIGSEGV

The python client is currently crashing when a TableAlterer is directly
instantiated as opposed to instantiating via the new_table_alterer method
in the Client class. This patch addresses this issue and includes a new
unit test.

Change-Id: I074737c8f6992a7cd21f72f711337e915ffe2e82
Reviewed-on: http://gerrit.cloudera.org:8080/5822
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
(cherry picked from commit 2882f0f3e255ed28769aa444f4d999ff796ebd7d)
Reviewed-on: http://gerrit.cloudera.org:8080/6203
Tested-by: Kudu Jenkins
Reviewed-by: Jordan Birdsell <jt...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/3b05eee4
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/3b05eee4
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/3b05eee4

Branch: refs/heads/branch-1.2.x
Commit: 3b05eee4393765fb3cbf5b18fa0f1447b1d22059
Parents: 32d4d5f
Author: Jordan Birdsell <jt...@apache.org>
Authored: Sat Jan 28 11:17:28 2017 -0500
Committer: Jordan Birdsell <jt...@apache.org>
Committed: Wed Mar 1 18:58:51 2017 +0000

----------------------------------------------------------------------
 python/kudu/client.pyx           |  8 +++-----
 python/kudu/tests/test_client.py | 21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/3b05eee4/python/kudu/client.pyx
----------------------------------------------------------------------
diff --git a/python/kudu/client.pyx b/python/kudu/client.pyx
index ab6a70a..c95844d 100644
--- a/python/kudu/client.pyx
+++ b/python/kudu/client.pyx
@@ -544,11 +544,7 @@ cdef class Client:
         -------
         alterer : TableAlterer
         """
-        cdef:
-            TableAlterer alterer = TableAlterer(table)
-
-        alterer._init(self.cp.NewTableAlterer(tobytes(table.name)))
-        return alterer
+        return TableAlterer(table)
 
 
 #----------------------------------------------------------------------
@@ -2563,6 +2559,8 @@ cdef class TableAlterer:
     def __cinit__(self, Table table):
         self._table = table
         self._new_name = None
+        self._init(self._table.parent.cp
+                   .NewTableAlterer(tobytes(self._table.name)))
 
     def __dealloc__(self):
         if self._alterer != NULL:

http://git-wip-us.apache.org/repos/asf/kudu/blob/3b05eee4/python/kudu/tests/test_client.py
----------------------------------------------------------------------
diff --git a/python/kudu/tests/test_client.py b/python/kudu/tests/test_client.py
index 3183051..779cd25 100644
--- a/python/kudu/tests/test_client.py
+++ b/python/kudu/tests/test_client.py
@@ -345,6 +345,27 @@ class TestClient(KuduTestBase, unittest.TestCase):
         with self.assertRaises(KeyError):
             col = table['added-column']
 
+    def test_alter_table_direct_instantiation(self):
+        # Run the add_drop_column test with direct instantiation of
+        # the TableAlterer
+        table = self.client.table(self.ex_table)
+        alterer = kudu.client.TableAlterer(table)
+        alterer.add_column('added-column', type_='int64', default=0)
+        table = alterer.alter()
+
+        # Confirm column was added
+        expected_repr = 'Column(added-column, parent={0}, type=int64)' \
+            .format(self.ex_table)
+        self.assertEqual(expected_repr, repr(table['added-column']))
+
+        alterer = self.client.new_table_alterer(table)
+        alterer.drop_column('added-column')
+        table = alterer.alter()
+
+        # Confirm column has been dropped.
+        with self.assertRaises(KeyError):
+            col = table['added-column']
+
     def test_alter_table_add_drop_partition(self):
         # Add Range Partition
         table = self.client.table(self.ex_table)


[2/2] kudu git commit: KUDU-1852. KuduTableAlterer should not crash with nullptr arguments

Posted by da...@apache.org.
KUDU-1852. KuduTableAlterer should not crash with nullptr arguments

Change-Id: Ifa7c34b476111ecd33d4fd3ef5cc363a410ad76a
Reviewed-on: http://gerrit.cloudera.org:8080/5797
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins
(cherry picked from commit 8a2ea8bddc88e8de60952b369f112b32710ee19b)
Reviewed-on: http://gerrit.cloudera.org:8080/6204
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1af814b0
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1af814b0
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1af814b0

Branch: refs/heads/branch-1.2.x
Commit: 1af814b0908b191e8c41a027670b99a8c2bc78f3
Parents: 3b05eee
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Jan 25 17:43:25 2017 -0800
Committer: Jean-Daniel Cryans <jd...@apache.org>
Committed: Wed Mar 1 19:10:13 2017 +0000

----------------------------------------------------------------------
 src/kudu/client/client.cc                      | 6 ++++++
 src/kudu/integration-tests/alter_table-test.cc | 7 +++++++
 2 files changed, 13 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/1af814b0/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 3ca0aba..c6ef71f 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -920,14 +920,17 @@ KuduTableAlterer* KuduTableAlterer::AddRangePartition(
 
   if (lower_bound == nullptr || upper_bound == nullptr) {
     data_->status_ = Status::InvalidArgument("range partition bounds may not be null");
+    return this;
   }
   if (!lower_bound->schema()->Equals(*upper_bound->schema())) {
     data_->status_ = Status::InvalidArgument("range partition bounds must have matching schemas");
+    return this;
   }
   if (data_->schema_ == nullptr) {
     data_->schema_ = lower_bound->schema();
   } else if (!lower_bound->schema()->Equals(*data_->schema_)) {
     data_->status_ = Status::InvalidArgument("range partition bounds must have matching schemas");
+    return this;
   }
 
   Data::Step s { AlterTableRequestPB::ADD_RANGE_PARTITION,
@@ -948,14 +951,17 @@ KuduTableAlterer* KuduTableAlterer::DropRangePartition(
     KuduTableCreator::RangePartitionBound upper_bound_type) {
   if (lower_bound == nullptr || upper_bound == nullptr) {
     data_->status_ = Status::InvalidArgument("range partition bounds may not be null");
+    return this;
   }
   if (!lower_bound->schema()->Equals(*upper_bound->schema())) {
     data_->status_ = Status::InvalidArgument("range partition bounds must have matching schemas");
+    return this;
   }
   if (data_->schema_ == nullptr) {
     data_->schema_ = lower_bound->schema();
   } else if (!lower_bound->schema()->Equals(*data_->schema_)) {
     data_->status_ = Status::InvalidArgument("range partition bounds must have matching schemas");
+    return this;
   }
 
   Data::Step s { AlterTableRequestPB::DROP_RANGE_PARTITION,

http://git-wip-us.apache.org/repos/asf/kudu/blob/1af814b0/src/kudu/integration-tests/alter_table-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/alter_table-test.cc b/src/kudu/integration-tests/alter_table-test.cc
index 2d0b8eb..45eba3b 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -1502,6 +1502,13 @@ TEST_F(AlterTableTest, TestAlterRangePartitioningInvalid) {
   ASSERT_FALSE(s.ok());
   ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step");
   ASSERT_EQ(100, CountTableRows(table.get()));
+
+  // Bad arguments (null ranges)
+  table_alterer.reset(client_->NewTableAlterer(table_name));
+  table_alterer->DropRangePartition(nullptr, nullptr);
+  s = table_alterer->Alter();
+  ASSERT_FALSE(s.ok());
+  ASSERT_STR_CONTAINS(s.ToString(), "range partition bounds may not be null");
 }
 
 // Attempts to exhaustively check all cases of single-column range partition