This is an automated email from the ASF dual-hosted git repository.
achennaka pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 51f761c37 KUDU-1261 [Java] Fix ColumnSchemaBuilder copy constructor
51f761c37 is described below
commit 51f761c37259d8159142e890d4d8106f68bfa520
Author: Abhishek Chennaka <[email protected]>
AuthorDate: Mon Oct 20 14:00:00 2025 -0700
KUDU-1261 [Java] Fix ColumnSchemaBuilder copy constructor
Fix ColumnSchemaBuilder(ColumnSchema) to retain array type info when
copying NESTED columns. The builder now restores the element type and
marks isArray=true, preventing loss of the nested descriptor on build().
Added ColumnSchemaTest.testCloneArrayColumn() to verify array columns
can be cloned and rebuilt without exceptions or metadata loss.
Replaced Assert.assert* with the static import style assert* for
readability.
Change-Id: I8cdd3ec034c3118b476e7d0da264819b1d4c2cd8
Reviewed-on: http://gerrit.cloudera.org:8080/23564
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Abhishek Chennaka <[email protected]>
---
.../main/java/org/apache/kudu/ColumnSchema.java | 33 +++++-----
.../java/org/apache/kudu/TestColumnSchema.java | 76 ++++++++++++++++------
2 files changed, 73 insertions(+), 36 deletions(-)
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
b/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
index e5706e156..c8e5ca41c 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
@@ -412,7 +412,8 @@ public class ColumnSchema {
Objects.equals(keyUnique, that.keyUnique) &&
Objects.equals(autoIncrementing, that.autoIncrementing) &&
Objects.equals(typeAttributes, that.typeAttributes) &&
- Objects.equals(comment, that.comment);
+ Objects.equals(comment, that.comment) &&
+ Objects.equals(nestedTypeDescriptor, that.nestedTypeDescriptor);
}
@Override
@@ -458,9 +459,6 @@ public class ColumnSchema {
private ColumnTypeAttributes typeAttributes = null;
private Common.DataType wireType = null;
private String comment = "";
- // Nested descriptor captured when array(true) called
- private NestedTypeDescriptor nestedTypeDescriptor = null;
-
private boolean isArray = false;
@@ -490,7 +488,13 @@ public class ColumnSchema {
*/
public ColumnSchemaBuilder(ColumnSchema that) {
this.name = that.name;
- this.type = that.type;
+ if (that.isArray()) {
+ this.type =
that.getNestedTypeDescriptor().getArrayDescriptor().getElemType();
+ this.isArray = true;
+ } else {
+ this.type = that.getType();
+ this.isArray = false;
+ }
this.key = that.key;
this.keyUnique = that.keyUnique;
this.nullable = that.nullable;
@@ -502,7 +506,6 @@ public class ColumnSchema {
this.typeAttributes = that.typeAttributes;
this.wireType = that.wireType;
this.comment = that.comment;
- this.nestedTypeDescriptor = that.nestedTypeDescriptor;
}
/**
@@ -667,18 +670,16 @@ public class ColumnSchema {
*/
public ColumnSchema build() {
final Type finalType;
+ final ColumnSchema.NestedTypeDescriptor nestedDesc;
if (this.isArray) {
- // The builder's `type` currently holds the element (scalar) type.
- if (this.type == Type.NESTED) {
- throw new IllegalArgumentException("Builder.type must be scalar when
creating " +
- "an array column");
- }
- ColumnSchema.ArrayTypeDescriptor arrDesc = new
ColumnSchema.ArrayTypeDescriptor(this.type);
- nestedTypeDescriptor =
ColumnSchema.NestedTypeDescriptor.forArray(arrDesc);
+ // Always promote the scalar element type to a 1D array descriptor.
+ ColumnSchema.ArrayTypeDescriptor arrDesc =
+ new ColumnSchema.ArrayTypeDescriptor(this.type);
+ nestedDesc = ColumnSchema.NestedTypeDescriptor.forArray(arrDesc);
finalType = Type.NESTED;
} else {
- nestedTypeDescriptor = null;
+ nestedDesc = null;
finalType = this.type;
}
@@ -697,7 +698,7 @@ public class ColumnSchema {
}
} else {
// For arrays, element type rules apply.
- Type elemType =
nestedTypeDescriptor.getArrayDescriptor().getElemType();
+ Type elemType = nestedDesc.getArrayDescriptor().getElemType();
if (elemType == Type.VARCHAR) {
if (typeAttributes == null || !typeAttributes.hasLength() ||
typeAttributes.getLength() < CharUtil.MIN_VARCHAR_LENGTH ||
@@ -716,7 +717,7 @@ public class ColumnSchema {
// Finally build immutable ColumnSchema with nested descriptor if any.
return new ColumnSchema(name, finalType, key, keyUnique, nullable,
immutable,
/* autoIncrementing */ false, defaultValue, desiredBlockSize,
encoding,
- compressionAlgorithm, typeAttributes, wireType, comment,
nestedTypeDescriptor);
+ compressionAlgorithm, typeAttributes, wireType, comment,
nestedDesc);
}
}
diff --git
a/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
b/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
index c11300ecf..0f7ea8cd7 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
@@ -18,7 +18,10 @@
package org.apache.kudu;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
import org.junit.Assert;
import org.junit.Rule;
@@ -70,15 +73,15 @@ public class TestColumnSchema {
ColumnSchema isKey = new ColumnSchemaBuilder("col1", Type.STRING)
.key(true)
.build();
- Assert.assertTrue(isKey.isKey());
+ assertTrue(isKey.isKey());
assertNotEquals(stringCol1, isKey);
// Difference between key and nonUniqueKey
ColumnSchema isNonUniqueKey = new ColumnSchemaBuilder("col1", Type.STRING)
.nonUniqueKey(true)
.build();
- Assert.assertTrue(isNonUniqueKey.isKey());
- Assert.assertFalse(isNonUniqueKey.isKeyUnique());
+ assertTrue(isNonUniqueKey.isKey());
+ assertFalse(isNonUniqueKey.isKeyUnique());
assertNotEquals(isKey, isNonUniqueKey);
// Different by type
@@ -113,26 +116,26 @@ public class TestColumnSchema {
@Test
public void testOutOfRangeVarchar() throws Exception {
- Throwable thrown = Assert.assertThrows(IllegalArgumentException.class, new
ThrowingRunnable() {
+ Throwable thrown = assertThrows(IllegalArgumentException.class, new
ThrowingRunnable() {
@Override
public void run() throws Exception {
new ColumnSchemaBuilder("col1", Type.VARCHAR)
.typeAttributes(CharUtil.typeAttributes(70000)).build();
}
});
- Assert.assertTrue(thrown.getMessage()
+ assertTrue(thrown.getMessage()
.contains("VARCHAR's length must be set and between 1 and 65535"));
}
@Test
public void testVarcharWithoutLength() throws Exception {
- Throwable thrown = Assert.assertThrows(IllegalArgumentException.class, new
ThrowingRunnable() {
+ Throwable thrown = assertThrows(IllegalArgumentException.class, new
ThrowingRunnable() {
@Override
public void run() throws Exception {
new ColumnSchemaBuilder("col1", Type.VARCHAR).build();
}
});
- Assert.assertTrue(thrown.getMessage()
+ assertTrue(thrown.getMessage()
.contains("VARCHAR's length must be set and between 1 and 65535"));
}
@@ -140,23 +143,23 @@ public class TestColumnSchema {
public void testAutoIncrementing() throws Exception {
// Create auto-incrementing column with AutoIncrementingColumnSchemaBuilder
ColumnSchema autoIncrementing = new
AutoIncrementingColumnSchemaBuilder().build();
- Assert.assertTrue(autoIncrementing.isAutoIncrementing());
+ assertTrue(autoIncrementing.isAutoIncrementing());
assertEquals(Schema.getAutoIncrementingColumnType(),
autoIncrementing.getType());
- Assert.assertTrue(autoIncrementing.isKey());
- Assert.assertFalse(autoIncrementing.isKeyUnique());
- Assert.assertFalse(autoIncrementing.isNullable());
- Assert.assertFalse(autoIncrementing.isImmutable());
+ assertTrue(autoIncrementing.isKey());
+ assertFalse(autoIncrementing.isKeyUnique());
+ assertFalse(autoIncrementing.isNullable());
+ assertFalse(autoIncrementing.isImmutable());
assertEquals(null, autoIncrementing.getDefaultValue());
// Create column with auto-incrementing column name with
ColumnSchemaBuilder
- Throwable thrown = Assert.assertThrows(IllegalArgumentException.class, new
ThrowingRunnable() {
+ Throwable thrown = assertThrows(IllegalArgumentException.class, new
ThrowingRunnable() {
@Override
public void run() throws Exception {
new ColumnSchemaBuilder(Schema.getAutoIncrementingColumnName(),
Schema.getAutoIncrementingColumnType()).build();
}
});
- Assert.assertTrue(thrown.getMessage().contains("Column name " +
+ assertTrue(thrown.getMessage().contains("Column name " +
Schema.getAutoIncrementingColumnName() + " is reserved by Kudu
engine"));
}
@@ -170,7 +173,7 @@ public class TestColumnSchema {
assertEquals("str_arr", strArrayCol.getName());
assertEquals(Type.NESTED, strArrayCol.getType());
- Assert.assertTrue(strArrayCol.isNullable());
+ assertTrue(strArrayCol.isNullable());
assertEquals(Type.STRING,
strArrayCol.getNestedTypeDescriptor().getArrayDescriptor().getElemType());
@@ -188,7 +191,7 @@ public class TestColumnSchema {
assertEquals("dec_arr", decimalArrayCol.getName());
assertEquals(Type.NESTED, decimalArrayCol.getType());
- Assert.assertTrue(decimalArrayCol.isNullable());
+ assertTrue(decimalArrayCol.isNullable());
assertEquals(Type.DECIMAL,
decimalArrayCol.getNestedTypeDescriptor().getArrayDescriptor().getElemType());
@@ -200,7 +203,7 @@ public class TestColumnSchema {
assertEquals("int_arr", intArrayCol.getName());
assertEquals(Type.NESTED, intArrayCol.getType());
- Assert.assertFalse(intArrayCol.isNullable());
+ assertFalse(intArrayCol.isNullable());
assertEquals(Type.INT32,
intArrayCol.getNestedTypeDescriptor().getArrayDescriptor().getElemType());
@@ -217,17 +220,50 @@ public class TestColumnSchema {
assertEquals("varchar_arr", varcharArrayCol.getName());
assertEquals(Type.NESTED, varcharArrayCol.getType());
- Assert.assertTrue(varcharArrayCol.isNullable());
+ assertTrue(varcharArrayCol.isNullable());
assertEquals(Type.VARCHAR,
varcharArrayCol.getNestedTypeDescriptor().getArrayDescriptor().getElemType());
// Test constructor restriction: cannot pass NESTED directly
- IllegalArgumentException thrown =
Assert.assertThrows(IllegalArgumentException.class, () ->
+ IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, () ->
new ColumnSchema.ColumnSchemaBuilder("nested", Type.NESTED)
);
- Assert.assertTrue(thrown.getMessage().contains(
+ assertTrue(thrown.getMessage().contains(
"Column nested cannot be set to NESTED type. Use
ColumnSchemaBuilder.array(true) instead"
));
}
+
+ @Test
+ public void testColumnSchemaBuilderCopyConstructor() {
+ // --- Scalar column setup ---
+ ColumnSchema scalarCol =
+ new ColumnSchema.ColumnSchemaBuilder("age", Type.INT32)
+ .nullable(false)
+ .key(true)
+ .comment("scalar column")
+ .build();
+
+ ColumnSchemaBuilder scalarBuilderCopy = new ColumnSchemaBuilder(scalarCol);
+ ColumnSchema scalarCopy = scalarBuilderCopy.build();
+ // Verify the ColumnSchema objects are identical
+ assertEquals(scalarCopy, scalarCol);
+
+
+ // --- Array column setup ---
+ ColumnSchema arrayCol =
+ new ColumnSchema.ColumnSchemaBuilder("scores", Type.INT16)
+ .array(true)
+ .nullable(true)
+ .comment("array column")
+ .build();
+
+ ColumnSchemaBuilder arrayBuilderCopy = new ColumnSchemaBuilder(arrayCol);
+ ColumnSchema arrayCopy = arrayBuilderCopy.build();
+ assertEquals(arrayCopy, arrayCol);
+ // Ensure nestedTypeDescriptor and attributes are copied
+ assertEquals(arrayCol.getNestedTypeDescriptor(),
arrayCopy.getNestedTypeDescriptor());
+ assertEquals(arrayCol.getTypeAttributes(), arrayCopy.getTypeAttributes());
+ }
+
}
