JingsongLi commented on code in PR #10675: URL: https://github.com/apache/paimon/pull/10675#discussion_r4345546897
########## paimon-python/pypaimon/schema/default_value.py: ########## @@ -0,0 +1,263 @@ +# 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. + +"""Validate a scalar column default before it is stored. + +The stored value is still the original string. Scalar checks follow the Java +string casts used by ``DefaultValueUtils`` for booleans, numbers, dates, times +and timestamps. ARRAY, MAP and ROW defaults are accepted by Java's +``StringToArrayCastRule``, ``StringToMapCastRule`` and ``StringToRowCastRule``; +Python does not parse those forms yet and rejects them explicitly. +""" + +import re +from calendar import monthrange +from datetime import datetime +from decimal import Decimal +from typing import Optional + +from pypaimon.schema.data_types import AtomicType + +_TRUE = {"t", "true", "y", "yes", "1"} +_FALSE = {"f", "false", "n", "no", "0"} +_INT_RANGES = { + "TINYINT": (-128, 127), + "SMALLINT": (-32768, 32767), + "INT": (-2147483648, 2147483647), + "INTEGER": (-2147483648, 2147483647), + "BIGINT": (-9223372036854775808, 9223372036854775807), +} +_INTEGRAL = set(_INT_RANGES) +_APPROXIMATE = {"FLOAT", "DOUBLE"} +_CHARACTER = {"STRING", "CHAR", "VARCHAR"} +_BINARY = {"BINARY", "VARBINARY", "BYTES"} +_UNSIGNED_INTEGER_RE = re.compile(r"[0-9]+") +# StringUtils.isNumeric guards the epoch fast path of DATE, TIME and TIMESTAMP. +# It allows a leading '-' but not '+', and only ASCII digits after it. +_JAVA_NUMERIC_RE = re.compile(r"-?[0-9]+") +_INTEGRAL_RE = re.compile(r"[+-]?(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]*)") +_DECIMAL_RE = re.compile(r"[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?") +# Float.parseFloat / Double.parseDouble accept hexadecimal literals and type +# suffixes, but not Python's underscores or case-insensitive special values. +_FLOAT_RE = re.compile( + r"[+-]?(?:NaN|Infinity|" + r"(?:(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)(?:[eE][+-]?[0-9]+)?|" + r"0[xX](?:[0-9a-fA-F]+(?:\.[0-9a-fA-F]*)?|\.[0-9a-fA-F]+)" + r"[pP][+-]?[0-9]+)[fFdD]?)") +_JAVA_TRIM_CHARS = "".join(chr(i) for i in range(33)) +# DateTimeUtils.DEFAULT_TIMESTAMP_FORMATTER: the time is optional, a space +# requires hour:minute:second, and 'T' requires hour:minute with optional +# seconds. appendFraction has a minimum width of 0, so a bare '.' is allowed. +# Anchor with \Z, not $, so a trailing newline does not slip through. +_TIMESTAMP_SPACE_RE = re.compile( + r"^(\d{4})-(\d{1,2})-(\d{1,2})" Review Comment: [P2] Use the JVM timestamp formatter digit grammar Python `\d` includes Unicode digits, so a TIMESTAMP(3) default such as `２０２４-０１-０１ ００:００:００` is accepted and persisted. The Java formatter rejects it at index 0 with `DateTimeParseException`; the writer then fails during default initialization. This is reproducible from the exact Python-written schema and also affects TIMESTAMP_LTZ. Restrict these numeric components to the characters accepted by Java and add a persisted-schema interoperability test. ########## paimon-python/pypaimon/schema/default_value.py: ########## @@ -0,0 +1,263 @@ +# 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. + +"""Validate a scalar column default before it is stored. + +The stored value is still the original string. Scalar checks follow the Java +string casts used by ``DefaultValueUtils`` for booleans, numbers, dates, times +and timestamps. ARRAY, MAP and ROW defaults are accepted by Java's +``StringToArrayCastRule``, ``StringToMapCastRule`` and ``StringToRowCastRule``; +Python does not parse those forms yet and rejects them explicitly. +""" + +import re +from calendar import monthrange +from datetime import datetime +from decimal import Decimal +from typing import Optional + +from pypaimon.schema.data_types import AtomicType + +_TRUE = {"t", "true", "y", "yes", "1"} +_FALSE = {"f", "false", "n", "no", "0"} +_INT_RANGES = { + "TINYINT": (-128, 127), + "SMALLINT": (-32768, 32767), + "INT": (-2147483648, 2147483647), + "INTEGER": (-2147483648, 2147483647), + "BIGINT": (-9223372036854775808, 9223372036854775807), +} +_INTEGRAL = set(_INT_RANGES) +_APPROXIMATE = {"FLOAT", "DOUBLE"} +_CHARACTER = {"STRING", "CHAR", "VARCHAR"} +_BINARY = {"BINARY", "VARBINARY", "BYTES"} +_UNSIGNED_INTEGER_RE = re.compile(r"[0-9]+") +# StringUtils.isNumeric guards the epoch fast path of DATE, TIME and TIMESTAMP. +# It allows a leading '-' but not '+', and only ASCII digits after it. +_JAVA_NUMERIC_RE = re.compile(r"-?[0-9]+") +_INTEGRAL_RE = re.compile(r"[+-]?(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]*)") +_DECIMAL_RE = re.compile(r"[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?") +# Float.parseFloat / Double.parseDouble accept hexadecimal literals and type +# suffixes, but not Python's underscores or case-insensitive special values. +_FLOAT_RE = re.compile( + r"[+-]?(?:NaN|Infinity|" + r"(?:(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)(?:[eE][+-]?[0-9]+)?|" + r"0[xX](?:[0-9a-fA-F]+(?:\.[0-9a-fA-F]*)?|\.[0-9a-fA-F]+)" + r"[pP][+-]?[0-9]+)[fFdD]?)") +_JAVA_TRIM_CHARS = "".join(chr(i) for i in range(33)) +# DateTimeUtils.DEFAULT_TIMESTAMP_FORMATTER: the time is optional, a space +# requires hour:minute:second, and 'T' requires hour:minute with optional +# seconds. appendFraction has a minimum width of 0, so a bare '.' is allowed. +# Anchor with \Z, not $, so a trailing newline does not slip through. +_TIMESTAMP_SPACE_RE = re.compile( + r"^(\d{4})-(\d{1,2})-(\d{1,2})" + r"(?: (\d{1,2}):(\d{1,2}):(\d{1,2})(?:\.(\d{0,9}))?)?\Z") +_TIMESTAMP_T_RE = re.compile( + r"^(\d{4})-(\d{1,2})-(\d{1,2})" + r"T(\d{1,2}):(\d{1,2})(?::(\d{1,2})(?:\.(\d{0,9}))?)?\Z") + + +def validate_default_value(data_type, default_value: Optional[str]) -> None: + """Raise if ``default_value`` cannot be cast to ``data_type``. + + ``None`` clears the default and is always accepted. + """ + if default_value is None: + return + if not isinstance(data_type, AtomicType): + raise RuntimeError( + "Unsupported default value `{}` for type {}. Python currently " + "validates scalar defaults only; ARRAY, MAP and ROW defaults " + "are not supported.".format(default_value, data_type)) + try: + _convert(data_type, _unquote(default_value)) + except Exception as cause: + raise RuntimeError( + "Unsupported default value `{}` for type {}".format( + default_value, data_type) + ) from cause + + +def _unquote(value: str) -> str: + if len(value) >= 2 and value.startswith("'") and value.endswith("'"): + return value[1:-1] + return value + + +def _convert(data_type, text: str) -> None: + root = _type_root(data_type.type) + if root in _CHARACTER or root in _BINARY: + return + if root == "BOOLEAN" or root == "BOOL": + if text.lower() not in _TRUE and text.lower() not in _FALSE: + raise ValueError("Cannot parse {!r} as BOOLEAN.".format(text)) + return + if root in _INTEGRAL: + _parse_integral(text, root) + return + if root in _APPROXIMATE: + if _FLOAT_RE.fullmatch(text.strip(_JAVA_TRIM_CHARS)) is None: + raise ValueError("Invalid floating-point value {}".format(text)) + return + if root == "DECIMAL" or root == "NUMERIC": + _parse_decimal(text) + return + if root == "DATE": + _parse_date(text) + return + if root == "TIME": + _parse_time(text) + return + if root == "TIMESTAMP": + if _JAVA_NUMERIC_RE.fullmatch(text): + _parse_integral(text, "BIGINT") + return + _parse_timestamp(text) + return + if root == "TIMESTAMP_LTZ": + _parse_timestamp(text) + return + raise RuntimeError( + "Default value do not support the type of {}".format(data_type)) + + +def _type_root(type_name: str) -> str: + match = re.match(r"^([A-Za-z_]+)", type_name.strip()) + if match is None: + raise ValueError("Unrecognized type {}".format(type_name)) + return match.group(1).upper() + + +def _parse_integral(text: str, root: str) -> int: + if _INTEGRAL_RE.fullmatch(text) is None: + raise ValueError("Invalid integer {}".format(text)) + sign = -1 if text[0] == "-" else 1 + body = text[1:] if text[0] in "+-" else text + # BinaryStringUtils truncates fractions, including '.5' and '10.'. It + # checks the original bytes, so whitespace and Unicode digits are invalid. + number = sign * int(body.split(".", 1)[0] or "0") + low, high = _INT_RANGES[root] + if number < low or number > high: + raise ValueError("Value {} is out of range for {}".format(number, root)) + return number + + +def _parse_decimal(text: str) -> None: + match = _DECIMAL_RE.fullmatch(text) + if match is None: + raise ValueError("Invalid decimal {}".format(text)) + Decimal(text) + + +def _parse_date(text: str) -> None: + if _JAVA_NUMERIC_RE.fullmatch(text): + _parse_integral(text, "INT") + return + # DateTimeUtils.parseDate also accepts year-month and ignores a timestamp's + # time suffix. Missing month/day components default to January/the first. + space = text.find(" ") + if space > 0: + text = text[:space] + parts = [part.strip(_JAVA_TRIM_CHARS) for part in text.split("-")] + if len(parts) > 3 or any( + _UNSIGNED_INTEGER_RE.fullmatch(part) is None for part in parts): + raise ValueError("Cannot parse {!r} as DATE.".format(text)) + values = [_parse_integral(part, "INT") for part in parts] + year, month, day = values + [1] * (3 - len(values)) + # Java DATE permits year zero, unlike datetime. + if not (0 <= year <= 9999 and 1 <= month <= 12 + and 1 <= day <= monthrange(year, month)[1]): + raise ValueError("Cannot parse {!r} as DATE.".format(text)) + + +def _parse_time(text: str) -> None: + if _JAVA_NUMERIC_RE.fullmatch(text): + _parse_integral(text, "INT") + return + # DateTimeUtils.parseTime splits on ':' and '.' and only requires each + # component to be a non-negative int. It does not bound hour/minute/second, + # so '25:00:00' and '12:00:99' are accepted, and an optional '[+-]hh[:mm]' + # timezone suffix may follow. Mirror that instead of building a real time. + end = len(text) + timezone = text.find("-") + if timezone < 0: + timezone = text.find("+") + if timezone >= 0: + end = timezone + suffix = text[timezone + 1:] + colon = suffix.find(":") + parts = [suffix] if colon < 0 else [suffix[:colon], suffix[colon + 1:]] + for part in parts: + _check_java_int(part) + body = text[:end] + colon1 = body.find(":") + if colon1 < 0: + _check_java_int(body) + return + _check_java_int(body[:colon1]) + colon2 = body.find(":", colon1 + 1) + if colon2 < 0: + _check_java_int(body[colon1 + 1:]) Review Comment: [P2] Reject TIME forms that the Java reader/writer cannot cast For a TIME column, `UpdateColumnDefaultValue(["b"], "12:30+01:00")` currently commits successfully, but loading that persisted schema in Java and calling `table.newWrite()` throws `DateTimeException`. This parser finds the separators only after removing the offset; the current Java DateTimeUtils.parseTime examines the original string, so this specific form is rejected there (`12:30:00+01:00` and `12:30+01` are accepted by both). The new validation must match the deployed Java cast before publishing the default, otherwise a successful Python ALTER leaves JVM writers unable to start. ########## paimon-python/pypaimon/schema/default_value.py: ########## @@ -0,0 +1,263 @@ +# 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. + +"""Validate a scalar column default before it is stored. + +The stored value is still the original string. Scalar checks follow the Java +string casts used by ``DefaultValueUtils`` for booleans, numbers, dates, times +and timestamps. ARRAY, MAP and ROW defaults are accepted by Java's +``StringToArrayCastRule``, ``StringToMapCastRule`` and ``StringToRowCastRule``; +Python does not parse those forms yet and rejects them explicitly. +""" + +import re +from calendar import monthrange +from datetime import datetime +from decimal import Decimal +from typing import Optional + +from pypaimon.schema.data_types import AtomicType + +_TRUE = {"t", "true", "y", "yes", "1"} +_FALSE = {"f", "false", "n", "no", "0"} +_INT_RANGES = { + "TINYINT": (-128, 127), + "SMALLINT": (-32768, 32767), + "INT": (-2147483648, 2147483647), + "INTEGER": (-2147483648, 2147483647), + "BIGINT": (-9223372036854775808, 9223372036854775807), +} +_INTEGRAL = set(_INT_RANGES) +_APPROXIMATE = {"FLOAT", "DOUBLE"} +_CHARACTER = {"STRING", "CHAR", "VARCHAR"} +_BINARY = {"BINARY", "VARBINARY", "BYTES"} +_UNSIGNED_INTEGER_RE = re.compile(r"[0-9]+") +# StringUtils.isNumeric guards the epoch fast path of DATE, TIME and TIMESTAMP. +# It allows a leading '-' but not '+', and only ASCII digits after it. +_JAVA_NUMERIC_RE = re.compile(r"-?[0-9]+") +_INTEGRAL_RE = re.compile(r"[+-]?(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]*)") +_DECIMAL_RE = re.compile(r"[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?") +# Float.parseFloat / Double.parseDouble accept hexadecimal literals and type +# suffixes, but not Python's underscores or case-insensitive special values. +_FLOAT_RE = re.compile( + r"[+-]?(?:NaN|Infinity|" + r"(?:(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)(?:[eE][+-]?[0-9]+)?|" + r"0[xX](?:[0-9a-fA-F]+(?:\.[0-9a-fA-F]*)?|\.[0-9a-fA-F]+)" + r"[pP][+-]?[0-9]+)[fFdD]?)") +_JAVA_TRIM_CHARS = "".join(chr(i) for i in range(33)) +# DateTimeUtils.DEFAULT_TIMESTAMP_FORMATTER: the time is optional, a space +# requires hour:minute:second, and 'T' requires hour:minute with optional +# seconds. appendFraction has a minimum width of 0, so a bare '.' is allowed. +# Anchor with \Z, not $, so a trailing newline does not slip through. +_TIMESTAMP_SPACE_RE = re.compile( + r"^(\d{4})-(\d{1,2})-(\d{1,2})" + r"(?: (\d{1,2}):(\d{1,2}):(\d{1,2})(?:\.(\d{0,9}))?)?\Z") +_TIMESTAMP_T_RE = re.compile( + r"^(\d{4})-(\d{1,2})-(\d{1,2})" + r"T(\d{1,2}):(\d{1,2})(?::(\d{1,2})(?:\.(\d{0,9}))?)?\Z") + + +def validate_default_value(data_type, default_value: Optional[str]) -> None: + """Raise if ``default_value`` cannot be cast to ``data_type``. + + ``None`` clears the default and is always accepted. + """ + if default_value is None: + return + if not isinstance(data_type, AtomicType): + raise RuntimeError( + "Unsupported default value `{}` for type {}. Python currently " + "validates scalar defaults only; ARRAY, MAP and ROW defaults " + "are not supported.".format(default_value, data_type)) + try: + _convert(data_type, _unquote(default_value)) + except Exception as cause: + raise RuntimeError( + "Unsupported default value `{}` for type {}".format( + default_value, data_type) + ) from cause + + +def _unquote(value: str) -> str: + if len(value) >= 2 and value.startswith("'") and value.endswith("'"): + return value[1:-1] + return value + + +def _convert(data_type, text: str) -> None: + root = _type_root(data_type.type) + if root in _CHARACTER or root in _BINARY: + return + if root == "BOOLEAN" or root == "BOOL": + if text.lower() not in _TRUE and text.lower() not in _FALSE: + raise ValueError("Cannot parse {!r} as BOOLEAN.".format(text)) + return + if root in _INTEGRAL: + _parse_integral(text, root) + return + if root in _APPROXIMATE: + if _FLOAT_RE.fullmatch(text.strip(_JAVA_TRIM_CHARS)) is None: + raise ValueError("Invalid floating-point value {}".format(text)) + return + if root == "DECIMAL" or root == "NUMERIC": + _parse_decimal(text) + return + if root == "DATE": + _parse_date(text) + return + if root == "TIME": + _parse_time(text) + return + if root == "TIMESTAMP": + if _JAVA_NUMERIC_RE.fullmatch(text): + _parse_integral(text, "BIGINT") + return + _parse_timestamp(text) + return + if root == "TIMESTAMP_LTZ": + _parse_timestamp(text) + return + raise RuntimeError( + "Default value do not support the type of {}".format(data_type)) + + +def _type_root(type_name: str) -> str: + match = re.match(r"^([A-Za-z_]+)", type_name.strip()) + if match is None: + raise ValueError("Unrecognized type {}".format(type_name)) + return match.group(1).upper() + + +def _parse_integral(text: str, root: str) -> int: + if _INTEGRAL_RE.fullmatch(text) is None: + raise ValueError("Invalid integer {}".format(text)) + sign = -1 if text[0] == "-" else 1 + body = text[1:] if text[0] in "+-" else text + # BinaryStringUtils truncates fractions, including '.5' and '10.'. It + # checks the original bytes, so whitespace and Unicode digits are invalid. + number = sign * int(body.split(".", 1)[0] or "0") + low, high = _INT_RANGES[root] + if number < low or number > high: + raise ValueError("Value {} is out of range for {}".format(number, root)) + return number + + +def _parse_decimal(text: str) -> None: + match = _DECIMAL_RE.fullmatch(text) + if match is None: + raise ValueError("Invalid decimal {}".format(text)) + Decimal(text) Review Comment: [P2] Enforce Java BigDecimal exponent and scale limits `Decimal(text)` accepts `1e2147483648` (and `1e-2147483648`), so the new setter persists it for DECIMAL(10,2). Java BigDecimal rejects these with `NumberFormatException` because its exponent/scale range is bounded, and initializing the writer for that table subsequently fails. This is distinct from ordinary precision overflow, which Java may cast to null. Validate Java-compatible exponent/scale bounds before committing the schema. -- 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]
