Skip to content

PHOENIX-7795 : Tenant TTL using Tenant Views on Multi-Tenant Table#2400

Open
palashc wants to merge 14 commits into
apache:masterfrom
palashc:tenant_ttl
Open

PHOENIX-7795 : Tenant TTL using Tenant Views on Multi-Tenant Table#2400
palashc wants to merge 14 commits into
apache:masterfrom
palashc:tenant_ttl

Conversation

@palashc
Copy link
Copy Markdown
Contributor

@palashc palashc commented Apr 6, 2026

Summary

Enable per-tenant TTL on multi-tenant base tables by letting a tenant create a view without a WHERE clause and set TTL on it. Read masking and compaction use the existing View TTL infrastructure.

Changes

  • WhereOptimizer.getRowKeyMatcher: compute ROW_KEY_MATCHER from the connection's tenant-id when a tenant view is created without a WHERE clause, so it participates in the compaction RowKeyMatcher trie. Wraps ScanUtil.getTenantIdBytes in try/catch to avoid breaking creation when the tenant-id type doesn't match the schema.
  • CreateTableCompiler.compile: always invoke getRowKeyMatcher for UPDATABLE views (no longer gated on where != null).

New validation (ViewUtil.validateTenantViewWithoutWhereTTLCoexistence)

On a multi-tenant base table, among a given tenant's views without a WHERE clause, we allow either:

  • (a) any number of no-WHERE views without TTL, or
  • (b) exactly one no-WHERE view with TTL.
    Multiple no-WHERE TTL views share the same ROW_KEY_MATCHER (tenant-id bytes) and would conflict in the compaction RowKeyMatcher trie. The check rejects such operations with TENANT_VIEW_WITHOUT_WHERE_TTL_CONFLICT. Views with WHERE clauses are out of scope for this check (their prefix-conflict potential is a pre-existing concern tracked separately).
    The helper lives in ViewUtil and is invoked from:
  • CREATE VIEW (CreateTableCompiler) when creating a no-WHERE view on a multi-tenant base table.
  • ALTER VIEW SET TTL (MetaDataClient.evaluateStmtProperties) when the view being altered is a no-WHERE tenant view on a multi-tenant base table; the view itself is excluded from the sibling check.
    Gracefully skips when CHILD_LINK is unavailable (namespace mapping edge cases) or in connectionless mode (unit tests).

Tests (TenantTTLIT)

  • testReadMaskingWithDifferentTenantTTLs
  • testCompactionWithDifferentTenantTTLs — INTEGER non-tenant PK
  • testThreeTenantsDifferentTTLsCHAR(3) + BIGINT PK columns
  • testCompactionWithSplitRegions — pre-split base table
  • testCompactionWithSaltedMultiTenantTable
  • testCompactionWithTenantTableIndex — with secondary index on the base table
  • testSameViewNameAcrossDifferentTenants
  • testTenantTTLWithUpdatableViewRestrictionEnabled
  • testTenantTTLViewCoexistenceRules — 9 cases exercising every CREATE-path branch (no-WHERE non-TTL, no-WHERE TTL, mixed, different tenants, newly-allowed WHERE-scoped creations)
  • testAlterTTLOnTenantViewWithSiblingIsRejected — ALTER-path rejection for no-WHERE views
  • testAlterTTLOnWhereScopedTenantViewIsAllowed — ALTER-path allowed for WHERE-scoped views
  • testAlterTenantViewToAddTTL — ALTER TTL masking/compaction end-to-end
  • testChildViewsOfTenantTTLViewInheritTTL — child views inherit parent tenant view's TTL

@palashc palashc marked this pull request as draft April 6, 2026 22:32
@palashc palashc requested a review from jpisaac April 6, 2026 22:32
@palashc palashc marked this pull request as ready for review April 7, 2026 20:01
@palashc palashc changed the title PHOENIX-7795 : Tenant TTL using Global Views PHOENIX-7795 : Tenant TTL using Tenant Views on Multi-Tenant Table Apr 8, 2026
Copy link
Copy Markdown
Contributor

@jpisaac jpisaac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, Few test suggestions

