From 8a24f1de9c0489db9f6647c9716d7ea9d7084edb Mon Sep 17 00:00:00 2001
From: Yi Hu <yathu@google.com>
Date: Fri, 15 Sep 2023 16:44:44 -0400
Subject: [PATCH] Add comment

* Mark Python logical types as internal

* Mark possible investigation in Logical type instance creation in Java
---
 .../apache/beam/sdk/schemas/SchemaTranslation.java |  3 +++
 sdks/python/apache_beam/io/jdbc.py                 | 14 ++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/SchemaTranslation.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/SchemaTranslation.java
index fcd186fbdaf4..c0683ef44616 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/SchemaTranslation.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/SchemaTranslation.java
@@ -443,6 +443,7 @@ private static FieldType fieldTypeFromProtoWithoutNullable(SchemaApi.FieldType p
           return FieldType.DECIMAL;
         } else if (urn.startsWith("beam:logical_type:")) {
           if (!logicalType.getPayload().isEmpty()) {
+            // logical type has a payload, try to recover the instance by deserialization
             try {
               return FieldType.logicalType(
                   (LogicalType)
@@ -454,6 +455,8 @@ private static FieldType fieldTypeFromProtoWithoutNullable(SchemaApi.FieldType p
                   urn);
             }
           } else {
+            // logical type does not have a payload. This happens when it is passed xlang.
+            // TODO(yathu) it appears this path is called heavily, consider cache the instance
             LOG.debug("Constructing non-standard logical type {} as UnknownLogicalType", urn);
           }
         }
diff --git a/sdks/python/apache_beam/io/jdbc.py b/sdks/python/apache_beam/io/jdbc.py
index f681319632c8..903b0d1b0fef 100644
--- a/sdks/python/apache_beam/io/jdbc.py
+++ b/sdks/python/apache_beam/io/jdbc.py
@@ -363,7 +363,12 @@ def __init__(
 
 @LogicalType.register_logical_type
 class JdbcDateType(LogicalType[datetime.date, MillisInstant, str]):
-  """Support of Legacy JdbcIO DATE logical type."""
+  """
+  For internal use only; no backwards-compatibility guarantees.
+
+  Support of Legacy JdbcIO DATE logical type. Deemed to change when Java JDBCIO
+  has been migrated to Beam portable logical types.
+  """
   def __init__(self, argument=""):
     pass
 
@@ -405,7 +410,12 @@ def _from_typing(cls, typ):
 
 @LogicalType.register_logical_type
 class JdbcTimeType(LogicalType[datetime.time, MillisInstant, str]):
-  """Support of Legacy JdbcIO TIME logical type."""
+  """
+  For internal use only; no backwards-compatibility guarantees.
+
+  Support of Legacy JdbcIO TIME logical type. . Deemed to change when Java
+  JDBCIO has been migrated to Beam portable logical types.
+  """
   def __init__(self, argument=""):
     pass