PDGGK commented on PR #39804: URL: https://github.com/apache/beam/pull/39804#issuecomment-5329868672
That is a fair bar, and I would rather the model stay small too. Let me put the specific numbers here, because I think this case is narrower than adding a new abstract type. `beam:logical_type:time:v1` is **already on the wire from the Java SDK today**, with no entry in the model. `Time.IDENTIFIER` is a hardcoded string starting with `beam:logical_type:`, and `getLogicalTypeUrn` passes such identifiers through unchanged, so it never gets the `javasdk_` treatment that other unregistered Java types get. Measured on `master` at `ca6065508a3`: ``` urn on the wire = beam:logical_type:time:v1 payload bytes = 0 read back as = UnknownLogicalType read back urn = beam:logical_type:time:v1 ``` So today Java occupies the portable namespace for this URN, emits it with an empty payload — and cannot read its own output back. Any SDK receiving it has a URN that looks standard, is documented nowhere, and carries nothing to reconstruct from. That is what makes me read this as documenting an obligation the Java SDK already imposes, rather than creating one. The enum entry costs other SDKs nothing they are not already paying: an SDK that does not implement `TIME` behaves exactly as it does now. If the preference is the other direction, the consistent alternative is to stop squatting on the portable namespace — change `Time.IDENTIFIER` to a non-portable identifier so it serialises as `beam:logical_type:javasdk_time:v1` like other Java-only types. I would rather not propose that as the default, since it changes the URN currently on the wire and so is the more disruptive of the two, but it is a coherent resolution and I am happy to write it instead. Either way the present state seems worth not leaving alone. Whichever you prefer, I will follow. -- 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]