@palashc palashc requested a review from jpisaac April 21, 2026 18:31
Palash Chauhan added 2 commits April 21, 2026 14:23
@palashc palashc marked this pull request as draft April 23, 2026 06:36
@palashc palashc marked this pull request as ready for review April 23, 2026 22:14
@jpisaac
Copy link
Copy Markdown
Contributor

jpisaac commented May 11, 2026

Code Review: PHOENIX-7795 PR #2400 — Tenant TTL using Tenant Views on Multi-Tenant Table

Summary

Thorough review (two rounds, extreme depth) covering the new validation, WhereOptimizer changes, CompactionScanner trie behavior, hierarchy constraint interactions, and PHOENIX_UPDATABLE_VIEW_RESTRICTION_ENABLED interactions. Happy-path tests are excellent. Three issues found at the edges.


[MEDIUM] hasTTLProperty returns true for TTL = NONE / TTL = 0 — false-positive rejection on CREATE path

File: CreateTableCompiler.java:451-458

hasTTLProperty checks for the presence of the TTL property key without examining the resolved value. Both TTL = NONE and TTL = 0 resolve to TTL_EXPRESSION_NOT_DEFINED via TTLExpressionFactory (i.e. semantically "no TTL"), but hasTTLProperty returns true. This is passed as newViewHasTTL to validateTenantViewWithoutWhereTTLCoexistence, which rejects the CREATE with TENANT_VIEW_WITHOUT_WHERE_TTL_CONFLICT.

The ALTER path at MetaDataClient.java:6628 correctly gates on !newTTL.equals(TTL_EXPRESSION_NOT_DEFINED) — the CREATE path should do the same.

Concrete consequence: CREATE VIEW v2 AS SELECT * FROM t TTL = NONE fails when a no-WHERE non-TTL sibling exists, even though the new view has no effective TTL.

Suggested fix: resolve through TableProperty.TTL.getValue() or compute the effective TTL the same way MetaDataClient.createTableInternal does, and treat null / TTL_EXPRESSION_NOT_DEFINED as "no TTL".


[HIGH] Silent fail-open in WhereOptimizer.getRowKeyMatcher — compaction-time NPE

File: WhereOptimizer.java:504-510

When ScanUtil.getTenantIdBytes throws SQLException (tenant-id type mismatch), the catch block sets tenantIdBytes = EMPTY_BYTE_ARRAY. For a no-WHERE view this causes getRowKeyMatcher to return EMPTY_BYTE_ARRAY, which is stored as NULL for ROW_KEY_MATCHER in SYSTEM.CATALOG.

At compaction time, CompactionScanner.getTTLInfo reads null for ROW_KEY_MATCHER and constructs a TableTTLInfo with matchPattern = null and a real TTL expression. The gate at CompactionScanner.java:623 passes (TTL is defined), and matcher.put(null, tableId) hits key.length on a null key — NullPointerException that crashes the region's compaction scanner.

The null-matcher path exists pre-PR (WHERE-scoped views where keySlots == null), but this PR adds a new, more likely trigger for it.

Suggested fix: (a) re-throw the SQLException for no-WHERE tenant views with TTL so the DDL fails at CREATE time — this is the correct behavior since it tells the user their tenant-id is incompatible before they store data they can't expire; (b) add a null guard before matcher.put in initializeMatcher/refreshMatcher as defense-in-depth for the pre-existing null-matcher path.


[MEDIUM] No-WHERE TTL view shadows WHERE-scoped TTL views in the RowKeyMatcher trie

[Needs more assessment]

Files: RowKeyMatcher.java:95-112, CompactionScanner.java:1353-1364

The RowKeyMatcher trie returns on shortest prefix match (get returns at the first node with a non-null tableId). A no-WHERE TTL view's ROW_KEY_MATCHER is just the tenant-id prefix (e.g. \x04acme), which is strictly shorter than any WHERE-scoped TTL view's matcher (e.g. \x04acme\x05AUDIT). When both exist for the same tenant, the no-WHERE prefix shadows the WHERE-scoped entry — all rows get the no-WHERE view's TTL, silently overriding the WHERE-scoped view's TTL.

Example:

-- Tenant 'acme' creates two sibling views on a multi-tenant table:
CREATE VIEW AUDIT_EVENTS AS SELECT * FROM EVENTS WHERE EVENT_TYPE = 'AUDIT' TTL = 200;
CREATE VIEW ALL_EVENTS AS SELECT * FROM EVENTS TTL = 100;
-- At compaction: audit event rows get TTL=100 instead of TTL=200

