-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[Fix][Connector-V2] Field information lost during Paimon DataType and SeaTunnel Column conversion #6767
[Fix][Connector-V2] Field information lost during Paimon DataType and SeaTunnel Column conversion #6767
Conversation
# Conflicts: # docs/en/connector-v2/sink/Doris.md
Any test case? |
OK, thanks for your review. I'm going to add some test cases |
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.
LGTM
@@ -320,7 +321,7 @@ public static SeaTunnelRow convert(InternalRow rowData, SeaTunnelRowType seaTunn | |||
break; | |||
default: | |||
throw new PaimonConnectorException( | |||
CommonErrorCodeDeprecated.UNSUPPORTED_DATA_TYPE, | |||
CommonErrorCode.UNSUPPORTED_DATA_TYPE, |
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.
Please throw common error by use this method.
seatunnel/seatunnel-common/src/main/java/org/apache/seatunnel/common/exception/CommonError.java
Line 90 in a5609d6
public static SeaTunnelRuntimeException unsupportedDataType( |
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.
Please throw common error by use this method.
seatunnel/seatunnel-common/src/main/java/org/apache/seatunnel/common/exception/CommonError.java
Line 90 in a5609d6
public static SeaTunnelRuntimeException unsupportedDataType(
done.
public static SeaTunnelRuntimeException unsupportedDataType( | ||
String identifier, String dataType) { | ||
Map<String, String> params = new HashMap<>(); | ||
params.put("identifier", identifier); | ||
params.put("dataType", dataType); | ||
return new SeaTunnelRuntimeException(UNSUPPORTED_DATA_TYPE_SIMPLE, params); | ||
} |
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.
Please reuse public static SeaTunnelRuntimeException unsupportedDataType( String identifier, String dataType, String field)
. Because we should make sure user know which field can not supported.
return new SeaTunnelRuntimeException(UNSUPPORTED_ARRAY_GENERIC_TYPE, params); | ||
} | ||
|
||
public static SeaTunnelRuntimeException unsupportedRowKind(String identifier, String rowKind) { |
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.
we need tableId when invoke unsupportedRowKind
method.
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.
we need tableId when invoke
unsupportedRowKind
method.
OK, I will optimize the code based on your suggestions
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.
we need tableId when invoke
unsupportedRowKind
method.OK, I will optimize the code based on your suggestions
done.
public static SeaTunnelRuntimeException unsupportedArrayGenericType( | ||
String identifier, String dataType) { | ||
Map<String, String> params = new HashMap<>(); | ||
params.put("identifier", identifier); | ||
params.put("dataType", dataType); | ||
return new SeaTunnelRuntimeException(UNSUPPORTED_ARRAY_GENERIC_TYPE, params); | ||
} |
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.
ditto
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.
ditto
done.
# Conflicts: # seatunnel-common/src/main/java/org/apache/seatunnel/common/exception/CommonError.java # seatunnel-common/src/main/java/org/apache/seatunnel/common/exception/CommonErrorCode.java
2c327bf
to
3915e36
Compare
@@ -445,9 +496,8 @@ public SeaTunnelDataType<?> visit(RowType rowType) { | |||
|
|||
@Override | |||
protected SeaTunnelDataType defaultMethod(DataType dataType) { |
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.
@Hisoka-X This defaultMethod
method inherited from paimon seems to be difficult to obtain the fieldname.
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.
Please put fieldname as UNKNOWN
.
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.
Please put fieldname as
UNKNOWN
.
done.
builder.length(column.getColumnLength()); | ||
return builder.build(); | ||
case DECIMAL: | ||
int precision = |
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.
In some cases, the precision
and scale
obtained here may be 0, and you must handle this situation to achieve better compatibility.
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.
In some cases, the
precision
andscale
obtained here may be 0, and you must handle this situation to achieve better compatibility.
done.
|
||
OPERATION_NOT_SUPPORTED("COMMON-26", "<identifier> <operation> is unsupported."); | ||
OPERATION_NOT_SUPPORTED("COMMON-26", "<identifier> <operation> is unsupported."), | ||
UNSUPPORTED_DATA_TYPE_SIMPLE("COMMON-27", "'<identifier>' unsupported data type '<dataType>'"), |
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.
UNSUPPORTED_DATA_TYPE_SIMPLE("COMMON-27", "'<identifier>' unsupported data type '<dataType>'"), |
@xiaochen-zhou Please resolve the conflicts. |
|
# Conflicts: # seatunnel-common/src/main/java/org/apache/seatunnel/common/exception/CommonErrorCode.java # seatunnel-connectors-v2/connector-paimon/src/main/java/org/apache/seatunnel/connectors/seatunnel/paimon/catalog/PaimonCatalog.java
done. |
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.
LGTM if ci passed.
done. |
Purpose of this pull request
Field information lost during Paimon DataType and SeaTunnel Column conversion, such as: field name
Does this PR introduce any user-facing change?
no
How was this patch tested?
exists test
Check list
New License Guide
release-note
.