chamikaramj commented on code in PR #17608:
URL: https://github.com/apache/beam/pull/17608#discussion_r871848110
##########
model/pipeline/src/main/proto/org/apache/beam/model/pipeline/v1/schema.proto:
##########
@@ -31,6 +31,8 @@ option go_package =
"github.com/apache/beam/sdks/v2/go/pkg/beam/model/pipeline_v
option java_package = "org.apache.beam.model.pipeline.v1";
option java_outer_classname = "SchemaApi";
+import "org/apache/beam/model/pipeline/v1/beam_runner_api.proto";
Review Comment:
Do we need this dependency ?
##########
model/pipeline/src/main/proto/org/apache/beam/model/pipeline/v1/beam_runner_api.proto:
##########
@@ -1094,6 +1094,13 @@ message StandardCoders {
}
}
+message LogicalTypes {
Review Comment:
Move other types as well ?
##########
sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/PythonExternalTransform.java:
##########
@@ -162,6 +167,26 @@ public PythonExternalTransform<InputT, OutputT>
withKwargs(Row kwargs) {
return this;
}
+ /**
+ * Specifies the field type of arguments.
+ *
+ * <p>Type hints are especially useful for logical types since type
inference does not work well
+ * for logical types.
+ *
+ * @param argType A class object for the argument type.
+ * @param fieldType A schema field type for the argument.
+ * @return updated wrapper for the cross-language transform.
+ */
+ public PythonExternalTransform<InputT, OutputT> withTypeHint(
Review Comment:
I don't. Just wasn't sure. We can keep this per type if Robert and Brian are
OK.
##########
sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/PythonExternalTransform.java:
##########
@@ -179,16 +204,20 @@ Row buildOrGetKwargsRow() {
// Types that are not one of following are considered custom types.
// * Java primitives
// * Type String
+ // * Type PythonCallableSource
+ // * Any Type explicitly annotated by withTypeHint()
// * Type Row
- private static boolean isCustomType(java.lang.Class<?> type) {
+ private boolean isCustomType(java.lang.Class<?> type) {
boolean val =
!(ClassUtils.isPrimitiveOrWrapper(type)
|| type == String.class
+ || type == PythonCallableSource.class
Review Comment:
I would at least move this to a "isLogicalSchemaType()" method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
