TheNamesRai opened a new pull request, #2615:
URL: https://github.com/apache/phoenix/pull/2615
Fixes three bugs in CompactionScanner's View-TTL handling that could abort a
region's major compaction or apply the wrong TTL to a table:
- A View TTL created without a WHERE clause has a null/empty
`ROW_KEY_MATCHER`; compaction dereferenced it and aborted with a
`NullPointerException`. Such views are now skipped.
- Certain characters in a view's tenant id, schema, or name could make the
`SYSTEM.CATALOG` lookup fail and abort compaction. The lookup now binds these
values as query parameters instead of concatenating them.
- Compaction could apply `TTL` and `ROW_KEY_MATCHER` metadata from a view
not linked to the table being compacted. Each returned row's provenance is now
verified; non-matching rows are skipped.
Adds `ViewTTLCatalogLookupIT` and `ViewTTLGlobalViewScopeIT` covering the
fixed behavior.
### What changes were proposed in this pull request?
During major compaction, CompactionScanner works out the View TTL for each
view on the table being compacted. It enumerates the views linked to the
physical HBase table and, in `getTTLInfo()`, runs a `SYSTEM.CATALOG` lookup for
each view's TTL and `ROW_KEY_MATCHER`. This PR makes that path robust in three
ways, all confined to CompactionScanner (phoenix-core-server):
1. Parameterize the catalog lookup. The `(TENANT_ID, TABLE_SCHEM,
TABLE_NAME)` IN-list was built by string-concatenating each view's raw
identifiers; it is now built with bound parameters ((?, ?, ?) per view). The
binding reproduces the previous NULL-keyword-vs-quoted-literal rendering, so
the views the query legitimately matches are unchanged (confirmed by the
baseline purge tests).
2. Verify row provenance. A returned catalog row is honored only when its
(tenant, schema, name) matches one of the views actually linked to the table
being compacted; other rows are logged and skipped. `ROW_KEY_MATCHER` is a
table-agnostic byte prefix, so without this check a different view's
TTL/ROW_KEY_MATCHER could be applied to the table.
3. Guard a null/empty `ROW_KEY_MATCHER`. Views (or indexes) that carry a TTL
but a null or empty `ROW_KEY_MATCHER` are skipped at both
`RowKeyMatcher.put(...)` sites instead of being dereferenced.
### Why are the changes needed?
Each case aborts or misdirects major compaction, and a stalled major
compaction stops View-TTL enforcement for every view on that table until the
offending view is dropped:
1. Null/empty `ROW_KEY_MATCHER` → `NullPointerException`. A view created
with a TTL but no WHERE clause (a bare SELECT *) has a null ROW_KEY_MATCHER;
compaction dereferenced it in RowKeyMatcher.put(...) and aborted the region's
major compaction.
2. Certain characters in an identifier → failed lookup. Because identifiers
were concatenated into the query text, a view whose tenant id, schema, or name
contains a character such as a single quote (') produced a malformed query
(ERROR 604 (42P00)) that aborted compaction.
3. Missing provenance check → wrong TTL applied. The lookup result was used
without confirming the row belonged to a view linked to the table being
compacted, so compaction could apply a different view's TTL and ROW_KEY_MATCHER.
### Does this PR introduce any user-facing change?
No new features, APIs, or configuration. It is a behavioral bug fix: major
compaction no longer aborts when a view on the table has a null
`ROW_KEY_MATCHER` (TTL but no WHERE) or an identifier containing characters
such as a single quote, and each table's View TTL is applied only from views
actually linked to it. Previously such a view stalled major compaction (expired
rows were not purged) or caused a different view's TTL to be applied.
The two new ITs contain the exact, runnable reproduction; illustratively,
the following previously aborted major compaction and is now handled:
-- (a) TTL with no WHERE clause -> null ROW_KEY_MATCHER
`CREATE VIEW my_view AS SELECT * FROM my_table TTL = '10';`
-- (b) a delimited view name containing a single quote
`CREATE VIEW "v'x" AS SELECT * FROM my_table WHERE k = 'p' TTL = '10';`
How was this patch tested?
- `ViewTTLCatalogLookupIT` - asserts the corrected behavior on a patched
build: a baseline view TTL still purges expired rows; a view whose name or
schema contains an unusual character no longer aborts compaction (rows purge as
expected); a view with a null matcher (TTL, no WHERE) is skipped rather than
aborting compaction; and metadata from a view not linked to the table is not
applied (those rows survive).
- `ViewTTLGlobalViewScopeIT` - a scope check for how a global view's TTL is
applied on a multi-tenant table (a tenant-id-pinned matcher vs. a shared
post-tenant key), including a positive control that a covered prefix does purge.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Anthropic Claude Opus 4.8)****
--
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]
