Add entityType discriminator and table-scoped HTS queries

[ruolin59]

Summary

This is the first PR toward supporting Iceberg views in OpenHouse. Views will share the
(databaseId, objectId) key space with tables, so this change adds the discriminator that tells
them apart and makes the existing table queries filter on it.

The new entity_type column runs from MySQL/H2 through HTS, the generated client, and the internal
HouseTable pointer. NULL and TABLE mean table, VIEW means view. The column is nullable and
not backfilled, so existing rows and existing table writes are unaffected. Nothing writes VIEW
yet, so this is inert at runtime.

JDBC methods

Entity type is chosen by calling a different method rather than by passing an argument.

Scope	Methods
Neutral	findBy…, existsBy…, deleteBy…, findById, existsById, deleteById, renameTableId
Databases	findAllDistinctDatabaseIds ×2
Both types	findAllByFilters ×2, findAllByDatabaseIdAndTableIdLikeAllIgnoreCase ×2
Tables only (new)	findTableBy…, findAllTablesByFilters ×2, findAllTablesByDatabaseIdAndTableIdLikeAllIgnoreCase ×2

Nothing existing was renamed or changed behaviourally. The shared JPQL moved into constants
(COMMON_FILTER_CLAUSES, PATTERN_KEY_CLAUSES, TABLE_ROW_PREDICATE) so each table-scoped method
composes it, and the expansions were compared to confirm the general queries are byte-identical to
before. View methods follow the same shape and land with the code that calls them.

Three things shaped this:

• Methods returning database names stay unfiltered. Filtering them would hide a database that
  contains only views, even though it exists and is addressable.
• Point reads and mutations on the shared key stay neutral, since the PUT, delete, and restore paths
  need to see a row of any type to detect a collision.
• The pattern methods kept dedicated table-scoped versions instead of folding into
  findAllByFilters, which matches tableId exactly. Merging a LIKE parameter would make _
  behave as a wildcard, and identifiers here often contain underscores.

findAllByDatabaseIdIgnoreCase ×2 were dropped. The paged one was never called, and paged
listTables already went through findAllByFilters, so findAllTablesByFilters covers both.

Callers need no type logic

HTS returns the correct rows, so none of its callers check entity type. A view 404s from the table point
read, surfaces as HouseTableNotFoundException, and reads as absent — which makes doRefresh,
dropTable, findTableRefById, and rename-source all correct as written. dropTable matters here
because it bypasses loadTable to survive corrupted metadata, so a guard on the refresh path would
have missed it.

Name occupancy — "what is at this key?" — needs to see rows of any type, and its only callers are
CREATE TABLE and the rename-destination check. That lands with the view work.

Incidental fix

MapStruct applies any String -> String method on a mapper to every String property.
stripOhNamespace assumed a non-null key, which held only because HouseTable had no nullable
String field until now. It is now null-safe; a null tableVersion would have hit the same bug.

Tests

Existing tests cover every changed call site in UserTablesServiceImpl — testGetUserTables,
testUserTableQuery, testGetUserTablesWithTablePattern, testGetUserTablesWithSearchFilter,
testUserTableGet, testListDatabases. All still pass unmodified: the diff on that test class is
247 insertions, 0 deletions, and no existing assertion anywhere in this PR was deleted or
relaxed. Since the table behaviour was meant to be unchanged, those tests are the regression proof.

New tests were written before the implementation. They cover both-types vs tables-only results
across the filter and pattern families, page counts with views interleaved, the point read treating
a view as absent while the neutral read still returns it, and unrecognised discriminators failing
closed. Page assertions check content, size, total elements, and total pages, so filtering a
returned page instead of the query would fail them. Case handling is asserted in Java, since H2 in
MODE=MySQL is case-sensitive and production MySQL is not.

Module	Tests	Failures
services:housetables	177	0
iceberg:openhouse:internalcatalog	87	0
services:tables	475	0
tables-test-fixtures_2.12	8	0
openhouse-spark-3.5-itest	66	0

