Copilot commented on code in PR #6592:
URL: https://github.com/apache/hive/pull/6592#discussion_r3553449798
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/properties/PropertyStore.java:
##########
@@ -84,7 +84,7 @@ public abstract class PropertyStore {
/**
* Persists an iterator property map.
- * <p>May be useful to override to use one transaction.</p>
+ * <p>Maybe useful to override to use one transaction.</p>
Review Comment:
Javadoc grammar regression: “Maybe useful to override…” reads like “maybe”
instead of “may be”.
##########
PROPERTIES_PACKAGE.md:
##########
@@ -0,0 +1,169 @@
+# Hive Metastore Properties Package
+
+## Overview
+
+The `properties` package provides a **type-safe, schema-driven property
management system** for Hive metastore objects (clusters, databases, tables).
It allows you to declare, validate, and persist custom properties with
compile-time type checking and runtime serialization support.
+
+## Core Components
+
+### 1. **PropertySchema** (`PropertySchema.java`)
+Defines the blueprint for what properties can be set on a metastore object.
+
+**Key responsibilities:**
+- Declares allowed properties and their types (e.g., STRING, INTEGER, BOOLEAN,
DATE, etc.)
+- Manages default values for properties
+- Tracks schema version (incremented when properties are added/removed)
+- Generates a digest (UUID) for schema identity/caching
+- Supports thread-safe serialization/deserialization
+
+**Main methods:**
+- `declareProperty(name, type, defaultValue)` - Register a new property in the
schema
+- `getPropertyType(name)` - Retrieve the declared type
+- `getDefaultValue(name)` - Get the default value for a property
+- `removeProperty(name)` - Remove a property (testing/cleanup)
+- `getDigest()` - Get a stable UUID representing the schema state
+
+**Example use case:**
+```java
+PropertySchema tableSchema = new PropertySchema("table-schema", 1, new
TreeMap<>());
+tableSchema.declareProperty("owner", PropertyType.STRING, "system");
+tableSchema.declareProperty("created_date", PropertyType.DATETIME);
+tableSchema.declareProperty("tags", PropertyType.JSON);
+```
+
+### 2. **PropertyType** (`PropertyType.java`)
+An abstract type system for property values with parsing, formatting, and
serialization.
+
+**Supported types:**
+- `STRING` - Text values
+- `BOOLEAN` - true/false values
+- `INTEGER` - 32-bit integers
+- `LONG` - 64-bit integers
+- `DOUBLE` - Floating-point values
+- `DATETIME` - ISO8601 dates (UTC timezone)
+- `JSON` - Complex nested objects (Gson-backed)
+
+**Key methods:**
+- `cast(value)` - Type-coerce a value to this type
+- `parse(str)` - Parse from string (deserialization)
+- `format(value)` - Convert to string (serialization)
+- `read(DataInput)` / `write(DataOutput)` - Binary I/O
+- `register(type)` - Register custom property types
+
+**Type safety:**
+Each type handles conversion, null-safety, and format consistency. For
example, the DATETIME type always uses ISO8601 format with UTC timezone,
ensuring cross-system consistency.
+
+### 3. **PropertyMap** (`PropertyMap.java`)
+Holds the actual property values for a metastore object, validated against a
schema.
+
+**Key features:**
+- **Copy-on-write**: Dirty flag prevents unnecessary copies; shared content
until modified
+- **Thread-safe**: Isolated from concurrent modifications
+- **Schema-bound**: All properties must match the associated PropertySchema
+- **Serializable**: Can be persisted and restored with full type information
+
+**Purpose:**
+While PropertySchema defines *what properties are allowed*, PropertyMap holds
the *actual values* for a specific object instance.
+
+### 4. **PropertyManager** (`PropertyManager.java`)
+High-level manager orchestrating property operations at the session level.
+
+**Responsibilities:**
+- Manages multiple property schemas in a unified namespace
+- Handles transactional property reads and writes
+- Tracks dirty maps to batch updates and reduce persistence store hits
+- Integrates with JEXL for dynamic property expressions
+- Coordinates with PropertyStore for persistence
+
+**Key design:**
+- One PropertyManager instance per session
+- All PropertyMaps managed by the same manager must use one of its known
schemas
+- Avoids writing the entire map to the store on each property change
+
+### 5. **PropertyStore** (`PropertyStore.java`)
+Persistence layer for storing/retrieving properties from the metastore.
+
+**Responsibilities:**
+- Load property maps from the database
+- Save property maps to the database
+- Manage property schema persistence
+- Can be wrapped by `CachingPropertyStore` for performance
+
+### 6. Supporting Classes
+
+- **Digester** - Generates stable UUID digests for schema/map identity
+- **SerializationProxy** - Custom serialization to handle transient fields
safely
+- **SoftCache** - Soft reference cache for property schemas (GC-friendly)
+- **PropertyException** - Exception type for property-related errors
+
+## Workflow Example
+
+```java
+// 1. Define a schema (typically done at startup)
+PropertySchema tableSchema = new PropertySchema("table-schema", 1, new
TreeMap<>());
+tableSchema.declareProperty("owner", PropertyType.STRING, "admin");
+tableSchema.declareProperty("is_external", PropertyType.BOOLEAN, false);
+tableSchema.declareProperty("created_time", PropertyType.DATETIME);
+
+// 2. Create a property manager
+PropertyManager manager = new PropertyManager("my-namespace");
+
+// 3. Load or create a property map for a table
+PropertyMap tableProps = manager.newPropertyMap(tableSchema);
+
+// 4. Set values
+tableProps.put("owner", "alice");
+tableProps.put("is_external", true);
+
+// 5. Persist
+manager.persistProperties(tableProps);
+
+// 6. Later, retrieve and use
+PropertyMap loaded = manager.loadProperties(objectId);
+String owner = (String) loaded.get("owner");
+```
+
+## Design Patterns
+
+### Thread Safety
+- Schemas use `AtomicInteger` for version counters
+- PropertyMaps use copy-on-write with dirty flags
+- Digest computation is double-checked locked (lazy initialization)
+
+### Serialization
+- Custom `SerializationProxy` handles transient field safety
+- Transient fields prevent accidental Java serialization of internal state
+- Both XML (DataInput/DataOutput) and Java object serialization supported
Review Comment:
`DataInput`/`DataOutput` are binary stream APIs, not XML. Calling this “XML”
is misleading for readers trying to understand the serialization format.
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/properties/HMSPropertyManager.java:
##########
@@ -45,14 +51,13 @@ public class HMSPropertyManager extends PropertyManager {
/** Table maintenance operation type. */
public enum MaintenanceOpType {
- COMPACTION,
- SNAPSHOT_EXPIRY,
- STATS_REBUILD,
- MV_BUILD,
- MV_REFRESH,
- SHUFFLE_TO_NEW_PART,
- RECOMPRESS,
- REORG
+ EXPIRE_SNAPSHOT,
+ REWRITE_DATA_FILES,
+ COMPRESS_DATA_FILES,
+ REWRITE_MANIFEST,
Review Comment:
The PR description says there is no user-facing change, but
`HMSPropertyManager` exposes public enums
(`MaintenanceOpType`/`MaintenanceOpStatus`) whose constants were
renamed/removed. That is a source-incompatible change for any downstream code
referencing the old names; consider keeping deprecated aliases (or otherwise
preserving compatibility) and/or updating the PR description/release notes to
reflect the breaking API change.
##########
PROPERTIES_PACKAGE.md:
##########
@@ -0,0 +1,169 @@
+# Hive Metastore Properties Package
+
+## Overview
+
+The `properties` package provides a **type-safe, schema-driven property
management system** for Hive metastore objects (clusters, databases, tables).
It allows you to declare, validate, and persist custom properties with
compile-time type checking and runtime serialization support.
+
+## Core Components
+
+### 1. **PropertySchema** (`PropertySchema.java`)
+Defines the blueprint for what properties can be set on a metastore object.
+
+**Key responsibilities:**
+- Declares allowed properties and their types (e.g., STRING, INTEGER, BOOLEAN,
DATE, etc.)
+- Manages default values for properties
+- Tracks schema version (incremented when properties are added/removed)
+- Generates a digest (UUID) for schema identity/caching
+- Supports thread-safe serialization/deserialization
+
+**Main methods:**
+- `declareProperty(name, type, defaultValue)` - Register a new property in the
schema
+- `getPropertyType(name)` - Retrieve the declared type
+- `getDefaultValue(name)` - Get the default value for a property
+- `removeProperty(name)` - Remove a property (testing/cleanup)
+- `getDigest()` - Get a stable UUID representing the schema state
+
+**Example use case:**
+```java
+PropertySchema tableSchema = new PropertySchema("table-schema", 1, new
TreeMap<>());
+tableSchema.declareProperty("owner", PropertyType.STRING, "system");
+tableSchema.declareProperty("created_date", PropertyType.DATETIME);
+tableSchema.declareProperty("tags", PropertyType.JSON);
+```
+
+### 2. **PropertyType** (`PropertyType.java`)
+An abstract type system for property values with parsing, formatting, and
serialization.
+
+**Supported types:**
+- `STRING` - Text values
+- `BOOLEAN` - true/false values
+- `INTEGER` - 32-bit integers
+- `LONG` - 64-bit integers
+- `DOUBLE` - Floating-point values
+- `DATETIME` - ISO8601 dates (UTC timezone)
+- `JSON` - Complex nested objects (Gson-backed)
+
+**Key methods:**
+- `cast(value)` - Type-coerce a value to this type
+- `parse(str)` - Parse from string (deserialization)
+- `format(value)` - Convert to string (serialization)
+- `read(DataInput)` / `write(DataOutput)` - Binary I/O
+- `register(type)` - Register custom property types
+
+**Type safety:**
+Each type handles conversion, null-safety, and format consistency. For
example, the DATETIME type always uses ISO8601 format with UTC timezone,
ensuring cross-system consistency.
+
+### 3. **PropertyMap** (`PropertyMap.java`)
+Holds the actual property values for a metastore object, validated against a
schema.
+
+**Key features:**
+- **Copy-on-write**: Dirty flag prevents unnecessary copies; shared content
until modified
+- **Thread-safe**: Isolated from concurrent modifications
+- **Schema-bound**: All properties must match the associated PropertySchema
+- **Serializable**: Can be persisted and restored with full type information
+
+**Purpose:**
+While PropertySchema defines *what properties are allowed*, PropertyMap holds
the *actual values* for a specific object instance.
+
+### 4. **PropertyManager** (`PropertyManager.java`)
+High-level manager orchestrating property operations at the session level.
+
+**Responsibilities:**
+- Manages multiple property schemas in a unified namespace
+- Handles transactional property reads and writes
+- Tracks dirty maps to batch updates and reduce persistence store hits
+- Integrates with JEXL for dynamic property expressions
+- Coordinates with PropertyStore for persistence
+
+**Key design:**
+- One PropertyManager instance per session
+- All PropertyMaps managed by the same manager must use one of its known
schemas
+- Avoids writing the entire map to the store on each property change
+
+### 5. **PropertyStore** (`PropertyStore.java`)
+Persistence layer for storing/retrieving properties from the metastore.
+
+**Responsibilities:**
+- Load property maps from the database
+- Save property maps to the database
+- Manage property schema persistence
+- Can be wrapped by `CachingPropertyStore` for performance
+
+### 6. Supporting Classes
+
+- **Digester** - Generates stable UUID digests for schema/map identity
+- **SerializationProxy** - Custom serialization to handle transient fields
safely
+- **SoftCache** - Soft reference cache for property schemas (GC-friendly)
+- **PropertyException** - Exception type for property-related errors
+
+## Workflow Example
+
+```java
+// 1. Define a schema (typically done at startup)
+PropertySchema tableSchema = new PropertySchema("table-schema", 1, new
TreeMap<>());
+tableSchema.declareProperty("owner", PropertyType.STRING, "admin");
+tableSchema.declareProperty("is_external", PropertyType.BOOLEAN, false);
+tableSchema.declareProperty("created_time", PropertyType.DATETIME);
+
+// 2. Create a property manager
+PropertyManager manager = new PropertyManager("my-namespace");
+
+// 3. Load or create a property map for a table
+PropertyMap tableProps = manager.newPropertyMap(tableSchema);
+
+// 4. Set values
+tableProps.put("owner", "alice");
+tableProps.put("is_external", true);
+
+// 5. Persist
+manager.persistProperties(tableProps);
+
+// 6. Later, retrieve and use
+PropertyMap loaded = manager.loadProperties(objectId);
+String owner = (String) loaded.get("owner");
+```
Review Comment:
The workflow example won’t compile as written: `PropertyManager` is abstract
(no public constructor) and APIs like `newPropertyMap`, `persistProperties`,
and `loadProperties` don’t exist. Updating this to use
`PropertyManager.create(...)`, `setProperty(...)`, and `commit()` would make
the documentation actionable.
##########
PROPERTIES_PACKAGE.md:
##########
@@ -0,0 +1,169 @@
+# Hive Metastore Properties Package
+
+## Overview
+
+The `properties` package provides a **type-safe, schema-driven property
management system** for Hive metastore objects (clusters, databases, tables).
It allows you to declare, validate, and persist custom properties with
compile-time type checking and runtime serialization support.
+
+## Core Components
+
+### 1. **PropertySchema** (`PropertySchema.java`)
+Defines the blueprint for what properties can be set on a metastore object.
+
+**Key responsibilities:**
+- Declares allowed properties and their types (e.g., STRING, INTEGER, BOOLEAN,
DATE, etc.)
+- Manages default values for properties
+- Tracks schema version (incremented when properties are added/removed)
+- Generates a digest (UUID) for schema identity/caching
+- Supports thread-safe serialization/deserialization
+
+**Main methods:**
+- `declareProperty(name, type, defaultValue)` - Register a new property in the
schema
+- `getPropertyType(name)` - Retrieve the declared type
+- `getDefaultValue(name)` - Get the default value for a property
+- `removeProperty(name)` - Remove a property (testing/cleanup)
+- `getDigest()` - Get a stable UUID representing the schema state
+
+**Example use case:**
+```java
+PropertySchema tableSchema = new PropertySchema("table-schema", 1, new
TreeMap<>());
+tableSchema.declareProperty("owner", PropertyType.STRING, "system");
+tableSchema.declareProperty("created_date", PropertyType.DATETIME);
+tableSchema.declareProperty("tags", PropertyType.JSON);
Review Comment:
This example uses `new PropertySchema("table-schema", 1, new TreeMap<>())`,
but that constructor is package-private in `PropertySchema`; as written, the
snippet won’t compile for typical callers. Consider showing the supported
pattern (declaring schema properties via a manager such as
`HMSPropertyManager`).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
