[#10878] feat(core): add relational storage for logical views with versioning#10879
[#10878] feat(core): add relational storage for logical views with versioning#10879jerryshao merged 10 commits intoapache:mainfrom
Conversation
00086c5 to
894ad76
Compare
Code Coverage Report
Files
|
894ad76 to
b91803d
Compare
Introduce the storage layer for the Logical View Management feature. Each view has a versioned history tracked in a new view_version_info table; view_meta is extended with audit_info and points to the current version. Snapshots include columns, properties, default catalog/schema and multi-dialect SQL representations serialized as JSON. Main changes: * PO/Mapper: ViewVersionInfoPO + mapper with base/PostgreSQL SQL providers; ViewPO and ViewMetaMapper providers extended with audit_info and default_catalog/default_schema. * Service: ViewMetaService rewritten for full CRUD with version management and JSON representation serialization. * meta: new ViewEntity implementing View and HasIdentifier. * JDBCBackend routes ViewEntity inserts through ViewMetaService; CURRENT_SCRIPT_VERSION bumped to 1.3.0. * Minimal compilation stubs added in ViewOperationDispatcher and test doubles for the expanded ViewCatalog interface (full implementation lands in Phase 3). The relational scripts (apache#10860) and view API surface (apache#10830) are already on main; this change builds on them. Unit tests cover insert, load, update (with new version row), soft delete, list-by-namespace and multi-representation round-trips. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b91803d to
6b6207d
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a relational persistence layer for Logical Views with versioned snapshots, integrating it into the JDBC backend and updating catalog/view dispatch/testing to use the new ViewEntity + version tables.
Changes:
- Introduces
ViewEntityandview_version_infopersistence (PO/Mapper/SQL providers) and joins current version when reading views. - Refactors
ViewMetaServiceto CRUD views with version creation, JSON (de)serialization, and soft-delete cleanup for both meta and versions. - Updates dispatcher/backend/tests to store/import
ViewEntityand aligncreateViewtest signatures with main API.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/org/apache/gravitino/meta/ViewEntity.java | Adds a concrete view metadata entity implementing View for storage and import. |
| core/src/main/java/org/apache/gravitino/storage/relational/service/ViewMetaService.java | Reworks view persistence to read/write version rows and handle deletes across meta + versions. |
| core/src/main/java/org/apache/gravitino/storage/relational/po/ViewPO.java | Adds JSON (de)serialization and conversion helpers between PO and ViewEntity. |
| core/src/main/java/org/apache/gravitino/storage/relational/po/ViewVersionInfoPO.java | Introduces PO for version snapshot rows with required-field validation. |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/ViewVersionInfoMapper.java | Adds MyBatis mapper for view version info table operations. |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/ViewVersionInfoSQLProviderFactory.java | Adds provider factory for dialect-specific SQL generation for version info. |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/ViewVersionInfoBaseSQLProvider.java | Adds base SQL provider implementations for version info (MySQL/H2 style). |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/ViewVersionInfoPostgreSQLProvider.java | Adds PostgreSQL-specific SQL for upserts and limited deletes for version info. |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/ViewMetaMapper.java | Updates result mapping to include joined current version info data. |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/ViewMetaBaseSQLProvider.java | Updates view meta queries to join current version info and include audit fields. |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/ViewMetaPostgreSQLProvider.java | PostgreSQL overrides for joined view+version selects and audit fields. |
| core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/DefaultMapperPackageProvider.java | Registers the new ViewVersionInfoMapper. |
| core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java | Routes ViewEntity inserts through ViewMetaService. |
| core/src/main/java/org/apache/gravitino/catalog/ViewOperationDispatcher.java | Imports catalog views into entity store as ViewEntity including full view definition. |
| core/src/main/java/org/apache/gravitino/catalog/EntityCombinedView.java | Switches combined view wrapper to use ViewEntity instead of GenericEntity. |
| core/src/test/java/org/apache/gravitino/storage/relational/service/TestViewMetaService.java | Adds/updates unit tests for CRUD + versioning + soft-delete lifecycle. |
| core/src/test/java/org/apache/gravitino/catalog/TestViewOperationDispatcher.java | Updates tests to use ViewEntity and include a non-empty representation. |
| core/src/test/java/org/apache/gravitino/connector/TestCatalogOperations.java | Updates test connector to satisfy new ViewCatalog methods. |
| common/src/main/java/org/apache/gravitino/config/ConfigConstants.java | Bumps CURRENT_SCRIPT_VERSION to 1.3.0. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replay the local-vs-remote diff as one standalone commit on top of the current PR head so it can be pushed without force. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace GenericEntity(VIEW) write/update paths with ViewEntity, and remove GenericEntity fallback insertion for VIEW in relational backend. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge origin/main into the PR branch and resolve conflict blocks while keeping the ViewEntity-based write path changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| } | ||
| } | ||
|
|
||
| private ViewEntity buildViewEntityForImport(Long uid, NameIdentifier ident, View view) { |
There was a problem hiding this comment.
What's the meaning of buildViewEntityForImport?
There was a problem hiding this comment.
buildViewEntityForImport is the materialization step in the loadView auto-import path: it converts a catalog View into a persisted ViewEntity with Gravitino-generated id/identifier.
It is similar in purpose to table import, but currently lighter than importTable (no string-identifier reconciliation/rename correction path, and import here is best-effort).
Given ViewOperationDispatcher currently only supports loadView + auto-import, we expect to clean up/refactor this helper when full view CRUD lands in follow-up PRs.
| LOG.info("Successfully imported view {} into entity store with id {}", ident, uid); | ||
| return EntityCombinedView.of(entityCombinedView.viewFromCatalog(), newViewEntity) | ||
| .withImported(true); | ||
| return EntityCombinedView.of(catalogView, newViewEntity).withImported(true); |
There was a problem hiding this comment.
If we import the view from external source to Gravitino, shall we still need to combine them, I think they're the same, am I right?
There was a problem hiding this comment.
Different catalogs support views to varying degrees, so combining them with viewEntity is necessary. Specific handling will be added in subsequent PRs.
| @Getter | ||
| @EqualsAndHashCode | ||
| @ToString | ||
| public class ViewPO { |
There was a problem hiding this comment.
Why don't you use lombok to simplify the code?
Handle zero-row optimistic update in ViewMetaService.updateView by failing the transaction and surfacing IOException, preventing orphaned view_version_info rows. Also add a regression test that deletes the view during updater execution to verify no extra version row is persisted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace manual getters/equals/hashCode/builder in ViewPO with Lombok while preserving equality semantics by excluding viewVersionInfoPO. Update ViewMetaService to use Lombok-generated ViewPOBuilder and add TestViewPO to verify builder behavior and equals/hashCode compatibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| m.put("type", Representation.TYPE_SQL); | ||
| m.put("dialect", sql.dialect()); | ||
| m.put("sql", sql.sql()); | ||
| return m.build(); |
There was a problem hiding this comment.
How does this Representation deserialize in DTO tranferring? I think we should align the json serde with DTO. Here the hard-coded toMap is very hard to maintain.
There was a problem hiding this comment.
Or, you can figure out a more explicit way to do this.
| SessionUtils.getWithoutCommit( | ||
| ViewMetaMapper.class, mapper -> mapper.updateViewMeta(newViewPO, oldViewPO))); | ||
| if (updateResult.get() == 0) { | ||
| throw new RuntimeException("Failed to update the entity: " + ident); |
There was a problem hiding this comment.
Shall we throw a NoSuchEntityException here?
There was a problem hiding this comment.
updateViewMeta(...) returning 0 here does not necessarily mean the view is missing; it can also be an optimistic-concurrency conflict (current_version mismatch).
We intentionally throw a runtime exception inside the transaction to force rollback, then map it to IOException for consistency with other update paths (e.g., TableMetaService and FilesetMetaService).
Align view representation serialization with FunctionPO style by introducing typed Representation DTOs instead of hard-coded map conversion in ViewPO. Add DTO serde tests in common and round-trip compatibility tests in core, including legacy json without explicit type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add constructor Javadoc for RepresentationDTO to satisfy common:javadoc with -Werror. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
What changes were proposed in this pull request?
Adds the relational storage layer for the Logical View Management feature:
ViewEntity(implementsView,HasIdentifier) withdefaultCatalog/defaultSchemaaligned with main's view API.ViewVersionInfoPO+ViewVersionInfoMapperwith base + PostgreSQL SQL providers.ViewPO/ViewMetaMapperproviders extended to read/writeaudit_info,default_catalog,default_schemaand join the currentview_version_inforow.ViewMetaServicerewritten for full CRUD with version management and JSON serialization of columns/properties/representations; soft-delete support.JDBCBackendroutesViewEntityinserts throughViewMetaService; bumpsCURRENT_SCRIPT_VERSIONto1.3.0.ViewOperationDispatcherupdated with acreateViewstub matching main'sViewCatalog#createView(..., String defaultCatalog, String defaultSchema, Map)signature; full CRUD dispatchers will land in a follow-up PR.Why are the changes needed?
This is the storage portion of the Logical View Management EPIC (#604). The API and DDL are already on main; this PR wires the persistence layer so views can be stored in Gravitino's metadata store with versioned snapshots.
Fix: #10878
Does this PR introduce any user-facing change?
No new user-facing APIs in this PR.
CURRENT_SCRIPT_VERSIONis bumped from1.2.0to1.3.0to match the schema files already present on main; deployments will need to apply theupgrade-1.2.0-to-1.3.0-*.sqlscript (already in the repo).How was this patch tested?
TestViewMetaServicecovering insert / load / update (with new version row) / soft delete / list-by-namespace / multi-representation round-trip.TestViewOperationDispatcherextended for the updatedcreateViewsignature../gradlew :core:test --tests "*TestViewMetaService*" --tests "*TestViewOperationDispatcher*" -PskipITs— all 16 tests pass.