Both fixture variants compile and the 1.2 fixture's tests run, since HouseTableRepository is
inherited by Spring Data proxies in published fixture code.

Rollout

schema.sql uses CREATE TABLE IF NOT EXISTS, so production needs
ALTER TABLE user_table_row ADD COLUMN entity_type VARCHAR(128) DEFAULT NULL before deploying this pr. No backfill needed. Deploy HTS before the tables service, since the filtering lives in HTS.

[cbb330]

Thanks for the change!

The piece I want to understand is why views should live in the same rows as tables. Iceberg does not require that, it needs a pointer to view metadata and a views API.

apache/polaris also puts both in the same store under one unique name. but /tables and /views stay different APIs, so a table call never returns a view. this is compatible with the iceberg rest spec as well.

Adding entityType onto UserTable gets the unique name. but GET /hts/tables will 404 a view, which is ok, but PUT / delete / rename on that path still see any row, and a view can be written through the tables endpoint. That is what will confuse people later: the column is still table_id.

Could you say why not a separate view table, and whether views get their own endpoint before anything writes a VIEW?

[cbb330]

entity_type will change the composite PK, and require updating the index in mysql. so this should be as tiny as possible, to allow the index to scale. consider either: a new type (enum) or smaller value.

[ruolin59]

hey @cbb330, I'm not sure how adding a new general column will modify the composite PK. As you can see in the unchanged line 12, the primary keypair remains as (database_id, table_id). Unless you are suggesting that we should add entity_type as part of the PK? This is not necessary because we specifically are relying on the fact that the pk is (database_id, table_id) to enforce naming uniqueness across views and tables.

Re. your other comment related to separating tables for views, this is a part of the fundamental design which has already gone over multiple rounds of review and has been documented extensively. Briefly here, table paths will only see tables, and view paths will only see views, neither will see the other except for the case of confirming name uniqueness

[cbb330]

You're right that this column does not change the PK.

Could you add the comparison for keeping views on /hts/tables vs a separate view endpoint (or a separate table)? Unique names can live in one store either way, so I want the data for this API choice: what it buys, what it costs, and what was tried.

In this PR, GET /hts/tables hides views, but PUT / delete / rename still see any row. If /hts/views is meant to own those writes, when does it land relative to the first VIEW row?

[ruolin59]

this pr is the first of a series of prs that implements views, it only adds the new column to differentiate between tables and views in HTS and does not modify the behavior of any endpoints, except to ensure that fetching tables will not return views. As of this pr, there is still no way to insert a view into the hts

[cbb330]

1. How does a table write not overwrite a view?

This PR makes GET /hts/tables treat a view identifier as missing, while PUT / delete / rename still key only on (databaseId, tableId). PUT already accepts entityType=VIEW.

The M1 doc puts the 409 (NAME_ALREADY_EXISTS_AS_TABLE) and “table drop/rename reject VIEW” on the tables service, not in HTS. Which change actually prevents a table PUT from attaching to that row? If it is a later PR, what stops a write through HTS in the meantime?

2. Why does a separate table not work?

This PR puts entity_type on user_table_row. The M1 doc says two key spaces would let a table and a view share a name, and that cannot be tightened later.

That argues against two independent names, not against two tables with a uniqueness check. Why does that option lose?

[ruolin59]

1. this pr does not add any paths pertaining to views, it is only scoped to adding entityType in the db to be able to differentiate views. There is another pr that will implement the actual api paths which ensures that table endpoints only touch tables, and view endpoints only touch views
2. please read the on the extra context discussion doc linked at the beginning of M1, it goes into details as to why 2 tables don't work to ensure uniqueness

[cbb330]

1. GET /hts/tables hides views. but, the other /hts/tables APIs still see them. PUT / delete / rename stay on findById and will return or update a VIEW row.

