dianfu commented on code in PR #28924: URL: https://github.com/apache/flink/pull/28924#discussion_r3764341846
########## flink-python/pyflink/table/literal.py: ########## @@ -0,0 +1,219 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################ + +import calendar +import datetime +import time +from array import array + +from pyflink.common import Row +from pyflink.java_gateway import get_gateway +from pyflink.table.types import ( + _array_type_mappings, + _to_java_data_type, + ArrayType, + DataType, + DateType, + DayTimeIntervalType, + LocalZonedTimestampType, + MapType, + MultisetType, + RowType, + TimeType, + TimestampType, + ZonedTimestampType, +) +from pyflink.util.api_stability_decorators import Internal + + +@Internal() +def _to_java_literal_value(value, data_type: DataType = None): + """Converts Python-only literal values into objects accepted by Py4J.""" + if data_type is None: + return _to_java_inferred_literal_value(value) + return _to_java_typed_literal_value(value, data_type) + + +def _to_java_inferred_literal_value(value): + if value is None: + return value + + gateway = get_gateway() + jvm = gateway.jvm + if isinstance(value, datetime.datetime): + return _to_java_typed_literal_value(value, TimestampType()) + elif isinstance(value, datetime.date): + return _to_java_typed_literal_value(value, DateType()) + elif isinstance(value, datetime.time): + return _to_java_typed_literal_value(value, TimeType()) + elif isinstance(value, datetime.timedelta): + return _to_java_typed_literal_value( + value, + DayTimeIntervalType(DayTimeIntervalType.DayTimeResolution.DAY_TO_SECOND), + ) + elif isinstance(value, array): + if value.typecode not in _array_type_mappings: + raise TypeError(f"not supported type: array({value.typecode})") + element_data_type = _to_java_data_type(_array_type_mappings[value.typecode]) + j_array = jvm.java.lang.reflect.Array.newInstance( + element_data_type.getConversionClass(), len(value) + ) + for pos, element in enumerate(value): + j_array[pos] = element + return j_array + elif isinstance(value, (list, tuple)): + j_values = jvm.java.util.ArrayList() + for element in value: + j_values.add(_to_java_inferred_literal_value(element)) + return j_values + elif isinstance(value, Row): + return _to_java_row(value) + return value + + +def _to_java_typed_literal_value(value, data_type: DataType): + if value is None or data_type._conversion_cls: + return value + + jvm = get_gateway().jvm + if isinstance(data_type, DateType) and isinstance(value, datetime.datetime): + value = value.date() + if isinstance(data_type, DateType) and isinstance(value, datetime.date): + return jvm.java.time.LocalDate.of(value.year, value.month, value.day) + elif isinstance(data_type, TimeType) and isinstance(value, datetime.time): + return jvm.java.time.LocalTime.of( + value.hour, value.minute, value.second, value.microsecond * 1000 + ) + elif isinstance(data_type, TimestampType) and isinstance(value, datetime.datetime): + return jvm.java.time.LocalDateTime.of( + value.year, + value.month, + value.day, + value.hour, + value.minute, + value.second, + value.microsecond * 1000, + ) + elif isinstance(data_type, LocalZonedTimestampType) and isinstance( + value, datetime.datetime + ): + seconds = ( Review Comment: This combines UTC-normalized whole seconds from utctimetuple() with the original local microsecond value. For valid time-zone offsets containing fractional seconds, this creates the wrong instant. For example, 1970-01-01 00:00:00.100000+00:00:00.500000 should represent -0.4s, but the current code produces -0.9s. Please normalize the complete datetime to UTC before extracting both seconds and nanoseconds. This would also allow awareness to be checked via utcoffset() and avoid losing fold information through timetuple()/mktime(). ########## flink-python/pyflink/table/literal.py: ########## @@ -0,0 +1,219 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################ + +import calendar +import datetime +import time +from array import array + +from pyflink.common import Row +from pyflink.java_gateway import get_gateway +from pyflink.table.types import ( + _array_type_mappings, + _to_java_data_type, + ArrayType, + DataType, + DateType, + DayTimeIntervalType, + LocalZonedTimestampType, + MapType, + MultisetType, + RowType, + TimeType, + TimestampType, + ZonedTimestampType, +) +from pyflink.util.api_stability_decorators import Internal + + +@Internal() +def _to_java_literal_value(value, data_type: DataType = None): + """Converts Python-only literal values into objects accepted by Py4J.""" + if data_type is None: + return _to_java_inferred_literal_value(value) + return _to_java_typed_literal_value(value, data_type) + + +def _to_java_inferred_literal_value(value): + if value is None: + return value + + gateway = get_gateway() + jvm = gateway.jvm + if isinstance(value, datetime.datetime): + return _to_java_typed_literal_value(value, TimestampType()) Review Comment: Could we preserve the offset/instant when inferring a literal from a timezone-aware datetime? This branch currently converts every datetime to LocalDateTime, so 2026-08-03 12:00+08:00 and 2026-08-03 04:00+00:00, which represent the same instant, become two different TIMESTAMP literals. ########## flink-python/pyflink/table/expressions.py: ########## @@ -115,10 +116,14 @@ def lit(v, data_type: DataType = None) -> Expression: >>> tab.select(col("key"), lit("abc")) """ - if data_type is None: - return _unary_op("lit", v) - else: - return _binary_op("lit", v, _to_java_data_type(data_type)) + _j_literal_value = _to_java_literal_value(v, data_type) Review Comment: Could we validate or convert data_type before passing it to _to_java_literal_value()? With this current implementation, lit(1, "INT") now raises AttributeError: 'str' object has no attribute '_conversion_cls', whereas the previous implementation reported an appropriate TypeError from _to_java_data_type(). An explicit isinstance(data_type, DataType) check, or calling _to_java_data_type() first, would preserve the expected error behavior. -- 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]
