JingsongLi commented on code in PR #8187:
URL: https://github.com/apache/paimon/pull/8187#discussion_r3409022049
##########
paimon-python/pypaimon/schema/schema_manager.py:
##########
@@ -55,49 +174,70 @@ def _get_rename_mappings(changes: List[SchemaChange]) ->
dict:
def _handle_update_column_comment(
change: UpdateColumnComment, new_fields: List[DataField]
):
- field_name = change.field_names[-1]
- field_index = _find_field_index(new_fields, field_name)
- if field_index is None:
- raise ColumnNotExistException(field_name)
- field = new_fields[field_index]
- new_fields[field_index] = DataField(
- field.id, field.name, field.type, change.new_comment,
field.default_value
- )
+ def update_func(field: DataField, depth: int) -> DataField:
+ return DataField(
+ field.id, field.name, field.type, change.new_comment,
field.default_value
+ )
+ _update_nested_column(new_fields, change.field_names, update_func)
+
+
+def _assert_nullability_change(old_nullability: bool, new_nullability: bool,
+ field_name: str, disable_null_to_not_null:
bool):
+ if disable_null_to_not_null and old_nullability and not new_nullability:
+ raise ValueError(
+ "Cannot update column type from nullable to non nullable for {}. "
+ "You can set table configuration option "
+ "'alter-column-null-to-not-null.disabled' = 'false' "
+ "to allow converting null columns to not null".format(field_name)
+ )
def _handle_update_column_nullability(
- change: UpdateColumnNullability, new_fields: List[DataField]
+ change: UpdateColumnNullability, new_fields: List[DataField],
+ disable_null_to_not_null: bool
):
- field_name = change.field_names[-1]
- field_index = _find_field_index(new_fields, field_name)
- if field_index is None:
- raise ColumnNotExistException(field_name)
- field = new_fields[field_index]
from pypaimon.schema.data_types import DataTypeParser
- field_type_dict = field.type.to_dict()
- new_type = DataTypeParser.parse_data_type(field_type_dict)
- new_type.nullable = change.new_nullability
- new_fields[field_index] = DataField(
- field.id, field.name, new_type, field.description, field.default_value
- )
+ field_names = change.field_names
+ max_depth = len(field_names)
+
+ def update_func(field: DataField, depth: int) -> DataField:
+ source_root = _get_root_type(field.type, depth, max_depth)
+ _assert_nullability_change(
+ source_root.nullable, change.new_nullability,
+ '.'.join(field_names), disable_null_to_not_null)
+ new_root = DataTypeParser.parse_data_type(source_root.to_dict())
+ new_root.nullable = change.new_nullability
+ new_type = _get_array_map_type_with_target_type_root(
+ field.type, new_root, depth, max_depth)
+ return DataField(
+ field.id, field.name, new_type, field.description,
field.default_value
+ )
+ _update_nested_column(new_fields, field_names, update_func)
def _handle_update_column_type(
change: UpdateColumnType, new_fields: List[DataField]
):
- field_name = change.field_names[-1]
- field_index = _find_field_index(new_fields, field_name)
- if field_index is None:
- raise ColumnNotExistException(field_name)
- field = new_fields[field_index]
from pypaimon.schema.data_types import DataTypeParser
- new_type_dict = change.new_data_type.to_dict()
- new_type = DataTypeParser.parse_data_type(new_type_dict)
- if change.keep_nullability:
- new_type.nullable = field.type.nullable
- new_fields[field_index] = DataField(
- field.id, field.name, new_type, field.description, field.default_value
- )
+ field_names = change.field_names
+ max_depth = len(field_names)
+
+ def update_func(field: DataField, depth: int) -> DataField:
+ source_root = _get_root_type(field.type, depth, max_depth)
+ target_root =
DataTypeParser.parse_data_type(change.new_data_type.to_dict())
+ if change.keep_nullability:
+ target_root.nullable = source_root.nullable
+ if not supports_cast(source_root, target_root):
Review Comment:
This path still bypasses the new null-to-not-null guard when
`update_column_type` receives a non-nullable target type. For example,
`SchemaChange.update_column_type('v', AtomicType('BIGINT', nullable=False))`
succeeds with the default table options and changes a nullable column to
`BIGINT NOT NULL`, while Java `SchemaManager` rejects the same transition
unless `alter-column-null-to-not-null.disabled=false`. Could we call
`_assert_nullability_change(...)` here when `keep_nullability` is false, and
add a regression test for `update_column_type(... NOT NULL)`?
##########
paimon-python/pypaimon/read/split_read.py:
##########
@@ -627,6 +627,33 @@ def _genarate_deletion_file_readers(self):
class RawFileSplitRead(SplitRead):
+ def __init__(
+ self,
+ table,
+ predicate: Optional[Predicate],
+ read_type: List[DataField],
+ split: Split,
+ row_tracking_enabled: bool,
+ outer_extract_name_paths: Optional[List[List[str]]] = None,
+ outer_flat_read_type: Optional[List[DataField]] = None,
+ limit: Optional[int] = None):
+ # Nested-leaf projection is NOT pushed down by name: a leaf path is
+ # only valid against the latest schema, while each data file stores
+ # its own (possibly renamed / retyped) sub-fields. Instead the read
+ # widens to the full top-level columns, which the per-file field-id
+ # normalization aligns to the latest schema, and the requested leaf
+ # paths are extracted afterwards (``outer_extract_name_paths``).
+ super().__init__(
Review Comment:
Widening nested projection to the top-level field changes `read_type` from
e.g. `['id', 'mv_v']` to `['id', 'mv']`, so predicates built against the
projected leaf are considered outside the read fields and get silently dropped.
I reproduced `with_projection(['id', 'mv.v']).with_filter(mv_v > 15)` returning
both `mv.v = 10` and `20` on append-only and PK raw-convertible reads. Could
the predicate be applied after `NestedLeafBatchReader` / outer projection, or
rewritten to the widened nested path instead of being dropped?
##########
paimon-python/pypaimon/casting/data_type_casts.py:
##########
@@ -0,0 +1,189 @@
+# 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.
+
+"""Type-cast support rules used to validate ``update column type`` schema
+changes.
+
+The rules mirror the engine-wide cast specification so a type change accepted
+here is one the read path can also materialize: an *implicit* cast is a safe
+widening (e.g. INT -> BIGINT, any numeric -> DECIMAL/DOUBLE), while an
+*explicit* cast covers the broader, possibly lossy conversions a user opts into
+(e.g. DOUBLE -> INT truncation, anything -> STRING). Read-time execution then
+applies the conversion leniently.
+"""
+
+from pypaimon.schema.data_types import (ArrayType, AtomicType, DataTypeParser,
+ MapType, MultisetType, RowType,
+ VectorType)
+
+# ---- Type roots
--------------------------------------------------------------
+
+CHAR = "CHAR"
+VARCHAR = "VARCHAR"
+BOOLEAN = "BOOLEAN"
+BINARY = "BINARY"
+VARBINARY = "VARBINARY"
+DECIMAL = "DECIMAL"
+TINYINT = "TINYINT"
+SMALLINT = "SMALLINT"
+INTEGER = "INTEGER"
+BIGINT = "BIGINT"
+FLOAT = "FLOAT"
+DOUBLE = "DOUBLE"
+DATE = "DATE"
+TIME = "TIME"
+TIMESTAMP = "TIMESTAMP"
+TIMESTAMP_LTZ = "TIMESTAMP_LTZ"
+ARRAY = "ARRAY"
+MAP = "MAP"
+MULTISET = "MULTISET"
+ROW = "ROW"
+VECTOR = "VECTOR"
+VARIANT = "VARIANT"
+BLOB = "BLOB"
+
+# ---- Families
----------------------------------------------------------------
+
+CHARACTER_STRING = {CHAR, VARCHAR}
+BINARY_STRING = {BINARY, VARBINARY}
+INTEGER_NUMERIC = {TINYINT, SMALLINT, INTEGER, BIGINT}
+NUMERIC = INTEGER_NUMERIC | {FLOAT, DOUBLE, DECIMAL}
+TIMESTAMP_FAMILY = {TIMESTAMP, TIMESTAMP_LTZ}
+TIME_FAMILY = {TIME}
+DATETIME = {DATE, TIME, TIMESTAMP, TIMESTAMP_LTZ}
+PREDEFINED = {
+ CHAR, VARCHAR, BOOLEAN, BINARY, VARBINARY, DECIMAL,
+ TINYINT, SMALLINT, INTEGER, BIGINT, FLOAT, DOUBLE,
+ DATE, TIME, TIMESTAMP, TIMESTAMP_LTZ,
+}
+CONSTRUCTED = {ARRAY, MAP, MULTISET, ROW, VECTOR}
+# Constructed types the read path can render as a character string
+# ('{v1, v2}' / '[e1, e2]' / '{k -> v}'). VECTOR and MULTISET have no string
+# rendering, so a type change from them to CHAR/VARCHAR is rejected here
+# rather than failing when an old file is read.
+STRING_RENDERABLE_CONSTRUCTED = {ARRAY, MAP, ROW}
+
+
+def _root(data_type) -> str:
+ if isinstance(data_type, RowType):
+ return ROW
+ if isinstance(data_type, ArrayType):
+ return ARRAY
+ if isinstance(data_type, MapType):
+ return MAP
+ if isinstance(data_type, MultisetType):
+ return MULTISET
+ if isinstance(data_type, VectorType):
+ return VECTOR
+ if isinstance(data_type, AtomicType):
+ t = data_type.type.upper()
+ if t.startswith("DECIMAL") or t.startswith("NUMERIC") or
t.startswith("DEC"):
+ return DECIMAL
+ if t in ("INT", "INTEGER"):
+ return INTEGER
+ if t in (TINYINT, SMALLINT, BIGINT, FLOAT, DOUBLE, BOOLEAN, DATE):
+ return t
+ if t == "STRING" or t.startswith("VARCHAR"):
+ return VARCHAR
+ if t.startswith("CHAR"):
+ return CHAR
+ if t == "BYTES" or t.startswith("VARBINARY"):
+ return VARBINARY
+ if t.startswith("BINARY"):
+ return BINARY
+ if t == "BLOB":
+ return BLOB
+ if t.startswith("TIMESTAMP_LTZ"):
+ return TIMESTAMP_LTZ
+ if t.startswith("TIMESTAMP"):
+ return TIMESTAMP
+ if t.startswith("TIME"):
+ return TIME
+ if t == "VARIANT":
+ return VARIANT
+ return None
+
+
+def _build_rules():
+ implicit = {}
+ explicit = {}
+ # Identity cast for every root.
+ for root in (PREDEFINED | CONSTRUCTED | {VARIANT, BLOB}):
+ implicit[root] = {root}
+ explicit[root] = set()
+
+ def rule(target, implicit_from=None, explicit_from=None):
+ implicit[target] |= set(implicit_from or set())
+ explicit[target] |= set(explicit_from or set())
+
+ rule(CHAR, {CHAR}, PREDEFINED | STRING_RENDERABLE_CONSTRUCTED)
+ rule(VARCHAR, CHARACTER_STRING, PREDEFINED | STRING_RENDERABLE_CONSTRUCTED)
+ rule(BOOLEAN, {BOOLEAN}, CHARACTER_STRING | INTEGER_NUMERIC)
+ rule(BINARY, {BINARY}, CHARACTER_STRING | {VARBINARY})
+ rule(VARBINARY, BINARY_STRING, CHARACTER_STRING | {BINARY})
+ rule(DECIMAL, NUMERIC, CHARACTER_STRING | {BOOLEAN, TIMESTAMP,
TIMESTAMP_LTZ})
Review Comment:
Some casts accepted by these rules cannot actually be materialized by the
Python read path. I reproduced `TIMESTAMP(3) -> DECIMAL(10, 0)`:
`supports_cast` accepts it and alter succeeds, but reading old files fails in
`array.cast(...)` with `ArrowNotImplementedError: Unsupported cast from
timestamp[ms] to decimal128`. Java also checks `CastExecutors.resolve(...) !=
null`; Python needs either an equivalent executable-cast check or narrower
rules that only allow casts implemented by PyArrow/custom code.
--
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]
