You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/11/14 01:20:07 UTC

[GitHub] [spark] zhengruifeng commented on a diff in pull request #38634: [WIP] [CONNECT] Support Cast and DataTypes as Literals

zhengruifeng commented on code in PR #38634:
URL: https://github.com/apache/spark/pull/38634#discussion_r1021005394


##########
python/pyspark/sql/connect/column.py:
##########
@@ -14,20 +14,75 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+import types
 import uuid
-from typing import cast, get_args, TYPE_CHECKING, Callable, Any
+from typing import cast, get_args, TYPE_CHECKING, Callable, Any, Union
 
 import decimal
 import datetime
 
 import pyspark.sql.connect.proto as proto
 from pyspark.sql.connect._typing import PrimitiveType
+from pyspark.sql.types import DataType
+import pyspark.sql.types as sqlTypes
+import pyspark.sql.types as sql_types
 
 if TYPE_CHECKING:
     from pyspark.sql.connect.client import RemoteSparkSession
     import pyspark.sql.connect.proto as proto
 
 
+def _string_to_sql_type(dt: str) -> DataType:

Review Comment:
   it can not support types like ArrayType, MapType, etc here
   
   if we can continue depending on `pyspark.sql`, what about reusing `_parse_datatype_json_value` or `fromJson` in python/pyspark/sql/types.py?
   
   



##########
python/pyspark/sql/connect/column.py:
##########
@@ -14,20 +14,75 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+import types
 import uuid
-from typing import cast, get_args, TYPE_CHECKING, Callable, Any
+from typing import cast, get_args, TYPE_CHECKING, Callable, Any, Union
 
 import decimal
 import datetime
 
 import pyspark.sql.connect.proto as proto
 from pyspark.sql.connect._typing import PrimitiveType
+from pyspark.sql.types import DataType
+import pyspark.sql.types as sqlTypes
+import pyspark.sql.types as sql_types

Review Comment:
   why use both `sqlTypes` and `sql_types`?



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org