-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for tuple ClickHouse #29715
Changes from all commits
3bbbe2f
a3a1850
53e4e81
a1d5182
a3cafa6
120bb2b
256187f
c1b3f98
a477cbc
ccfdc37
67434ef
f27b943
b4ce111
748dca6
57cb6a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.stream.Collectors; | ||
import org.apache.beam.sdk.schemas.Schema; | ||
import org.apache.beam.sdk.schemas.logicaltypes.FixedBytes; | ||
import org.checkerframework.checker.nullness.qual.Nullable; | ||
|
@@ -111,6 +112,14 @@ public static Schema.FieldType getEquivalentFieldType(ColumnType columnType) { | |
return Schema.FieldType.STRING; | ||
case BOOL: | ||
return Schema.FieldType.BOOLEAN; | ||
case TUPLE: | ||
List<Schema.Field> fields = | ||
columnType.tupleTypes().entrySet().stream() | ||
.map(x -> Schema.Field.of(x.getKey(), Schema.FieldType.DATETIME)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @mzitnik, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, This quick adaptation seems to work:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, Also, it seems that This seems to fix the issue : There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
.collect(Collectors.toList()); | ||
Schema.Field[] array = fields.toArray(new Schema.Field[fields.size()]); | ||
Schema schema = Schema.of(array); | ||
return Schema.FieldType.row(schema); | ||
} | ||
|
||
// not possible, errorprone checks for exhaustive switch | ||
|
@@ -168,7 +177,9 @@ public enum TypeName { | |
// Composite type | ||
ARRAY, | ||
// Primitive type | ||
BOOL | ||
BOOL, | ||
// Composite type | ||
TUPLE | ||
} | ||
|
||
/** | ||
|
@@ -208,6 +219,7 @@ public abstract static class ColumnType implements Serializable { | |
public static final ColumnType UINT32 = ColumnType.of(TypeName.UINT32); | ||
public static final ColumnType UINT64 = ColumnType.of(TypeName.UINT64); | ||
public static final ColumnType BOOL = ColumnType.of(TypeName.BOOL); | ||
public static final ColumnType TUPLE = ColumnType.of(TypeName.TUPLE); | ||
|
||
// ClickHouse doesn't allow nested nullables, so boolean flag is enough | ||
public abstract boolean nullable(); | ||
|
@@ -220,6 +232,8 @@ public abstract static class ColumnType implements Serializable { | |
|
||
public abstract @Nullable ColumnType arrayElementType(); | ||
|
||
public abstract @Nullable Map<String, ColumnType> tupleTypes(); | ||
|
||
public ColumnType withNullable(boolean nullable) { | ||
return toBuilder().nullable(nullable).build(); | ||
} | ||
|
@@ -265,6 +279,14 @@ public static ColumnType array(ColumnType arrayElementType) { | |
.build(); | ||
} | ||
|
||
public static ColumnType tuple(Map<String, ColumnType> elements) { | ||
return ColumnType.builder() | ||
.typeName(TypeName.TUPLE) | ||
.nullable(false) | ||
.tupleTypes(elements) | ||
.build(); | ||
} | ||
|
||
/** | ||
* Parse string with ClickHouse type to {@link ColumnType}. | ||
* | ||
|
@@ -339,6 +361,8 @@ abstract static class Builder { | |
|
||
public abstract Builder fixedStringSize(Integer size); | ||
|
||
public abstract Builder tupleTypes(Map<String, ColumnType> tupleElements); | ||
|
||
public abstract ColumnType build(); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caller has a condition
if (type.toLowerCase().trim().startsWith("tuple(")) {
below, so herepayload
always (case insensitive) starts with "tuple(", so here the replace of "Tuple\(" won't workThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition is on with lower case, but the actual parsing is on the original string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean below when " if (type.toLowerCase().trim().startsWith("tuple(")) {" the this preprocessing will be executed.
So Tuple( (with backslash) is considered here but not the condition in the caller there. Coild this cause issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. Would you mind adding some comments about the proprocessing rule and example, for example, sth like
// Tuple(a String, b Integer) -> ...