For this PR, either drop entityType=VIEW from the PUT body until the later PR, or reject a VIEW row on table PUT / delete / rename (see it, don't use it). Until one of those lands, any caller of the unfiltered methods, or a new @Query without the predicate, sees views. The tables-service client is told not to filter at all.

Stepping back a bit and looking at the big picture: the view exclusion is a predicate on specific read methods, not on the row. Every new table query has to remember it. That feels like the wrong layer storage or the model should own it, not each API. The current gaps are below:

API	Filtered?
GET /hts/tables	yes
GET /hts/tables/query	yes
GET /v1/hts/tables/query	yes
GET /hts/tables/querySoftDeleted	no — other table, no entity_type
PUT /hts/tables	no
DELETE /hts/tables and /v1/hts/tables	no
PATCH /hts/tables/rename	no
PUT /hts/tables/restore	no — occupancy is findById
DELETE /hts/tables/purge	no — other table
list databases (empty query)	no — findAllDistinctDatabaseIds

3. Read the discussion section of your doc. Two tables vs one is the same cost. Table and view are independent rows either way. Occupancy for both is two HTS lookups, the result is O(1)×2 on the index. @mkuchenbecker can you call this?

	one table + entityType	two tables
CREATE	2 lookups + insert	2 lookups + insert
READ (typed)	1 lookup	1 lookup
READ (name)	2 lookups	2 lookups
UPDATE	1 lookup + write	1 lookup + write
DELETE	1 lookup + delete	1 lookup + delete

[ruolin59]

On the unfiltered methods: those are neutral by design, not by omission, and filtering them is what would introduce a bug.

restoreUserTable is the clearest case. It checks occupancy with findById (UserTablesServiceImpl:195-201), throws AlreadyExistsException if anything holds the name, and catches DataIntegrityViolationException for the TOCTOU window. That only works because the read sees every row type. Scope it to tables and a VIEW holding the name becomes invisible — restore proceeds, and save() overwrites the view row, because it's the same primary key. The "gap" is the thing preventing the clobber.

Same for PUT and rename: cross-type collision detection requires seeing rows you don't own. That's pinned by testNeutralPointReadStillSeesEveryEntityType.

On rejecting VIEW rows today: nothing populates entityType in this PR, so no VIEW row can exist. A rejection branch would be unreachable code with no way to test it. Population happens controller-side in the follow-up ticket — this PR contains no controller or handler changes at all, which is also why there's no entityType=VIEW PUT body to drop.

You're right about querySoftDeleted. soft_deleted_user_table_row has no entity_type, so the discriminator is lost on delete and returns NULL on restore. I found the same thing and it's tracked in BDP-108627 — it has to be fixed before any backfill, or restore reintroduces NULLs.

On two tables vs one: the cost table counts lookups and omits the constraint, which is where the difference is.

The PK (database_id, table_id) is what makes a name unique across both types. With one table the database enforces it: concurrent CREATE TABLE and CREATE VIEW on the same name, one gets a PK violation. Atomic, no application logic.

With two tables nothing enforces it. Occupancy becomes check-tables, check-views, then insert — and two concurrent creates can both pass their checks and both insert, because no constraint spans two tables. You'd need a trigger or a third names table, which is the shared key space with extra steps.

So CREATE isn't symmetric either: with one table it's insert-and-catch, not two lookups plus insert. Equal row counts, unequal guarantees — atomicity is the reason for the shared key space, not lookup count.

[cbb330]

Right now “tables only” is pasted onto some queries and left off others. That’s the remaining concern. Callers have to pick the right method on the same repo, with no type safety.

Do this once on the type:

1. Table reads: new class TableOnlyRow. Same table, Hibernate @Where = tables only. GET / list use this. New queries through this class hide views.
2. Writes / occupancy: keep UserTableRow with no filter. PUT / rename / restore / delete use this. A view at that name still shows up, so these paths do not treat it as missing.

Do not copy the filter onto each query.

[mkuchenbecker]

I think we should explicitly backfill vs implicitly treating null as table.

[mkuchenbecker]

Can we type this with an enum or does it need to be a string?

[ruolin59]

Agreed on the enum — adding one for the HTS-internal representation (UserTableRow, UserTableDto). It converts back to a String at the API boundary, though: UserTable feeds the OpenAPI spec and the generated :client:hts, and StorageType is the precedent here — deliberately a class of public static final Type constants with a fromString factory rather than a Java enum, so new types don't break existing clients.

[mkuchenbecker]

How are we backfilling for existing tables?

[mkuchenbecker]

I don't like an implicit default as table as compared to ensuring this is plumbed.

[mkuchenbecker]

Enum please.

[ruolin59]

Agreed on the enum — adding one for the HTS-internal representation (UserTableRow, UserTableDto). It converts back to a String at the API boundary, though: UserTable feeds the OpenAPI spec and the generated :client:hts, and StorageType is the precedent here — deliberately a class of public static final Type constants with a fromString factory rather than a Java enum, so new types don't break existing clients.

[mkuchenbecker]

how is this being added to the existing MySQL? I have seen database upgrades as modeled as a series of DDL operations after initial creation.

[mkuchenbecker]

The database needs to be updated before this code lands.

[ruolin59]

this has been called out in the description of this pr already

[abhisheknath2011]

This would be now referred in multiple places. Can we create enum for this?

[ruolin59]

Agreed on the enum — adding one for the HTS-internal representation (UserTableRow, UserTableDto). It converts back to a String at the API boundary, though: UserTable feeds the OpenAPI spec and the generated :client:hts, and StorageType is the precedent here — deliberately a class of public static final Type constants with a fromString factory rather than a Java enum, so new types don't break existing clients.

[abhisheknath2011]

As we are adding new column entity_type to the existing table. We could consider updating existing rows to table and then rollout the changes. If there any any new table created during the table schema update and server side deployment, there will be null for those tables. So we could update those entires again. After this we should be able to remove this NULL check from the code.

[abhisheknath2011]

Summary

This is the first PR toward supporting Iceberg views in OpenHouse. Views will share the (databaseId, objectId) key space with tables, so this change adds the discriminator that tells them apart and makes the existing table queries filter on it.

The new entity_type column runs from MySQL/H2 through HTS, the generated client, and the internal HouseTable pointer. NULL and TABLE mean table, VIEW means view. The column is nullable and not backfilled, so existing rows and existing table writes are unaffected. Nothing writes VIEW yet, so this is inert at runtime.

JDBC methods

Entity type is chosen by calling a different method rather than by passing an argument.

Scope Methods
Neutral findBy…, existsBy…, deleteBy…, findById, existsById, deleteById, renameTableId
Databases findAllDistinctDatabaseIds ×2
Both types findAllByFilters ×2, findAllByDatabaseIdAndTableIdLikeAllIgnoreCase ×2
Tables only (new) findTableBy…, findAllTablesByFilters ×2, findAllTablesByDatabaseIdAndTableIdLikeAllIgnoreCase ×2
Nothing existing was renamed or changed behaviourally. The shared JPQL moved into constants (COMMON_FILTER_CLAUSES, PATTERN_KEY_CLAUSES, TABLE_ROW_PREDICATE) so each table-scoped method composes it, and the expansions were compared to confirm the general queries are byte-identical to before. View methods follow the same shape and land with the code that calls them.

Three things shaped this:

• Methods returning database names stay unfiltered. Filtering them would hide a database that
  contains only views, even though it exists and is addressable.
• Point reads and mutations on the shared key stay neutral, since the PUT, delete, and restore paths
  need to see a row of any type to detect a collision.
• The pattern methods kept dedicated table-scoped versions instead of folding into
  findAllByFilters, which matches tableId exactly. Merging a LIKE parameter would make _
  behave as a wildcard, and identifiers here often contain underscores.

findAllByDatabaseIdIgnoreCase ×2 were dropped. The paged one was never called, and paged listTables already went through findAllByFilters, so findAllTablesByFilters covers both.

Callers need no type logic

HTS returns the correct rows, so none of its callers check entity type. A view 404s from the table point read, surfaces as HouseTableNotFoundException, and reads as absent — which makes doRefresh, dropTable, findTableRefById, and rename-source all correct as written. dropTable matters here because it bypasses loadTable to survive corrupted metadata, so a guard on the refresh path would have missed it.

Name occupancy — "what is at this key?" — needs to see rows of any type, and its only callers are CREATE TABLE and the rename-destination check. That lands with the view work.

Incidental fix

MapStruct applies any String -> String method on a mapper to every String property. stripOhNamespace assumed a non-null key, which held only because HouseTable had no nullable String field until now. It is now null-safe; a null tableVersion would have hit the same bug.

Tests

Existing tests cover every changed call site in UserTablesServiceImpl — testGetUserTables, testUserTableQuery, testGetUserTablesWithTablePattern, testGetUserTablesWithSearchFilter, testUserTableGet, testListDatabases. All still pass unmodified: the diff on that test class is 247 insertions, 0 deletions, and no existing assertion anywhere in this PR was deleted or relaxed. Since the table behaviour was meant to be unchanged, those tests are the regression proof.

New tests were written before the implementation. They cover both-types vs tables-only results across the filter and pattern families, page counts with views interleaved, the point read treating a view as absent while the neutral read still returns it, and unrecognised discriminators failing closed. Page assertions check content, size, total elements, and total pages, so filtering a returned page instead of the query would fail them. Case handling is asserted in Java, since H2 in MODE=MySQL is case-sensitive and production MySQL is not.

Module Tests Failures
services:housetables 177 0
iceberg:openhouse:internalcatalog 87 0
services:tables 475 0
tables-test-fixtures_2.12 8 0
openhouse-spark-3.5-itest 66 0
Both fixture variants compile and the 1.2 fixture's tests run, since HouseTableRepository is inherited by Spring Data proxies in published fixture code.

Rollout

schema.sql uses CREATE TABLE IF NOT EXISTS, so production needs ALTER TABLE user_table_row ADD COLUMN entity_type VARCHAR(128) DEFAULT NULL before deploying this pr. No backfill needed. Deploy HTS before the tables service, since the filtering lives in HTS.

Can we test the changes in docker container first and update the test results here? As a next step we could also validate the changes in test cluster before merging openhouse and li-openhouse (if any) changes. In such case in memory DB can use update schema.

[abhisheknath2011]

Can we have a common constant for this field?

[abhisheknath2011]

Any specific reasons for adding AllTables for these methods instead of All? Is view going to be served out of these methods?

[ruolin59]

the reason is that this will be a table-only method. for views, there will be separate "view"-named methods to handle. To keep things cleanly separated, the convention in the jdbc methods will be as follows:

• table specific methods: always filter on table-predicate, ie: is NULL OR = 'TABLE'
• view specific methods: always filter on view-predicate, ie; = 'VIEW'
• "general" entity fetching methods that returns entity of either: no table/view predicate filtering

[ruolin59]

I now have write access to this repo, so the same 17 commits are up as #696 on an upstream branch (rufan/views-entity-type-discriminator), rebased onto current main.

Keeping this PR open — it holds the review history, and the threads here are still the right place to continue those discussions. New review is probably easier on #696, since CI has full access there.

[ruolin59]

moving to different branch/pr at #696

[aastha25]

maybe a comment for the follow up PR, but in general, there should be some query patterns for findAllTablesAndViewsByDatabaseId(AndTableId)LikeAllIgnoreCase. User initiated queries like - list tables in a db should ideally, and have historically, returned all tables and views existing in a db. Why do we only want to get only tables / only views at this layer and let caller initiate 2 calls to HTS? instead we could return all tables + all views and let caller handle usecases in a targetted manner.

[aastha25]

ignore this comment, cross posted on the newer PR