nsivabalan commented on code in PR #20195: URL: https://github.com/apache/hudi/pull/20195#discussion_r4279667674
########## scripts/generate_config_catalog.py: ########## @@ -0,0 +1,1421 @@ +#!/usr/bin/env python3 +# +# 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. +# + +"""Extract every Hudi configuration property from the source tree, with code context. + +The published configuration reference tells a user a config's key, default and +description. It cannot tell them where the value is actually read, what else is +read beside it, or under what condition the read happens at all. That last part +is what makes a config silently do nothing. This script recovers it from source. + +Three passes: + + 1. Declarations. Parse ``ConfigProperty`` builder chains (and Flink's + ``ConfigOptions`` chains whose key is a literal ``hoodie.*``) out of the + main source trees. Chains span many lines and end at the statement + semicolon, so the parser works on brace/paren-balanced statement text + rather than on single lines. + + 2. Accessors. Find config-class methods whose body is a single + ``getInt(SOME_CONFIG)`` style read, so that call sites of + ``getInlineCompactDeltaCommitMax()`` can be attributed back to + ``hoodie.compact.inline.max.delta.commits``. + + 3. Read sites. Walk every source file again, attributing each mention of a + config constant or of a resolved accessor to the enclosing method, and + recording the other configs mentioned in that same method (co-configs) and + the enclosing ``if``/``switch``/``case`` conditions (gating). The gating + pass is a textual heuristic; every condition it emits is marked as such. + +Standard library only. Written to degrade: anything it cannot fully parse still +appears in the catalog, carrying a ``parseWarnings`` list. + +Usage: + python3 scripts/generate_config_catalog.py [--repo-root DIR] [--out-dir DIR] +""" + +import argparse +import json +import os +import re +import subprocess +import sys +from collections import defaultdict, OrderedDict + +# -------------------------------------------------------------------------- +# Tree walking +# -------------------------------------------------------------------------- + +# Modules that declare configs. Anything under the repo root that is not one of +# these is still scanned for *read sites*; declarations are only looked for in +# main source trees, which SOURCE_DIR_MARKER enforces. +SOURCE_DIR_MARKER = os.path.join("src", "main") + +EXCLUDED_PATH_PARTS = ( + os.sep + "target" + os.sep, + os.sep + "test" + os.sep, + os.sep + "src" + os.sep + "test" + os.sep, + os.sep + "node_modules" + os.sep, + os.sep + ".git" + os.sep, +) + +# Directories we never descend into at all. +PRUNED_DIR_NAMES = {"target", "node_modules", ".git", ".idea", "docker", "rfc"} + +READ_SITE_EXTENSIONS = (".java", ".scala") + +MAX_READ_SITES = 20 +MAX_CO_CONFIGS = 25 +MAX_GATES_PER_SITE = 3 + + +def is_excluded(path): + normalized = os.sep + path.strip(os.sep) + os.sep + return any(part in normalized for part in EXCLUDED_PATH_PARTS) + + +def walk_source_files(repo_root, extensions): + """Yield absolute paths of non-test, non-generated source files.""" + for dirpath, dirnames, filenames in os.walk(repo_root): + dirnames[:] = [d for d in dirnames if d not in PRUNED_DIR_NAMES] + if is_excluded(dirpath): + continue + for filename in filenames: + if filename.endswith(extensions): + full = os.path.join(dirpath, filename) + if not is_excluded(full): + yield full + + +# -------------------------------------------------------------------------- +# Lightweight Java lexing helpers +# -------------------------------------------------------------------------- + +def strip_comments_and_strings(text, keep_string_bodies=False): + """Blank out comments and (optionally) string bodies, preserving offsets. + + Offsets are preserved so that any index computed on the stripped text maps + straight back onto the original. Newlines survive so line numbers hold. + """ + out = list(text) + i = 0 + n = len(text) + while i < n: + ch = text[i] + if ch == "/" and i + 1 < n and text[i + 1] == "/": + while i < n and text[i] != "\n": + out[i] = " " + i += 1 + elif ch == "/" and i + 1 < n and text[i + 1] == "*": + out[i] = out[i + 1] = " " + i += 2 + while i < n and not (text[i] == "*" and i + 1 < n and text[i + 1] == "/"): + if text[i] != "\n": + out[i] = " " + i += 1 + if i < n: + out[i] = " " + if i + 1 < n: + out[i + 1] = " " + i += 2 + elif ch in ('"', "'"): + quote = ch + i += 1 + while i < n: + if text[i] == "\\": + if not keep_string_bodies: + out[i] = " " + if i + 1 < n: + out[i + 1] = " " + i += 2 + continue + if text[i] == quote: + break + if not keep_string_bodies and text[i] != "\n": + out[i] = " " + i += 1 + i += 1 + else: + i += 1 + return "".join(out) + + +def line_of(text, index): + return text.count("\n", 0, index) + 1 + + +def find_statement_end(text, start): + """Index just past the ``;`` ending the statement that starts at ``start``. + + Respects nesting and ignores semicolons inside strings, chars and comments. + """ + masked = strip_comments_and_strings(text[start:]) + depth_paren = depth_brace = depth_bracket = 0 + for offset, ch in enumerate(masked): + if ch == "(": + depth_paren += 1 + elif ch == ")": + depth_paren -= 1 + elif ch == "{": + depth_brace += 1 + elif ch == "}": + depth_brace -= 1 + elif ch == "[": + depth_bracket += 1 + elif ch == "]": + depth_bracket -= 1 + elif ch == ";" and depth_paren <= 0 and depth_brace <= 0 and depth_bracket <= 0: + return start + offset + 1 + return -1 + + +def split_top_level_args(arg_text): + """Split a call's argument text on top-level commas.""" + masked = strip_comments_and_strings(arg_text) + args = [] + depth = 0 + current_start = 0 + for i, ch in enumerate(masked): + if ch in "([{": + depth += 1 + elif ch in ")]}": + depth -= 1 + elif ch == "," and depth == 0: + args.append(arg_text[current_start:i].strip()) + current_start = i + 1 + tail = arg_text[current_start:].strip() + if tail: + args.append(tail) + return args + + +def extract_call_args(text, call_start): + """Given an index at the ``(`` of a call, return (args_text, index_past_close).""" + masked = strip_comments_and_strings(text) + if call_start >= len(text) or text[call_start] != "(": + return None, call_start + depth = 0 + for i in range(call_start, len(masked)): + if masked[i] == "(": + depth += 1 + elif masked[i] == ")": + depth -= 1 + if depth == 0: + return text[call_start + 1:i], i + 1 + return None, call_start + + +# -------------------------------------------------------------------------- +# Java string-literal evaluation +# -------------------------------------------------------------------------- + +_JAVA_ESCAPES = { + "n": "\n", "t": "\t", "r": "\r", "b": "\b", "f": "\f", + '"': '"', "'": "'", "\\": "\\", "0": "\0", +} + +STRING_LITERAL_RE = re.compile(r'"((?:[^"\\]|\\.)*)"', re.DOTALL) + + +def unescape_java(raw): + out = [] + i = 0 + while i < len(raw): + if raw[i] == "\\" and i + 1 < len(raw): + nxt = raw[i + 1] + if nxt == "u": + try: + out.append(chr(int(raw[i + 2:i + 6], 16))) + i += 6 + continue + except ValueError: + pass + out.append(_JAVA_ESCAPES.get(nxt, nxt)) + i += 2 + else: + out.append(raw[i]) + i += 1 + return "".join(out) + + +IDENT_PATH_RE = re.compile(r"[A-Za-z_$][A-Za-z0-9_$]*(?:\s*\.\s*[A-Za-z_$][A-Za-z0-9_$]*)*") + + +def concatenated_string_literal(expr, string_constants=None, owner_class=None, _depth=0): + """Evaluate a Java expression that is a concatenation of string literals. + + With ``string_constants`` supplied, identifiers that name a ``static final + String`` constant are substituted too, which is how the many + ``SOME_PREFIX + "suffix"`` config keys get resolved. Returns None when the + expression contains anything else, so that callers can distinguish "a + literal we read exactly" from "something computed". + """ + expr = expr.strip() + if not expr or _depth > 8: + return None + pieces = [] + pos = 0 + saw_literal = False + while pos < len(expr): + match = STRING_LITERAL_RE.match(expr, pos) + if match: + pieces.append(unescape_java(match.group(1))) + saw_literal = True + pos = match.end() + continue + ch = expr[pos] + if ch.isspace() or ch == "+": + pos += 1 + continue + if string_constants is not None: + ident = IDENT_PATH_RE.match(expr, pos) + if ident: + resolved = lookup_string_constant( + ident.group(0), string_constants, owner_class, _depth) + if resolved is not None: + pieces.append(resolved) + saw_literal = True + pos = ident.end() + continue + return None + return "".join(pieces) if saw_literal else None + + +def lookup_string_constant(path, string_constants, owner_class, depth): + """Resolve ``PREFIX`` / ``Holder.PREFIX`` to its literal value, recursively.""" + parts = [p.strip() for p in path.split(".")] + const = parts[-1] + qualifier = parts[-2] if len(parts) >= 2 else None + for candidate in ((qualifier, const), (owner_class, const), (None, const)): + expression = string_constants.get(candidate) + if expression is None: + continue + resolved = concatenated_string_literal( + expression, string_constants, candidate[0] or owner_class, depth + 1) + if resolved is not None: + return resolved + return None + + +ENUM_DESCRIPTION_RE = re.compile(r"@EnumDescription\s*\(") +ENUM_FIELD_DESCRIPTION_RE = re.compile(r"@EnumFieldDescription\s*\(") +ENUM_DECL_RE = re.compile(r"\benum\s+([A-Za-z_$][A-Za-z0-9_$]*)") +# The last constant in an enum body is followed by `}` rather than `,` or `;`. +ENUM_CONSTANT_RE = re.compile(r"\A\s*([A-Z][A-Z0-9_]*)\s*[,;(){}]") + + +def collect_enum_documentation(repo_root): + """Pass 0b. Enum name -> its ``@EnumDescription`` and per-constant descriptions. + + ``withDocumentation(SomeEnum.class)`` means the user-facing text for that + config lives on the enum, not at the declaration. Without this the catalog + would report "no documentation" for exactly the configs whose valid values + matter most. + """ + documented = {} + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + continue + if "@EnumDescription" not in original: + continue + masked = strip_comments_and_strings(original, keep_string_bodies=True) + for match in ENUM_DESCRIPTION_RE.finditer(masked): + args, after = extract_call_args(masked, match.end() - 1) + if args is None: + continue + enum_match = ENUM_DECL_RE.search(masked, after) + if not enum_match: + continue + name = enum_match.group(1) + body_start = masked.find("{", enum_match.end()) + if body_start < 0: + continue + depth = 0 + body_end = len(masked) + for i in range(body_start, len(masked)): + if masked[i] == "{": + depth += 1 + elif masked[i] == "}": + depth -= 1 + if depth == 0: + body_end = i + break + values = [] + for field in ENUM_FIELD_DESCRIPTION_RE.finditer(masked, body_start, body_end): + field_args, field_after = extract_call_args(masked, field.end() - 1) + if field_args is None: + continue + tail = masked[field_after:field_after + 160] + constant = ENUM_CONSTANT_RE.match(tail) + if not constant: + continue + values.append(OrderedDict([ + ("value", constant.group(1)), + ("description", concatenated_string_literal(field_args) or ""), + ])) + documented[name] = OrderedDict([ + ("enum", name), + ("file", os.path.relpath(file_path, repo_root)), + ("description", concatenated_string_literal(args) or ""), + ("values", values), + ]) + return documented + + +STRING_CONSTANT_RE = re.compile( + r"\bstatic\s+final\s+String\s+(?P<name>[A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*(?P<value>[^;]*);") + + +def collect_string_constants(repo_root): + """Pass 0. (class, constant) -> its initializer expression, for key prefixes. + + Keyed both by owning class and by bare name. A bare name claimed by two + different expressions is dropped, so an ambiguous prefix never silently + resolves to the wrong value. + """ + by_qualified = {} + bare = {} + bare_conflicts = set() + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + continue + if "static final String" not in original: + continue + masked = strip_comments_and_strings(original, keep_string_bodies=True) + for match in STRING_CONSTANT_RE.finditer(masked): + name = match.group("name") + value = match.group("value").strip() + if not value or len(value) > 400: + continue + owner = find_enclosing_class(masked, match.start()) + by_qualified[(owner, name)] = value + if name in bare and bare[name] != value: + bare_conflicts.add(name) + bare[name] = value + for name in bare_conflicts: + bare.pop(name, None) + constants = {(None, name): value for name, value in bare.items()} + constants.update(by_qualified) + return constants + + +def literal_or_expression(expr): + """Return (value, is_literal) for a default-value expression.""" + literal = concatenated_string_literal(expr) + if literal is not None: + return literal, True + collapsed = " ".join(expr.split()) + if re.fullmatch(r"(true|false)", collapsed): + return collapsed, True + if re.fullmatch(r"-?\d+[LlFfDd]?", collapsed): + return collapsed.rstrip("LlFfDd"), True + if re.fullmatch(r"-?\d*\.\d+[FfDd]?", collapsed): + return collapsed.rstrip("FfDd"), True + return collapsed, False + + +# -------------------------------------------------------------------------- +# Pass 1 -- config declarations +# -------------------------------------------------------------------------- + +# Matches both `ConfigProperty<String> FOO = ConfigProperty` and the Flink +# `ConfigOption<String> FOO = ConfigOptions` forms, plus the rarer single-line +# `= ConfigProperty.key(...)`. +DECLARATION_RE = re.compile( + r"(?P<decl>(?:public|protected|private)?\s*(?:static\s+)?(?:final\s+)?" + r"(?P<holder>ConfigProperty|ConfigOption)\s*<\s*(?P<type>[^>]*(?:<[^>]*>)?[^>]*)\s*>\s+" + r"(?P<name>[A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*)" + r"(?P<builder>ConfigProperty|ConfigOptions)\b" +) + +CLASS_DECL_RE = re.compile( + r"\b(?:public|protected|private)?\s*(?:static\s+)?(?:final\s+)?(?:abstract\s+)?" + r"(?:class|interface|enum)\s+([A-Za-z_$][A-Za-z0-9_$]*)" +) + +# Builder steps we care about, applied to the chain text after the key. +CHAIN_STEP_RE = re.compile(r"\.\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*\(") + + +def find_enclosing_class(text, index): + """Innermost-looking class name declared before ``index``.""" + best = None + for match in CLASS_DECL_RE.finditer(text, 0, index): + best = match.group(1) + return best + + +def parse_chain_steps(chain_text): + """Return an ordered list of (method_name, args_text) from a builder chain. + + Comments are blanked first -- authors do put explanatory comments between a + builder call's arguments, and those must not end up inside an extracted + value. String bodies are kept so literals survive. + """ + chain_text = strip_comments_and_strings(chain_text, keep_string_bodies=True) + steps = [] + pos = 0 + while True: + match = CHAIN_STEP_RE.search(chain_text, pos) + if not match: + break + args, after = extract_call_args(chain_text, match.end() - 1) + if args is None: + pos = match.end() + continue + steps.append((match.group(1), args)) + pos = after + return steps + + +DERIVED_KEY_RE = re.compile( + r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*key\(\s*\)\s*(?:\+\s*(.+))?") + + +def derived_from_config_key(expr, string_constants, owner_class, base_key_resolver): + """Resolve `OTHER_CONFIG.key()` and `OTHER_CONFIG.key() + ".suffix"`. + + Several configs name their own key, and several name an alternative key, + relative to another config's. Returns None when the base config is unknown. + """ + match = DERIVED_KEY_RE.fullmatch(" ".join(expr.split())) + if not match: + return None + base = base_key_resolver(match.group(1)) + if base is None: + return None + if match.group(2) is None: + return base + suffix = concatenated_string_literal(match.group(2), string_constants, owner_class) + return None if suffix is None else base + suffix + + +def parse_declaration(masked_for_scan, original, match, file_path, repo_root, + string_constants, base_key_resolver=lambda path: None, + enum_docs=None): + """Build one catalog entry from a declaration match. Never raises.""" + warnings = [] + start = match.start() + end = find_statement_end(original, start) + if end < 0: + end = min(len(original), start + 4000) + warnings.append("statement terminator not found; chain truncated at 4000 chars") + chain_text = original[start:end] + + steps = parse_chain_steps(chain_text) + step_names = [name for name, _ in steps] + + enclosing_class = find_enclosing_class(masked_for_scan, start) + + key = None + key_expr = None + key_resolution = None + for name, args in steps: + if name == "key": + key_expr = args.strip() + key = concatenated_string_literal(args) + if key is not None: + key_resolution = "literal" + else: + key = concatenated_string_literal( + args, string_constants, enclosing_class) + if key is not None: + key_resolution = "resolvedFromConstants" + break + + if key is None and key_expr: + # Flink re-exports read the key off another ConfigProperty, and some + # Java configs build the key from a shared prefix constant. + reference = re.fullmatch( + r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*key\(\s*\)", key_expr.strip()) + if reference: + return None, "alias:" + reference.group(1) + # `OTHER_CONFIG.key() + ".suffix"` -- a key derived from another config's. + key = derived_from_config_key( + key_expr, string_constants, enclosing_class, base_key_resolver) + if key is not None: + key_resolution = "derivedFromConfigKey" + else: + warnings.append( + "key is a computed expression: " + " ".join(key_expr.split())[:160]) + + entry = OrderedDict() + entry["key"] = key + entry["keyResolution"] = key_resolution + entry["keyExpression"] = (" ".join(key_expr.split()) + if key_expr and key_resolution != "literal" else None) + entry["type"] = " ".join(match.group("type").split()) + entry["declaredIn"] = OrderedDict([ + ("class", enclosing_class), + ("constant", match.group("name")), + ("file", os.path.relpath(file_path, repo_root)), + ("line", line_of(original, start)), + ]) + entry["configGroup"] = entry["declaredIn"]["class"] + entry["builderStyle"] = "ConfigProperty" if match.group("builder") == "ConfigProperty" else "FlinkConfigOptions" + + # Default value. + default_value = None + default_is_literal = False + has_default = None + for name, args in steps: + if name == "defaultValue": + parts = split_top_level_args(args) + if parts: + default_value, default_is_literal = literal_or_expression(parts[0]) + has_default = True + if len(parts) > 1: + doc_on_default = concatenated_string_literal(parts[1]) + if doc_on_default: + entry["docOnDefaultValue"] = doc_on_default + break + if name == "noDefaultValue": + has_default = False + break + if has_default is None: + has_default = False + if entry["builderStyle"] == "ConfigProperty": + warnings.append("neither defaultValue() nor noDefaultValue() found in chain") + entry["hasDefaultValue"] = has_default + entry["defaultValue"] = default_value + entry["defaultValueIsLiteral"] = default_is_literal + if has_default and not default_is_literal: + entry["defaultValueNote"] = "computed at class-init time; shown as the source expression" + + # Documentation: a string, or a class reference whose text lives elsewhere. + documentation = None + documentation_source = None + enum_documentation = None + for name, args in steps: + if name not in ("withDocumentation", "withDescription"): + continue + parts = split_top_level_args(args) + if not parts: + continue + class_ref = re.fullmatch(r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*class", parts[0].strip()) + if class_ref: + enum_name = class_ref.group(1).split(".")[-1] + documentation_source = "enumClass:" + enum_name + extra = concatenated_string_literal(parts[1]) if len(parts) > 1 else None + described = (enum_docs or {}).get(enum_name) + if described: + entry_enum = described + pieces = [p for p in (extra, described["description"]) if p] + documentation = "\n".join(pieces) + enum_documentation = entry_enum + else: + documentation = extra + warnings.append( + "documentation points at enum %s, whose @EnumDescription was not found" + % enum_name) + else: + documentation = concatenated_string_literal(parts[0]) + documentation_source = "literal" + if documentation is None: + documentation = concatenated_string_literal( + parts[0], string_constants, enclosing_class) + documentation_source = "resolvedFromConstants" + if documentation is None: + documentation_source = None + warnings.append("documentation is a computed expression") + break + entry["documentation"] = documentation + entry["documentationSource"] = documentation_source + entry["enumDocumentation"] = enum_documentation + + def collect_string_args(step_name): + for name, args in steps: + if name == step_name: + values = [] + for part in split_top_level_args(args): + literal = concatenated_string_literal(part) + if literal is None: + literal = concatenated_string_literal( + part, string_constants, enclosing_class) + if literal is None: + literal = derived_from_config_key( + part, string_constants, enclosing_class, base_key_resolver) + values.append(literal if literal is not None + else " ".join(part.split())) + return values + return [] + + entry["validValues"] = collect_string_args("withValidValues") + # A config whose documentation is an enum class declares its permitted values on the enum + # rather than via withValidValues(...). Fall back to the enum's constants so that + # "what can I set this to?" is answerable for strategy/policy configs, which are exactly + # the ones where it matters most. + if not entry["validValues"] and enum_documentation: + entry["validValues"] = [v["value"] for v in enum_documentation.get("values", []) if v.get("value")] + entry["alternatives"] = collect_string_args("withAlternatives") + since = collect_string_args("sinceVersion") + entry["sinceVersion"] = since[0] if since else None + deprecated = collect_string_args("deprecatedAfter") + entry["deprecatedAfter"] = deprecated[0] if deprecated else None + entry["supportedVersions"] = collect_string_args("supportedVersions") + entry["advanced"] = "markAdvanced" in step_names + entry["hasInferFunction"] = "withInferFunction" in step_names + entry["builderSteps"] = step_names + + if key is None: + warnings.append("key could not be resolved to a string literal") + if documentation is None and documentation_source != "enumClass": + warnings.append("no documentation text resolved") + + entry["parseWarnings"] = warnings + return entry, None + + +def collect_declarations(repo_root, verbose=False): + """Pass 1. Returns (entries, constant_index, stats).""" + entries = [] + # (file-local class, constant name) -> config key, so later passes can map a + # constant reference back to the config it names. + constant_index = {} + alias_pending = [] + stats = defaultdict(int) + + string_constants = collect_string_constants(repo_root) + stats["stringConstants"] = len(string_constants) + enum_docs = collect_enum_documentation(repo_root) + stats["documentedEnums"] = len(enum_docs) + if verbose: + print(" string constants indexed: %d" % len(string_constants), file=sys.stderr) + print(" documented enums indexed: %d" % len(enum_docs), file=sys.stderr) + + for file_path in walk_source_files(repo_root, (".java",)): Review Comment: Done — the generator now has a Scala pass covering `DataSourceReadOptions` and `DataSourceWriteOptions`, resolving defaults and valid values through Scala `val` string constants. 52 configs are declared in Scala and carry `"language": "scala"`, including all three you named. They come with an honesty flag: the generator does not parse Scala method bodies, so a Scala read site has `gatesResolved: false` and the config carries `readContextResolved: false` with a note saying how many of its sites were not analysed. An empty `gatingConditions` there means *not analysed*, not *unconditional*, and the skill is told to say which it is. ########## hudi-ai-operator/skills/hudi-config-consultant/references/catalog-schema.md: ########## @@ -0,0 +1,181 @@ +<!-- +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. +--> + +# Catalog schema and how to query it + +## Envelope + +```json +{ + "schemaVersion": 1, + "hudiVersion": "1.3.0-SNAPSHOT", + "generatedFrom": "<git sha of the tree it was generated from>", + "configs": [ ... ] +} +``` + +`configs` is sorted by `key`, so regeneration produces a readable diff. Read `hudiVersion` and +`generatedFrom` on every query; they belong in your answer. + +## A config entry + +### Declared facts — extracted verbatim from the builder chain + +| Field | Meaning | +|---|---| +| `key` | The property key, e.g. `hoodie.compact.inline.max.delta.commits`. | +| `keyResolution` | `literal`, `resolvedFromConstants` (key built from a `static final String` prefix), or `derivedFromConfigKey` (key built from another config's key). | +| `keyExpression` | The source expression, when the key was not a plain literal. | +| `type` | The declared type parameter: `String`, `Boolean`, `Integer`, `Long`, … | +| `declaredIn` | `{class, constant, file, line}` — where the `ConfigProperty` is declared. | +| `configGroup` | The owning config class, e.g. `HoodieCompactionConfig`. Use this to group related configs. | +| `module` | Top-level Maven module, e.g. `hudi-client`. | +| `hasDefaultValue` | `false` for `noDefaultValue()` configs — these are required or inferred. | Review Comment: Good catch, and it was the worst failure mode in the catalog — a confidently wrong default. The generator now has a pass for engine-specific effective defaults, reading the `setDefaultValue(PROP, getDefaultXxx(engineType))` calls in each builder's `build()`. Nine configs have them, including all four you named: | Config | Declared | Spark | Flink | Java | |---|---|---|---|---| | `hoodie.index.type` | *(none)* | SIMPLE | INMEMORY | SIMPLE | | `hoodie.write.markers.type` | TIMELINE_SERVER_BASED | *conditional* | DIRECT | DIRECT | | `hoodie.clustering.plan.strategy.class` | Spark size-based | *conditional* | *conditional* | Java size-based | | `hoodie.parquet.compression.codec` | *(none)* | *conditional* | zstd | gzip | Four are conditional rather than fixed — Spark's markers type branches on `isEmbeddedTimelineServerEnabled()`, the parquet codec on the Spark runtime version — so those carry `conditionalEngineDefaults` with the condition and both branches, and the skill reports the condition rather than picking a side. Each carries `engineDefaultsResolvedFrom` with the `file:line` of the overriding call. The skill is now instructed to give the default for the user's engine, ask which engine if it does not know, and never fall back to the declared value for an engine the generator could not resolve. The schema doc line you flagged has been corrected too. ########## scripts/generate_config_catalog.py: ########## @@ -0,0 +1,1421 @@ +#!/usr/bin/env python3 +# +# 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. +# + +"""Extract every Hudi configuration property from the source tree, with code context. + +The published configuration reference tells a user a config's key, default and +description. It cannot tell them where the value is actually read, what else is +read beside it, or under what condition the read happens at all. That last part +is what makes a config silently do nothing. This script recovers it from source. + +Three passes: + + 1. Declarations. Parse ``ConfigProperty`` builder chains (and Flink's + ``ConfigOptions`` chains whose key is a literal ``hoodie.*``) out of the + main source trees. Chains span many lines and end at the statement + semicolon, so the parser works on brace/paren-balanced statement text + rather than on single lines. + + 2. Accessors. Find config-class methods whose body is a single + ``getInt(SOME_CONFIG)`` style read, so that call sites of + ``getInlineCompactDeltaCommitMax()`` can be attributed back to + ``hoodie.compact.inline.max.delta.commits``. + + 3. Read sites. Walk every source file again, attributing each mention of a + config constant or of a resolved accessor to the enclosing method, and + recording the other configs mentioned in that same method (co-configs) and + the enclosing ``if``/``switch``/``case`` conditions (gating). The gating + pass is a textual heuristic; every condition it emits is marked as such. + +Standard library only. Written to degrade: anything it cannot fully parse still +appears in the catalog, carrying a ``parseWarnings`` list. + +Usage: + python3 scripts/generate_config_catalog.py [--repo-root DIR] [--out-dir DIR] +""" + +import argparse +import json +import os +import re +import subprocess +import sys +from collections import defaultdict, OrderedDict + +# -------------------------------------------------------------------------- +# Tree walking +# -------------------------------------------------------------------------- + +# Modules that declare configs. Anything under the repo root that is not one of +# these is still scanned for *read sites*; declarations are only looked for in +# main source trees, which SOURCE_DIR_MARKER enforces. +SOURCE_DIR_MARKER = os.path.join("src", "main") + +EXCLUDED_PATH_PARTS = ( + os.sep + "target" + os.sep, + os.sep + "test" + os.sep, + os.sep + "src" + os.sep + "test" + os.sep, + os.sep + "node_modules" + os.sep, + os.sep + ".git" + os.sep, +) + +# Directories we never descend into at all. +PRUNED_DIR_NAMES = {"target", "node_modules", ".git", ".idea", "docker", "rfc"} + +READ_SITE_EXTENSIONS = (".java", ".scala") + +MAX_READ_SITES = 20 +MAX_CO_CONFIGS = 25 +MAX_GATES_PER_SITE = 3 + + +def is_excluded(path): + normalized = os.sep + path.strip(os.sep) + os.sep + return any(part in normalized for part in EXCLUDED_PATH_PARTS) + + +def walk_source_files(repo_root, extensions): + """Yield absolute paths of non-test, non-generated source files.""" + for dirpath, dirnames, filenames in os.walk(repo_root): + dirnames[:] = [d for d in dirnames if d not in PRUNED_DIR_NAMES] + if is_excluded(dirpath): + continue + for filename in filenames: + if filename.endswith(extensions): + full = os.path.join(dirpath, filename) + if not is_excluded(full): + yield full + + +# -------------------------------------------------------------------------- +# Lightweight Java lexing helpers +# -------------------------------------------------------------------------- + +def strip_comments_and_strings(text, keep_string_bodies=False): + """Blank out comments and (optionally) string bodies, preserving offsets. + + Offsets are preserved so that any index computed on the stripped text maps + straight back onto the original. Newlines survive so line numbers hold. + """ + out = list(text) + i = 0 + n = len(text) + while i < n: + ch = text[i] + if ch == "/" and i + 1 < n and text[i + 1] == "/": + while i < n and text[i] != "\n": + out[i] = " " + i += 1 + elif ch == "/" and i + 1 < n and text[i + 1] == "*": + out[i] = out[i + 1] = " " + i += 2 + while i < n and not (text[i] == "*" and i + 1 < n and text[i + 1] == "/"): + if text[i] != "\n": + out[i] = " " + i += 1 + if i < n: + out[i] = " " + if i + 1 < n: + out[i + 1] = " " + i += 2 + elif ch in ('"', "'"): + quote = ch + i += 1 + while i < n: + if text[i] == "\\": + if not keep_string_bodies: + out[i] = " " + if i + 1 < n: + out[i + 1] = " " + i += 2 + continue + if text[i] == quote: + break + if not keep_string_bodies and text[i] != "\n": + out[i] = " " + i += 1 + i += 1 + else: + i += 1 + return "".join(out) + + +def line_of(text, index): + return text.count("\n", 0, index) + 1 + + +def find_statement_end(text, start): + """Index just past the ``;`` ending the statement that starts at ``start``. + + Respects nesting and ignores semicolons inside strings, chars and comments. + """ + masked = strip_comments_and_strings(text[start:]) + depth_paren = depth_brace = depth_bracket = 0 + for offset, ch in enumerate(masked): + if ch == "(": + depth_paren += 1 + elif ch == ")": + depth_paren -= 1 + elif ch == "{": + depth_brace += 1 + elif ch == "}": + depth_brace -= 1 + elif ch == "[": + depth_bracket += 1 + elif ch == "]": + depth_bracket -= 1 + elif ch == ";" and depth_paren <= 0 and depth_brace <= 0 and depth_bracket <= 0: + return start + offset + 1 + return -1 + + +def split_top_level_args(arg_text): + """Split a call's argument text on top-level commas.""" + masked = strip_comments_and_strings(arg_text) + args = [] + depth = 0 + current_start = 0 + for i, ch in enumerate(masked): + if ch in "([{": + depth += 1 + elif ch in ")]}": + depth -= 1 + elif ch == "," and depth == 0: + args.append(arg_text[current_start:i].strip()) + current_start = i + 1 + tail = arg_text[current_start:].strip() + if tail: + args.append(tail) + return args + + +def extract_call_args(text, call_start): + """Given an index at the ``(`` of a call, return (args_text, index_past_close).""" + masked = strip_comments_and_strings(text) + if call_start >= len(text) or text[call_start] != "(": + return None, call_start + depth = 0 + for i in range(call_start, len(masked)): + if masked[i] == "(": + depth += 1 + elif masked[i] == ")": + depth -= 1 + if depth == 0: + return text[call_start + 1:i], i + 1 + return None, call_start + + +# -------------------------------------------------------------------------- +# Java string-literal evaluation +# -------------------------------------------------------------------------- + +_JAVA_ESCAPES = { + "n": "\n", "t": "\t", "r": "\r", "b": "\b", "f": "\f", + '"': '"', "'": "'", "\\": "\\", "0": "\0", +} + +STRING_LITERAL_RE = re.compile(r'"((?:[^"\\]|\\.)*)"', re.DOTALL) + + +def unescape_java(raw): + out = [] + i = 0 + while i < len(raw): + if raw[i] == "\\" and i + 1 < len(raw): + nxt = raw[i + 1] + if nxt == "u": + try: + out.append(chr(int(raw[i + 2:i + 6], 16))) + i += 6 + continue + except ValueError: + pass + out.append(_JAVA_ESCAPES.get(nxt, nxt)) + i += 2 + else: + out.append(raw[i]) + i += 1 + return "".join(out) + + +IDENT_PATH_RE = re.compile(r"[A-Za-z_$][A-Za-z0-9_$]*(?:\s*\.\s*[A-Za-z_$][A-Za-z0-9_$]*)*") + + +def concatenated_string_literal(expr, string_constants=None, owner_class=None, _depth=0): + """Evaluate a Java expression that is a concatenation of string literals. + + With ``string_constants`` supplied, identifiers that name a ``static final + String`` constant are substituted too, which is how the many + ``SOME_PREFIX + "suffix"`` config keys get resolved. Returns None when the + expression contains anything else, so that callers can distinguish "a + literal we read exactly" from "something computed". + """ + expr = expr.strip() + if not expr or _depth > 8: + return None + pieces = [] + pos = 0 + saw_literal = False + while pos < len(expr): + match = STRING_LITERAL_RE.match(expr, pos) + if match: + pieces.append(unescape_java(match.group(1))) + saw_literal = True + pos = match.end() + continue + ch = expr[pos] + if ch.isspace() or ch == "+": + pos += 1 + continue + if string_constants is not None: + ident = IDENT_PATH_RE.match(expr, pos) + if ident: + resolved = lookup_string_constant( + ident.group(0), string_constants, owner_class, _depth) + if resolved is not None: + pieces.append(resolved) + saw_literal = True + pos = ident.end() + continue + return None + return "".join(pieces) if saw_literal else None + + +def lookup_string_constant(path, string_constants, owner_class, depth): + """Resolve ``PREFIX`` / ``Holder.PREFIX`` to its literal value, recursively.""" + parts = [p.strip() for p in path.split(".")] + const = parts[-1] + qualifier = parts[-2] if len(parts) >= 2 else None + for candidate in ((qualifier, const), (owner_class, const), (None, const)): + expression = string_constants.get(candidate) + if expression is None: + continue + resolved = concatenated_string_literal( + expression, string_constants, candidate[0] or owner_class, depth + 1) + if resolved is not None: + return resolved + return None + + +ENUM_DESCRIPTION_RE = re.compile(r"@EnumDescription\s*\(") +ENUM_FIELD_DESCRIPTION_RE = re.compile(r"@EnumFieldDescription\s*\(") +ENUM_DECL_RE = re.compile(r"\benum\s+([A-Za-z_$][A-Za-z0-9_$]*)") +# The last constant in an enum body is followed by `}` rather than `,` or `;`. +ENUM_CONSTANT_RE = re.compile(r"\A\s*([A-Z][A-Z0-9_]*)\s*[,;(){}]") + + +def collect_enum_documentation(repo_root): + """Pass 0b. Enum name -> its ``@EnumDescription`` and per-constant descriptions. + + ``withDocumentation(SomeEnum.class)`` means the user-facing text for that + config lives on the enum, not at the declaration. Without this the catalog + would report "no documentation" for exactly the configs whose valid values + matter most. + """ + documented = {} + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + continue + if "@EnumDescription" not in original: + continue + masked = strip_comments_and_strings(original, keep_string_bodies=True) + for match in ENUM_DESCRIPTION_RE.finditer(masked): + args, after = extract_call_args(masked, match.end() - 1) + if args is None: + continue + enum_match = ENUM_DECL_RE.search(masked, after) + if not enum_match: + continue + name = enum_match.group(1) + body_start = masked.find("{", enum_match.end()) + if body_start < 0: + continue + depth = 0 + body_end = len(masked) + for i in range(body_start, len(masked)): + if masked[i] == "{": + depth += 1 + elif masked[i] == "}": + depth -= 1 + if depth == 0: + body_end = i + break + values = [] + for field in ENUM_FIELD_DESCRIPTION_RE.finditer(masked, body_start, body_end): + field_args, field_after = extract_call_args(masked, field.end() - 1) + if field_args is None: + continue + tail = masked[field_after:field_after + 160] + constant = ENUM_CONSTANT_RE.match(tail) + if not constant: + continue + values.append(OrderedDict([ + ("value", constant.group(1)), + ("description", concatenated_string_literal(field_args) or ""), + ])) + documented[name] = OrderedDict([ + ("enum", name), + ("file", os.path.relpath(file_path, repo_root)), + ("description", concatenated_string_literal(args) or ""), + ("values", values), + ]) + return documented + + +STRING_CONSTANT_RE = re.compile( + r"\bstatic\s+final\s+String\s+(?P<name>[A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*(?P<value>[^;]*);") + + +def collect_string_constants(repo_root): + """Pass 0. (class, constant) -> its initializer expression, for key prefixes. + + Keyed both by owning class and by bare name. A bare name claimed by two + different expressions is dropped, so an ambiguous prefix never silently + resolves to the wrong value. + """ + by_qualified = {} + bare = {} + bare_conflicts = set() + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + continue + if "static final String" not in original: + continue + masked = strip_comments_and_strings(original, keep_string_bodies=True) + for match in STRING_CONSTANT_RE.finditer(masked): + name = match.group("name") + value = match.group("value").strip() + if not value or len(value) > 400: + continue + owner = find_enclosing_class(masked, match.start()) + by_qualified[(owner, name)] = value + if name in bare and bare[name] != value: + bare_conflicts.add(name) + bare[name] = value + for name in bare_conflicts: + bare.pop(name, None) + constants = {(None, name): value for name, value in bare.items()} + constants.update(by_qualified) + return constants + + +def literal_or_expression(expr): + """Return (value, is_literal) for a default-value expression.""" + literal = concatenated_string_literal(expr) + if literal is not None: + return literal, True + collapsed = " ".join(expr.split()) + if re.fullmatch(r"(true|false)", collapsed): + return collapsed, True + if re.fullmatch(r"-?\d+[LlFfDd]?", collapsed): + return collapsed.rstrip("LlFfDd"), True + if re.fullmatch(r"-?\d*\.\d+[FfDd]?", collapsed): + return collapsed.rstrip("FfDd"), True + return collapsed, False + + +# -------------------------------------------------------------------------- +# Pass 1 -- config declarations +# -------------------------------------------------------------------------- + +# Matches both `ConfigProperty<String> FOO = ConfigProperty` and the Flink +# `ConfigOption<String> FOO = ConfigOptions` forms, plus the rarer single-line +# `= ConfigProperty.key(...)`. +DECLARATION_RE = re.compile( + r"(?P<decl>(?:public|protected|private)?\s*(?:static\s+)?(?:final\s+)?" + r"(?P<holder>ConfigProperty|ConfigOption)\s*<\s*(?P<type>[^>]*(?:<[^>]*>)?[^>]*)\s*>\s+" + r"(?P<name>[A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*)" + r"(?P<builder>ConfigProperty|ConfigOptions)\b" +) + +CLASS_DECL_RE = re.compile( + r"\b(?:public|protected|private)?\s*(?:static\s+)?(?:final\s+)?(?:abstract\s+)?" + r"(?:class|interface|enum)\s+([A-Za-z_$][A-Za-z0-9_$]*)" +) + +# Builder steps we care about, applied to the chain text after the key. +CHAIN_STEP_RE = re.compile(r"\.\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*\(") + + +def find_enclosing_class(text, index): + """Innermost-looking class name declared before ``index``.""" + best = None + for match in CLASS_DECL_RE.finditer(text, 0, index): + best = match.group(1) + return best + + +def parse_chain_steps(chain_text): + """Return an ordered list of (method_name, args_text) from a builder chain. + + Comments are blanked first -- authors do put explanatory comments between a + builder call's arguments, and those must not end up inside an extracted + value. String bodies are kept so literals survive. + """ + chain_text = strip_comments_and_strings(chain_text, keep_string_bodies=True) + steps = [] + pos = 0 + while True: + match = CHAIN_STEP_RE.search(chain_text, pos) + if not match: + break + args, after = extract_call_args(chain_text, match.end() - 1) + if args is None: + pos = match.end() + continue + steps.append((match.group(1), args)) + pos = after + return steps + + +DERIVED_KEY_RE = re.compile( + r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*key\(\s*\)\s*(?:\+\s*(.+))?") + + +def derived_from_config_key(expr, string_constants, owner_class, base_key_resolver): + """Resolve `OTHER_CONFIG.key()` and `OTHER_CONFIG.key() + ".suffix"`. + + Several configs name their own key, and several name an alternative key, + relative to another config's. Returns None when the base config is unknown. + """ + match = DERIVED_KEY_RE.fullmatch(" ".join(expr.split())) + if not match: + return None + base = base_key_resolver(match.group(1)) + if base is None: + return None + if match.group(2) is None: + return base + suffix = concatenated_string_literal(match.group(2), string_constants, owner_class) + return None if suffix is None else base + suffix + + +def parse_declaration(masked_for_scan, original, match, file_path, repo_root, + string_constants, base_key_resolver=lambda path: None, + enum_docs=None): + """Build one catalog entry from a declaration match. Never raises.""" + warnings = [] + start = match.start() + end = find_statement_end(original, start) + if end < 0: + end = min(len(original), start + 4000) + warnings.append("statement terminator not found; chain truncated at 4000 chars") + chain_text = original[start:end] + + steps = parse_chain_steps(chain_text) + step_names = [name for name, _ in steps] + + enclosing_class = find_enclosing_class(masked_for_scan, start) + + key = None + key_expr = None + key_resolution = None + for name, args in steps: + if name == "key": + key_expr = args.strip() + key = concatenated_string_literal(args) + if key is not None: + key_resolution = "literal" + else: + key = concatenated_string_literal( + args, string_constants, enclosing_class) + if key is not None: + key_resolution = "resolvedFromConstants" + break + + if key is None and key_expr: + # Flink re-exports read the key off another ConfigProperty, and some + # Java configs build the key from a shared prefix constant. + reference = re.fullmatch( + r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*key\(\s*\)", key_expr.strip()) + if reference: + return None, "alias:" + reference.group(1) + # `OTHER_CONFIG.key() + ".suffix"` -- a key derived from another config's. + key = derived_from_config_key( + key_expr, string_constants, enclosing_class, base_key_resolver) + if key is not None: + key_resolution = "derivedFromConfigKey" + else: + warnings.append( + "key is a computed expression: " + " ".join(key_expr.split())[:160]) + + entry = OrderedDict() + entry["key"] = key + entry["keyResolution"] = key_resolution + entry["keyExpression"] = (" ".join(key_expr.split()) + if key_expr and key_resolution != "literal" else None) + entry["type"] = " ".join(match.group("type").split()) + entry["declaredIn"] = OrderedDict([ + ("class", enclosing_class), + ("constant", match.group("name")), + ("file", os.path.relpath(file_path, repo_root)), + ("line", line_of(original, start)), + ]) + entry["configGroup"] = entry["declaredIn"]["class"] + entry["builderStyle"] = "ConfigProperty" if match.group("builder") == "ConfigProperty" else "FlinkConfigOptions" + + # Default value. + default_value = None + default_is_literal = False + has_default = None + for name, args in steps: + if name == "defaultValue": + parts = split_top_level_args(args) + if parts: + default_value, default_is_literal = literal_or_expression(parts[0]) + has_default = True + if len(parts) > 1: + doc_on_default = concatenated_string_literal(parts[1]) + if doc_on_default: + entry["docOnDefaultValue"] = doc_on_default + break + if name == "noDefaultValue": + has_default = False + break + if has_default is None: + has_default = False + if entry["builderStyle"] == "ConfigProperty": + warnings.append("neither defaultValue() nor noDefaultValue() found in chain") + entry["hasDefaultValue"] = has_default + entry["defaultValue"] = default_value + entry["defaultValueIsLiteral"] = default_is_literal + if has_default and not default_is_literal: + entry["defaultValueNote"] = "computed at class-init time; shown as the source expression" + + # Documentation: a string, or a class reference whose text lives elsewhere. + documentation = None + documentation_source = None + enum_documentation = None + for name, args in steps: + if name not in ("withDocumentation", "withDescription"): + continue + parts = split_top_level_args(args) + if not parts: + continue + class_ref = re.fullmatch(r"([A-Za-z_$][A-Za-z0-9_$.]*)\s*\.\s*class", parts[0].strip()) + if class_ref: + enum_name = class_ref.group(1).split(".")[-1] + documentation_source = "enumClass:" + enum_name + extra = concatenated_string_literal(parts[1]) if len(parts) > 1 else None + described = (enum_docs or {}).get(enum_name) + if described: + entry_enum = described + pieces = [p for p in (extra, described["description"]) if p] + documentation = "\n".join(pieces) + enum_documentation = entry_enum + else: + documentation = extra + warnings.append( + "documentation points at enum %s, whose @EnumDescription was not found" + % enum_name) + else: + documentation = concatenated_string_literal(parts[0]) + documentation_source = "literal" + if documentation is None: + documentation = concatenated_string_literal( + parts[0], string_constants, enclosing_class) + documentation_source = "resolvedFromConstants" + if documentation is None: + documentation_source = None + warnings.append("documentation is a computed expression") + break + entry["documentation"] = documentation + entry["documentationSource"] = documentation_source + entry["enumDocumentation"] = enum_documentation + + def collect_string_args(step_name): + for name, args in steps: + if name == step_name: + values = [] + for part in split_top_level_args(args): + literal = concatenated_string_literal(part) + if literal is None: + literal = concatenated_string_literal( + part, string_constants, enclosing_class) + if literal is None: + literal = derived_from_config_key( + part, string_constants, enclosing_class, base_key_resolver) + values.append(literal if literal is not None + else " ".join(part.split())) + return values + return [] + + entry["validValues"] = collect_string_args("withValidValues") + # A config whose documentation is an enum class declares its permitted values on the enum + # rather than via withValidValues(...). Fall back to the enum's constants so that + # "what can I set this to?" is answerable for strategy/policy configs, which are exactly + # the ones where it matters most. + if not entry["validValues"] and enum_documentation: + entry["validValues"] = [v["value"] for v in enum_documentation.get("values", []) if v.get("value")] + entry["alternatives"] = collect_string_args("withAlternatives") + since = collect_string_args("sinceVersion") + entry["sinceVersion"] = since[0] if since else None + deprecated = collect_string_args("deprecatedAfter") + entry["deprecatedAfter"] = deprecated[0] if deprecated else None + entry["supportedVersions"] = collect_string_args("supportedVersions") + entry["advanced"] = "markAdvanced" in step_names + entry["hasInferFunction"] = "withInferFunction" in step_names + entry["builderSteps"] = step_names + + if key is None: + warnings.append("key could not be resolved to a string literal") + if documentation is None and documentation_source != "enumClass": + warnings.append("no documentation text resolved") + + entry["parseWarnings"] = warnings + return entry, None + + +def collect_declarations(repo_root, verbose=False): + """Pass 1. Returns (entries, constant_index, stats).""" + entries = [] + # (file-local class, constant name) -> config key, so later passes can map a + # constant reference back to the config it names. + constant_index = {} + alias_pending = [] + stats = defaultdict(int) + + string_constants = collect_string_constants(repo_root) + stats["stringConstants"] = len(string_constants) + enum_docs = collect_enum_documentation(repo_root) + stats["documentedEnums"] = len(enum_docs) + if verbose: + print(" string constants indexed: %d" % len(string_constants), file=sys.stderr) + print(" documented enums indexed: %d" % len(enum_docs), file=sys.stderr) + + for file_path in walk_source_files(repo_root, (".java",)): + if SOURCE_DIR_MARKER not in file_path: + continue + try: + with open(file_path, "r", encoding="utf-8", errors="replace") as handle: + original = handle.read() + except OSError: + stats["unreadableFiles"] += 1 + continue + if "ConfigProperty" not in original and "ConfigOptions" not in original: + continue + + masked = strip_comments_and_strings(original) + + def resolve_base_key(path, _index=constant_index): + parts = path.split(".") + const = parts[-1] + qualifier = parts[-2] if len(parts) >= 2 else None + return _index.get((qualifier, const)) or _index.get((None, const)) + + for match in DECLARATION_RE.finditer(masked): + stats["declarationsSeen"] += 1 + entry, alias_target = parse_declaration( + masked, original, match, file_path, repo_root, string_constants, + resolve_base_key, enum_docs) + if alias_target is not None: + alias_pending.append(( + find_enclosing_class(masked, match.start()), + match.group("name"), + alias_target.split(":", 1)[1], + os.path.relpath(file_path, repo_root), + line_of(original, match.start()), + )) + continue + if entry is None: + stats["unparseable"] += 1 + continue + # Flink ConfigOptions that do not name a hoodie.* key belong to the + # Flink option namespace, not the Hudi config namespace. + if entry["builderStyle"] == "FlinkConfigOptions" and not ( + entry["key"] or "").startswith("hoodie."): + stats["flinkOptionsSkipped"] += 1 + continue + entries.append(entry) + owner = entry["declaredIn"]["class"] + if entry["key"]: + constant_index[(owner, entry["declaredIn"]["constant"])] = entry["key"] + constant_index[(None, entry["declaredIn"]["constant"])] = entry["key"] + + # Resolve re-export aliases (`FOO = OtherHolder.FOO`) to the real key so a + # read site that uses the alias still lands on the right config. + aliases = [] + for owner, name, target, rel_file, line in alias_pending: + target_parts = target.split(".") + target_const = target_parts[-1] + target_class = target_parts[-2] if len(target_parts) >= 2 else None + key = constant_index.get((target_class, target_const)) or \ + constant_index.get((None, target_const)) + if key: + constant_index[(owner, name)] = key + aliases.append(OrderedDict([ + ("key", key), ("aliasClass", owner), ("aliasConstant", name), + ("file", rel_file), ("line", line)])) + stats["aliasesResolved"] += 1 + else: + stats["aliasesUnresolved"] += 1 + + return entries, constant_index, aliases, stats + + +# -------------------------------------------------------------------------- +# Pass 2 -- accessor methods +# -------------------------------------------------------------------------- + +# Methods and constructors, including package-private ones. The modifier is +# optional, so NON_METHOD_KEYWORDS below keeps `if (...) {` and friends out. +METHOD_SIGNATURE_RE = re.compile( + r"(?:(?:public|protected|private)\s+)?(?:static\s+|final\s+|synchronized\s+|abstract\s+)*" + r"(?:(?P<ret>[A-Za-z_$][A-Za-z0-9_$<>,.\[\]\s?]*?)\s+)?" + r"(?P<name>[A-Za-z_$][A-Za-z0-9_$]*)\s*\((?P<params>[^;{()]*(?:\([^()]*\)[^;{()]*)*)\)\s*" + r"(?:throws\s+[A-Za-z0-9_$.,\s]+)?\{" +) + +NON_METHOD_KEYWORDS = frozenset(( + "if", "else", "for", "while", "switch", "catch", "try", "do", "synchronized", + "return", "new", "case", "assert", "super", "this", +)) + +GETTER_CALL_RE = re.compile( + r"\bget(?:String|Int|Integer|Long|Boolean|Double|Float|StringOrDefault|" + r"IntOrDefault|BooleanOrDefault|LongOrDefault|Enum|Class)?\s*\(\s*" + r"(?P<arg>[A-Za-z_$][A-Za-z0-9_$.]*)\s*[,)]" +) + +ALT_KEYS_CALL_RE = re.compile( + r"\b(?:get|contains)(?:String|Int|Integer|Long|Boolean|Double|Raw)?WithAltKeys\s*\([^)]*?" + r"\b(?P<arg>[A-Za-z_$][A-Za-z0-9_$.]*)\s*[,)]" +) + + +def find_method_bodies(masked, original): + """Yield (name, body_start, body_end, signature_line) for each method. + + ``body_start`` indexes the ``{``; ``body_end`` the matching ``}``. + """ + for match in METHOD_SIGNATURE_RE.finditer(masked): + if match.group("name") in NON_METHOD_KEYWORDS: + continue + if (match.group("ret") or "").split()[-1:] and \ + (match.group("ret") or "").split()[-1] in NON_METHOD_KEYWORDS: + continue + brace_index = masked.find("{", match.end() - 1) + if brace_index < 0: + continue + depth = 0 + end = -1 + for i in range(brace_index, len(masked)): + if masked[i] == "{": + depth += 1 + elif masked[i] == "}": + depth -= 1 + if depth == 0: + end = i + break + if end < 0: + continue + yield match.group("name"), brace_index, end, line_of(original, match.start()) + + +def resolve_constant(expr, owner_class, constant_index): + """Map a (possibly qualified) constant reference to a config key.""" + parts = expr.split(".") + const = parts[-1] + qualifier = parts[-2] if len(parts) >= 2 else None + return (constant_index.get((qualifier, const)) + or (constant_index.get((owner_class, const)) if qualifier is None else None) + or constant_index.get((None, const))) Review Comment: Fixed — the lookup order was indeed the bug, and it was worse than the example you found. `resolve_constant` now tries the owning class before the ambiguous bare-name bucket. The same ordering bug had `HoodieIndexingConfig.getIndexType` filed under `hoodie.index.type`, when `HoodieIndexingConfig.INDEX_TYPE` is actually `hoodie.expression.index.type` — two classes, same constant name, different keys. While in here I also found `HoodieIndexConfig.Builder.build()` had been registered as an accessor, so **every** `.build()` call in the tree resolved to `hoodie.index.type` — all twenty of its read sites were fabricated (`AvroParquetReader.builder().build()`, writer constructors, `TimelineInspector`). Generic method names are now refused outright. `hoodie.metadata.enable` resolves `isEnabled` and `isMetadataTableEnabled` correctly and has its own read sites back. -- 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]
