Skip to content

Commit 9895baa

Browse files
[python] Check option changes on tables with snapshots like Java (#10856)
1 parent 5e5f2f4 commit 9895baa

4 files changed

Lines changed: 321 additions & 13 deletions

File tree

‎paimon-python/pypaimon/common/options/core_options.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ class CoreOptions:
123123
"type",
124124
"bucket-key",
125125
"bucket-function.type",
126+
"bucket-function.provider",
126127
"data-file.path-directory",
127128
"merge-engine",
128129
"sequence.snapshot-ordering",

‎paimon-python/pypaimon/schema/schema_manager.py‎

Lines changed: 112 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from pypaimon.common.identifier import DEFAULT_MAIN_BRANCH
2424
from pypaimon.common.json_util import JSON
2525
from pypaimon.common.options import CoreOptions, Options
26+
from pypaimon.common.options.options_utils import OptionsUtils
2627
from pypaimon.schema.column_directive_utils import (
2728
apply_add_column_directive, apply_directives,
2829
remove_dropped_directive_options)
@@ -305,18 +306,115 @@ def _is_unchanged_normalized_key(key, old_value, new_value, old_table_schema) ->
305306
"""Values whose canonical home is outside the options map (or the implicit
306307
default) are not a change when replayed. Mirrors the Java SchemaManager's
307308
isUnchangedNormalizedKey."""
308-
if old_value is not None or new_value is None:
309+
if new_value is None:
310+
return False
311+
if key == "type":
312+
effective_old = CoreOptions.TYPE.default_value() if old_value is None else old_value
313+
return new_value.lower() == effective_old.lower()
314+
if old_value is not None:
309315
return False
310316
if key == "primary-key":
311317
return _normalize_key_list(new_value) == old_table_schema.primary_keys
312318
if key == "partition":
313319
return _normalize_key_list(new_value) == old_table_schema.partition_keys
314-
if key == "type":
315-
# a table created without an explicit type is of the default type
316-
return new_value == CoreOptions.TYPE.default_value()
317320
return False
318321

319322

323+
def _parse_bucket(value: str) -> int:
324+
# Match Integer.parseInt's signed 32-bit decimal syntax.
325+
if value is None:
326+
raise ValueError("null")
327+
negative = value.startswith("-")
328+
digits = value[1:] if value.startswith(("+", "-")) else value
329+
try:
330+
if not digits:
331+
raise ValueError
332+
bucket = 0
333+
limit = 2147483648 if negative else 2147483647
334+
for c in digits:
335+
if not c.isdecimal() or ord(c) > 0xffff:
336+
raise ValueError
337+
bucket = bucket * 10 + int(c)
338+
if bucket > limit:
339+
raise ValueError
340+
return -bucket if negative else bucket
341+
except ValueError:
342+
raise ValueError(f'For input string: "{value}"') from None
343+
344+
345+
def _option_text(value) -> Optional[str]:
346+
# Options may be persisted as native values such as 2, True or enums; compare them as Options reads them.
347+
return None if value is None else OptionsUtils.convert_to_string(value)
348+
349+
350+
def _option_bool(value) -> bool:
351+
# Same interpretation as the runtime; unparsable values are false, like Boolean.parseBoolean.
352+
try:
353+
return value is not None and OptionsUtils.convert_to_boolean(value)
354+
except ValueError:
355+
return False
356+
357+
358+
def _check_alter_table_option(options: dict, key: str, old_value: Optional[str], new_value: str):
359+
# Booleans are read from the raw values, the way PyPaimon reads them.
360+
raw_old, raw_new = old_value, new_value
361+
old_value, new_value = _option_text(old_value), _option_text(new_value)
362+
if key in CoreOptions.IMMUTABLE_OPTIONS:
363+
raise NotImplementedError(f"Change '{key}' is not supported yet.")
364+
365+
if key == "data-evolution.nested-field.enabled":
366+
old_enabled = _option_bool(raw_old)
367+
new_enabled = _option_bool(raw_new)
368+
if old_enabled and not new_enabled:
369+
raise NotImplementedError(f"Cannot disable table option '{key}'.")
370+
371+
if key == "bucket":
372+
old_bucket = CoreOptions.BUCKET.default_value() if old_value is None else _parse_bucket(old_value)
373+
new_bucket = _parse_bucket(new_value)
374+
if old_bucket == -1:
375+
raise NotImplementedError("Cannot change bucket when it is -1.")
376+
if new_bucket == -1:
377+
raise NotImplementedError("Cannot change bucket to -1.")
378+
379+
if key == "deletion-vectors.enabled":
380+
dv_modifiable = _option_bool(options.get("deletion-vectors.modifiable"))
381+
if not dv_modifiable:
382+
old_dv = _option_bool(raw_old)
383+
new_dv = _option_bool(raw_new)
384+
if old_dv != new_dv:
385+
raise NotImplementedError(
386+
f"Cannot change deletion vectors mode from {str(old_dv).lower()} to {str(new_dv).lower()}. "
387+
"If modifying table deletion-vectors mode without full-compaction, "
388+
"this may result in data duplication. If you are confident, you can set table option "
389+
"'deletion-vectors.modifiable' = 'true' to allow deletion vectors modification.")
390+
391+
if key in ("ignore-delete", "ignore-update-before"):
392+
# The writers read ignore-update-before as the literal "true" only.
393+
parse = _option_bool if key == "ignore-delete" else (lambda v: str(v).lower() == "true")
394+
old_ignore = parse(raw_old)
395+
new_ignore = parse(raw_new)
396+
if old_ignore and not new_ignore:
397+
raise NotImplementedError(f"Cannot change {key} from true to false.")
398+
399+
if key == "clustering.columns" and "pk-clustering-override" in options:
400+
raise NotImplementedError("Cannot change clustering.columns when pk-clustering-override enabled.")
401+
402+
403+
def _check_reset_table_option(options: dict, key: str):
404+
if key in CoreOptions.IMMUTABLE_OPTIONS:
405+
raise NotImplementedError(f"Change '{key}' is not supported yet.")
406+
407+
if key == "bucket":
408+
raise NotImplementedError(f"Cannot reset {key}.")
409+
410+
if key in ("deletion-vectors.enabled", "data-evolution.nested-field.enabled",
411+
"ignore-delete", "ignore-update-before"):
412+
_check_alter_table_option(options, key, options.get(key), "false")
413+
414+
if key == "clustering.columns" and "pk-clustering-override" in options:
415+
raise NotImplementedError(f"Cannot reset {key} when pk-clustering-override enabled.")
416+
417+
320418
def _assert_not_updating_partition_keys(
321419
schema: 'TableSchema', field_names: List[str], operation: str):
322420
if len(field_names) > 1:
@@ -754,6 +852,7 @@ def commit_changes(self, changes: List[SchemaChange]) -> TableSchema:
754852
def _generate_table_schema(
755853
self, old_table_schema: TableSchema, changes: List[SchemaChange]
756854
) -> TableSchema:
855+
old_options = dict(old_table_schema.options)
757856
new_options = dict(old_table_schema.options)
758857
new_fields = []
759858
for field in old_table_schema.fields:
@@ -798,7 +897,7 @@ def _has_snapshots() -> bool:
798897
# compare against the accumulated options so repeated changes to
799898
# the same key in one batch apply in order
800899
old_value = new_options.get(change.key)
801-
unchanged = (old_value == change.value
900+
unchanged = (_option_text(old_value) == _option_text(change.value)
802901
or _is_unchanged_normalized_key(
803902
change.key, old_value, change.value, old_table_schema))
804903
if unchanged:
@@ -807,15 +906,17 @@ def _has_snapshots() -> bool:
807906
# format tables never create Paimon snapshots but can still hold
808907
# data -- reject independently of the snapshot check
809908
if change.key == "type":
810-
raise ValueError(f"Change '{change.key}' is not supported yet.")
811-
if change.key in CoreOptions.IMMUTABLE_OPTIONS and _has_snapshots():
812-
raise ValueError(f"Change '{change.key}' is not supported yet.")
909+
raise NotImplementedError(f"Change '{change.key}' is not supported yet.")
910+
if _has_snapshots():
911+
# Like Java, checks compare with the table's options before the batch.
912+
_check_alter_table_option(
913+
old_options, change.key, old_options.get(change.key), change.value)
813914
new_options[change.key] = change.value
814915
elif isinstance(change, RemoveOption):
815916
if change.key == "type":
816-
raise ValueError(f"Change '{change.key}' is not supported yet.")
817-
if change.key in CoreOptions.IMMUTABLE_OPTIONS and _has_snapshots():
818-
raise ValueError(f"Change '{change.key}' is not supported yet.")
917+
raise NotImplementedError(f"Change '{change.key}' is not supported yet.")
918+
if _has_snapshots():
919+
_check_reset_table_option(old_options, change.key)
819920
new_options.pop(change.key, None)
820921
elif isinstance(change, UpdateComment):
821922
new_comment = change.comment

‎paimon-python/pypaimon/tests/filesystem_catalog_test.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,18 @@ def test_alter_immutable_options(self):
304304
self.assertIn(
305305
"Change 'index-file-in-data-file-dir' is not supported yet.", str(ctx.exception))
306306

307+
for change, message in [
308+
(SchemaChange.set_option("bucket", "4"), "Cannot change bucket when it is -1."),
309+
(SchemaChange.remove_option("bucket"), "Cannot reset bucket."),
310+
(SchemaChange.set_option("deletion-vectors.enabled", "true"), "Cannot change deletion vectors mode"),
311+
(SchemaChange.set_option("bucket-function.provider", "custom"),
312+
"Change 'bucket-function.provider' is not supported yet."),
313+
]:
314+
with self.assertRaises(RuntimeError) as ctx:
315+
catalog.alter_table(identifier, [change], False)
316+
self.assertIn(message, str(ctx.exception))
317+
self.assertEqual(catalog.get_table(identifier).table_schema.id, table.table_schema.id)
318+
307319
# setting the default type explicitly is not a change
308320
catalog.alter_table(
309321
identifier, [SchemaChange.set_option("type", "table")], False)

0 commit comments

Comments
 (0)