Why existing validations don't prevent this:

  1. Hierarchy check (TTL_ALREADY_DEFINED_IN_HIERARCHY): checks vertically (ancestors on CREATE, descendants on ALTER), not laterally. Both views are siblings of the base table — neither is an ancestor or descendant of the other.

  2. New coexistence check (validateTenantViewWithoutWhereTTLCoexistence): explicitly excludes WHERE-scoped siblings (ViewUtil.java:503-505). By design it only prevents no-WHERE + no-WHERE TTL conflicts.

  3. Updatable view restriction (PHOENIX_UPDATABLE_VIEW_RESTRICTION_ENABLED, PHOENIX-4555): when ON, the outcome is order-dependent. If the WHERE-scoped view is created first (no siblings for check 4) it stays UPDATABLE and gets a trie entry — shadowing occurs. If the no-WHERE view is created first, check 4 flips the WHERE-scoped view to READ_ONLY (PK column mismatch with the no-WHERE sibling), preventing the trie entry — but then the WHERE-scoped view's TTL is silently non-functional.

Suggested approach: consider warning or rejecting on CREATE/ALTER of a no-WHERE TTL view when WHERE-scoped TTL siblings already exist for the same tenant, or file a follow-up JIRA to track trie-level multi-TTL resolution (e.g. longest-match-wins).


Missing test coverage

  • CREATE VIEW … TTL = NONE and … TTL = 0 against a no-WHERE non-TTL sibling — should be allowed.
  • Tenant has both a no-WHERE TTL view and a WHERE-scoped TTL view — verify which TTL applies to WHERE-matching rows post-compaction.
  • WhereOptimizer.getRowKeyMatcher with TENANTID_IS_OF_WRONG_TYPE — verify DDL fails or compaction handles null matcher without NPE.
  • ALTER VIEW … SET TTL = NONE on a tenant view with a sibling — should be allowed (verified clean, worth an explicit test).

@palashc
Copy link
Copy Markdown
Contributor Author

palashc commented May 11, 2026

Changes Summary — PR Review Round 2

Issue 1: hasTTLProperty false positive

File: CreateTableCompiler.java

  • hasTTLProperty now resolves the TTL value through TableProperty.TTL.getValue() and returns false when it resolves to TTL_EXPRESSION_NOT_DEFINED (i.e., TTL = NONE or TTL = 0).
  • Added imports for TableProperty and LiteralTTLExpression.TTL_EXPRESSION_NOT_DEFINED.

Issue 2: NPE risk from empty ROW_KEY_MATCHER on TTL views

CREATE-path fail-fastCreateTableCompiler.java:

  • If where == null && newViewHasTTL && rowKeyMatcher.length == 0 && parent.isMultiTenant() && tenantId != null → throw TENANTID_IS_OF_WRONG_TYPE.
  • Preserves existing TenantIdTypeIT behavior (no-TTL views with inconvertible tenant-id still succeed).

Defense-in-depthCompactionScanner.java:

  • initializeMatcher and refreshMatcher now skip TableTTLInfo entries whose matchPattern is null or empty (log warn instead of inserting a null key into the trie).

Tests added — TenantTTLIT.java

  • testCreateNoWhereViewWithTTLNoneAllowedWithSiblingCREATE VIEW ... TTL = 'NONE' and ... TTL = 0 allowed alongside a no-WHERE non-TTL sibling.
  • testAlterTTLToNoneAllowedWithSiblingALTER VIEW ... SET TTL = 'NONE' allowed even with siblings.
  • testCreateNoWhereTTLViewFailsForInconvertibleTenantIdCREATE VIEW ... TTL = 10 with inconvertible tenant-id fails fast with TENANTID_IS_OF_WRONG_TYPE; no-TTL view in same scenario still succeeds.

Issue 3

  • Skipped per prior agreement with reviewer.

Verification

Suite Tests Result
TenantTTLIT 16 Pass
TenantIdTypeIT 15 Pass (no regression)
ViewTTLIT 68 Pass (no regression)

FYI @jpisaac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants