kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jdcry...@apache.org
Subject [1/2] kudu git commit: [java] KUDU-1746/KUDU-1747 Improve addColumn API
Date Wed, 07 Dec 2016 16:43:41 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 7bd9bac67 -> 93649c469


[java] KUDU-1746/KUDU-1747 Improve addColumn API

Previously, the java client permitted adding

- a non-null column with a default
- a nullable column with no default

but did not allow adding

- a nullable column with a default

even though it should be allowed. This patch adds that capability
by adding a 'defaultVal' argument to an overloaded
AlterTableOptions.addNullableColumn.

It also adds a more general method to add a column based on a
ColumnSchema. This more advanced API should be easier to maintain
as more column options are added to Kudu. The other addColumn
variants are reimplemented in terms of this one.

Previous methods remain for backward-compatibility.

Additionally, a test was added to check behavior of the three
addColumn possibilities (which tests the new addColumn(ColumnSchema)
since they are implemented using it).

Finally, I fixed a checkstyle warning in AsyncKuduScanner.

Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Reviewed-on: http://gerrit.cloudera.org:8080/5132
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jdcryans@apache.org>


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

Branch: refs/heads/master
Commit: 75b1c1578ec6503e02b934a9058f08a1f63e3b4f
Parents: 7bd9bac
Author: Will Berkeley <wdberkeley@gmail.com>
Authored: Thu Nov 17 15:35:05 2016 -0500
Committer: Jean-Daniel Cryans <jdcryans@apache.org>
Committed: Wed Dec 7 16:40:52 2016 +0000

----------------------------------------------------------------------
 .../apache/kudu/client/AlterTableOptions.java   | 57 +++++++++++++-------
 .../apache/kudu/client/AsyncKuduScanner.java    |  4 +-
 .../org/apache/kudu/client/TestAlterTable.java  | 46 ++++++++++++++++
 3 files changed, 86 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/75b1c157/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
b/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
index 1eab671..72c70e7 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
@@ -46,6 +46,25 @@ public class AlterTableOptions {
   }
 
   /**
+   * Add a new column.
+   * @param colSchema the schema of the new column
+   * @return this instance
+   */
+  public AlterTableOptions addColumn(ColumnSchema colSchema) {
+    if (!colSchema.isNullable() && colSchema.getDefaultValue() == null) {
+      throw new IllegalArgumentException("A new non-null column must have a default value");
+    }
+    if (colSchema.isKey()) {
+      throw new IllegalArgumentException("Key columns cannot be added");
+    }
+    AlterTableRequestPB.Step.Builder step = pb.addAlterSchemaStepsBuilder();
+    step.setType(AlterTableRequestPB.StepType.ADD_COLUMN);
+    step.setAddColumn(AlterTableRequestPB.AddColumn.newBuilder()
+        .setSchema(ProtobufHelper.columnToPb(colSchema)));
+    return this;
+  }
+
+  /**
    * Add a new column that's not nullable.
    * @param name name of the new column
    * @param type type of the new column
@@ -53,33 +72,33 @@ public class AlterTableOptions {
    * @return this instance
    */
   public AlterTableOptions addColumn(String name, Type type, Object defaultVal) {
-    if (defaultVal == null) {
-      throw new IllegalArgumentException("A new column must have a default value, " +
-          "use addNullableColumn() to add a NULLABLE column");
-    }
-    AlterTableRequestPB.Step.Builder step = pb.addAlterSchemaStepsBuilder();
-    step.setType(AlterTableRequestPB.StepType.ADD_COLUMN);
-    step.setAddColumn(AlterTableRequestPB.AddColumn.newBuilder().setSchema(ProtobufHelper
-        .columnToPb(new ColumnSchema.ColumnSchemaBuilder(name, type)
-            .defaultValue(defaultVal)
-            .build())));
-    return this;
+    return addColumn(new ColumnSchema.ColumnSchemaBuilder(name, type)
+        .defaultValue(defaultVal)
+        .build());
   }
 
   /**
-   * Add a new column that's nullable, thus has no default value.
+   * Add a new column that's nullable and has no default value.
    * @param name name of the new column
    * @param type type of the new column
    * @return this instance
    */
   public AlterTableOptions addNullableColumn(String name, Type type) {
-    AlterTableRequestPB.Step.Builder step = pb.addAlterSchemaStepsBuilder();
-    step.setType(AlterTableRequestPB.StepType.ADD_COLUMN);
-    step.setAddColumn(AlterTableRequestPB.AddColumn.newBuilder().setSchema(ProtobufHelper
-        .columnToPb(new ColumnSchema.ColumnSchemaBuilder(name, type)
-            .nullable(true)
-            .build())));
-    return this;
+    return addNullableColumn(name, type, null);
+  }
+
+  /**
+   * Add a new column that's nullable.
+   * @param name name of the new column
+   * @param type type of the new column
+   * @param defaultVal the default value of the new column
+   * @return this instance
+   */
+  public AlterTableOptions addNullableColumn(String name, Type type, Object defaultVal) {
+    return addColumn(new ColumnSchema.ColumnSchemaBuilder(name, type)
+        .nullable(true)
+        .defaultValue(defaultVal)
+        .build());
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/kudu/blob/75b1c157/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
index 40906ae..2de2b7b 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
@@ -680,8 +680,8 @@ public final class AsyncKuduScanner {
     }
 
     public String toString() {
-      String ret = "AsyncKuduScanner$Response(scannerId=" + Bytes.pretty(scannerId) + ",
data=" + data +
-          ", more=" + more;
+      String ret = "AsyncKuduScanner$Response(scannerId=" + Bytes.pretty(scannerId) +
+          ", data=" + data + ", more=" + more;
       if (scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
         ret += ", responseScanTimestamp =" + scanTimestamp;
       }

http://git-wip-us.apache.org/repos/asf/kudu/blob/75b1c157/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
index ebb8bb8..3b8b5a2 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
@@ -16,12 +16,14 @@
 // under the License.
 package org.apache.kudu.client;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import com.google.common.collect.ImmutableList;
@@ -102,6 +104,50 @@ public class TestAlterTable extends BaseKuduTest {
   }
 
   @Test
+  public void testAlterAddColumns() throws Exception {
+    KuduTable table = createTable(ImmutableList.<Pair<Integer,Integer>>of());
+    insertRows(table, 0, 100);
+    assertEquals(100, countRowsInTable(table));
+
+    syncClient.alterTable(tableName, new AlterTableOptions()
+        .addColumn("addNonNull", Type.INT32, 100)
+        .addNullableColumn("addNullable", Type.INT32)
+        .addNullableColumn("addNullableDef", Type.INT32, 200));
+    boolean done = syncClient.isAlterTableDone(tableName);
+    assertTrue(done);
+
+    // Reopen table for the new schema.
+    table = syncClient.openTable(tableName);
+    assertEquals(5, table.getSchema().getColumnCount());
+
+    // Add a row with addNullableDef=null
+    KuduSession session = syncClient.newSession();
+    Insert insert = table.newInsert();
+    PartialRow row = insert.getRow();
+    row.addInt("c0", 101);
+    row.addInt("c1", 101);
+    row.addInt("addNonNull", 101);
+    row.addInt("addNullable", 101);
+    row.setNull("addNullableDef");
+    session.apply(insert);
+    session.flush();
+    RowError[] rowErrors = session.getPendingErrors().getRowErrors();
+    assertEquals(String.format("row errors: %s", Arrays.toString(rowErrors)), 0, rowErrors.length);
+
+    // Check defaults applied, and that row key=101
+    List<String> actual = scanTableToStrings(table);
+    List<String> expected = new ArrayList<>(101);
+    for (int i = 0; i < 100; i++) {
+      expected.add(i, String.format("INT32 c0=%d, INT32 c1=%d, INT32 addNonNull=100" +
+          ", INT32 addNullable=NULL, INT32 addNullableDef=200", i, i));
+    }
+    expected.add("INT32 c0=101, INT32 c1=101, INT32 addNonNull=101" +
+        ", INT32 addNullable=101, INT32 addNullableDef=NULL");
+    Collections.sort(expected);
+    assertArrayEquals(expected.toArray(new String[0]), actual.toArray(new String[0]));
+  }
+
+  @Test
   public void testAlterRangePartitioning() throws Exception {
     KuduTable table = createTable(ImmutableList.<Pair<Integer,Integer>>of());
     Schema schema = table.getSchema();


Mime
View raw message