Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,6 @@ public Page<TableIdentifier> listTables(Namespace namespace, Pageable pageable)
.map(houseTable -> TableIdentifier.of(houseTable.getDatabaseId(), houseTable.getTableId()));
}

/**
* Paginated listing that preserves the underlying {@link HouseTable} rows, so callers can read
* HTS-resident columns (e.g. tableLocation) without an extra metadata.json load per table.
*/
public Page<HouseTable> listHouseTables(Namespace namespace, Pageable pageable) {
NamespaceUtil.validateOperationNamespace(namespace);
return houseTableRepository.findAllByDatabaseId(namespace.toString(), pageable);
}

/**
* Direct HTS lookup that returns the {@link HouseTable} row without parsing metadata.json. Use
* this when only HTS-resident columns (e.g. tableUUID, tableLocation) are needed — for example,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.linkedin.openhouse.tables.api.spec.v0.response.GetAllSoftDeletedTablesResponseBody;
import com.linkedin.openhouse.tables.api.spec.v0.response.GetAllTablesResponseBody;
import com.linkedin.openhouse.tables.api.spec.v0.response.GetTableResponseBody;
import java.util.List;

/**
* Interface layer between REST and Tables backend. The implementation is injected into the Service
Expand Down Expand Up @@ -38,25 +37,16 @@ ApiResponse<GetTableResponseBody> getTable(

/**
* Function to Get one Page of Table Resources in a given databaseId given the page size and sort
* the results by the sortBy field. The caller may optionally request that additional fields
* (beyond databaseId + tableId) be populated on each returned table. When {@code fields} is
* non-empty, {@code actingPrincipal} must hold GET_TABLE_METADATA on the database.
* the results by the sortBy field.
*
* @param databaseId
* @param page
* @param size
* @param sortBy
* @param fields optional list of GetTableResponseBody field names to populate
* @param actingPrincipal authenticated user; required when {@code fields} is non-empty
* @return A page of tables in the given database.
*/
ApiResponse<GetAllTablesResponseBody> searchTables(
String databaseId,
int page,
int size,
String sortBy,
List<String> fields,
String actingPrincipal);
String databaseId, int page, int size, String sortBy);

/**
* Function to Create Table Resource in a given databaseId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import com.linkedin.openhouse.tables.dto.mapper.TablesMapper;
import com.linkedin.openhouse.tables.model.TableDto;
import com.linkedin.openhouse.tables.services.TablesService;
import java.util.List;
import java.util.stream.Collectors;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.util.Pair;
Expand Down Expand Up @@ -64,20 +63,15 @@ public ApiResponse<GetAllTablesResponseBody> searchTables(String databaseId) {

@Override
public ApiResponse<GetAllTablesResponseBody> searchTables(
String databaseId,
int page,
int size,
String sortBy,
List<String> fields,
String actingPrincipal) {
tablesApiValidator.validateSearchTables(databaseId, page, size, sortBy, fields);
String databaseId, int page, int size, String sortBy) {
tablesApiValidator.validateSearchTables(databaseId, page, size, sortBy);
return ApiResponse.<GetAllTablesResponseBody>builder()
.httpStatus(HttpStatus.OK)
.responseBody(
GetAllTablesResponseBody.builder()
.pageResults(
tableService
.searchTables(databaseId, page, size, sortBy, fields, actingPrincipal)
.searchTables(databaseId, page, size, sortBy)
.map(tableDto -> tablesMapper.toGetTableResponseBody(tableDto)))
.build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.linkedin.openhouse.tables.api.spec.v0.request.CreateUpdateLockRequestBody;
import com.linkedin.openhouse.tables.api.spec.v0.request.CreateUpdateTableRequestBody;
import com.linkedin.openhouse.tables.api.spec.v0.request.UpdateAclPoliciesRequestBody;
import java.util.List;

public interface TablesApiValidator {

Expand Down Expand Up @@ -34,12 +33,10 @@ public interface TablesApiValidator {
* @param page
* @param size
* @param sortBy
* @param fields optional list of GetTableResponseBody field names to populate
* @throws com.linkedin.openhouse.common.exception.RequestValidationFailureException if request is
* invalid
*/
void validateSearchTables(
String databaseId, int page, int size, String sortBy, List<String> fields);
void validateSearchTables(String databaseId, int page, int size, String sortBy);

/**
* Function to validate a request to create a Table Resource.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,6 @@
@Component
public class OpenHouseTablesApiValidator implements TablesApiValidator {

/**
* Fields selectable via the v2 search {@code ?fields=} query param. Each entry must match a field
* name on {@link com.linkedin.openhouse.tables.api.spec.v0.response.GetTableResponseBody}.
*/
private static final Set<String> SUPPORTED_SEARCH_FIELDS =
Collections.unmodifiableSet(new HashSet<>(Arrays.asList("tableLocation")));

@Autowired private Validator validator;

@Autowired private RetentionPolicySpecValidator retentionPolicySpecValidator;
Expand Down Expand Up @@ -80,23 +73,10 @@ public void validateSearchTables(String databaseId) {
}

@Override
public void validateSearchTables(
String databaseId, int page, int size, String sortBy, List<String> fields) {
public void validateSearchTables(String databaseId, int page, int size, String sortBy) {
List<String> validationFailures = new ArrayList<>();
validateDatabaseId(databaseId, validationFailures);
ApiValidatorUtil.validatePageable(page, size, sortBy, validationFailures);
if (fields != null && !fields.isEmpty()) {
List<String> unsupported =
fields.stream()
.filter(f -> !SUPPORTED_SEARCH_FIELDS.contains(f))
.collect(Collectors.toList());
if (!unsupported.isEmpty()) {
validationFailures.add(
String.format(
"fields : unsupported field(s) %s. Supported fields: %s",
unsupported, SUPPORTED_SEARCH_FIELDS));
}
}
if (!validationFailures.isEmpty()) {
throw new RequestValidationFailureException(validationFailures);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import java.util.List;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.annotation.Secured;
Expand Down Expand Up @@ -119,19 +118,10 @@ public ResponseEntity<GetAllTablesResponseBody> searchTablesPaginated(
@Parameter(description = "Database ID", required = true) @PathVariable String databaseId,
@RequestParam(required = false, defaultValue = "0") int page,
@RequestParam(required = false, defaultValue = "50") int size,
@RequestParam(required = false) String sortBy,
@Parameter(
description =
"Optional list of GetTableResponseBody field names to populate beyond the "
+ "identifiers (databaseId, tableId). Requires GET_TABLE_METADATA on the "
+ "database when set. Supported values: \"tableLocation\". Accepts "
+ "comma-separated (?fields=a,b) or repeated (?fields=a&fields=b) values.")
@RequestParam(name = "fields", required = false)
List<String> fields) {
@RequestParam(required = false) String sortBy) {

com.linkedin.openhouse.common.api.spec.ApiResponse<GetAllTablesResponseBody> apiResponse =
tablesApiHandler.searchTables(
databaseId, page, size, sortBy, fields, extractAuthenticatedUserPrincipal());
tablesApiHandler.searchTables(databaseId, page, size, sortBy);

return new ResponseEntity<>(
apiResponse.getResponseBody(), apiResponse.getHttpHeaders(), apiResponse.getHttpStatus());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.linkedin.openhouse.tables.dto.mapper;

import com.linkedin.openhouse.common.schema.IcebergSchemaHelper;
import com.linkedin.openhouse.internal.catalog.model.HouseTable;
import com.linkedin.openhouse.internal.catalog.model.SoftDeletedTableDto;
import com.linkedin.openhouse.tables.api.spec.v0.request.CreateUpdateTableRequestBody;
import com.linkedin.openhouse.tables.api.spec.v0.request.IcebergSnapshotsRequestBody;
Expand All @@ -15,7 +14,6 @@
import com.linkedin.openhouse.tables.model.TableDto;
import com.linkedin.openhouse.tables.model.TableDtoPrimaryKey;
import java.util.HashMap;
import java.util.Set;
import org.apache.iceberg.SortOrder;
import org.apache.iceberg.Table;
import org.apache.iceberg.catalog.TableIdentifier;
Expand Down Expand Up @@ -159,20 +157,6 @@ public interface TablesMapper {
})
TableDto toTableDto(TableIdentifier tableIdentifier);

/**
* Build a partially-populated TableDto from a HouseTable, populating only the fields the caller
* requested. Identifier fields (databaseId, tableId) are always populated. Used by the paginated
* search endpoint to avoid loading metadata.json per table.
*/
default TableDto toTableDto(HouseTable houseTable, Set<String> fields) {
TableDto.TableDtoBuilder builder =
TableDto.builder().databaseId(houseTable.getDatabaseId()).tableId(houseTable.getTableId());
if (fields != null && fields.contains("tableLocation")) {
builder.tableLocation(houseTable.getTableLocation());
}
return builder.build();
}

/**
* Transform {@link SoftDeletedTableDto} to {@link GetSoftDeletedTableResponseBody}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ public interface OpenHouseInternalRepository

Page<TableDto> searchTables(String databaseId, Pageable pageable);

/**
* Paginated search that populates a caller-selected subset of fields on each returned {@link
* TableDto}. Passing a null or empty {@code fields} list returns identifier-only results (same as
* the two-arg overload).
*/
Page<TableDto> searchTables(String databaseId, Pageable pageable, List<String> fields);

void rename(TableDtoPrimaryKey from, TableDtoPrimaryKey to);

Page<SoftDeletedTableDto> searchSoftDeletedTables(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@
import com.linkedin.openhouse.tables.repository.SchemaValidator;
import io.micrometer.core.instrument.MeterRegistry;
import io.opentelemetry.instrumentation.annotations.WithSpan;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.collections.CollectionUtils;
Expand Down Expand Up @@ -735,22 +733,6 @@ public Page<TableDto> searchTables(String databaseId, Pageable pageable) {
.map(tablesIdentifier -> mapper.toTableDto(tablesIdentifier));
}

@Timed(metricKey = MetricsConstant.REPO_TABLES_SEARCH_BY_DATABASE_PAGINATED_TIME)
@Override
public Page<TableDto> searchTables(String databaseId, Pageable pageable, List<String> fields) {
if (CollectionUtils.isEmpty(fields)) {
return searchTables(databaseId, pageable);
}
if (!(catalog instanceof OpenHouseInternalCatalog)) {
throw new UnsupportedOperationException(
"Does not support paginated search for getting all tables in a database");
}
Set<String> fieldSet = new HashSet<>(fields);
return ((OpenHouseInternalCatalog) catalog)
.listHouseTables(Namespace.of(databaseId), pageable)
.map(houseTable -> mapper.toTableDto(houseTable, fieldSet));
}

@Timed(metricKey = MetricsConstant.REPO_TABLE_IDS_FIND_ALL_TIME)
@Override
public List<TableDtoPrimaryKey> findAllIds() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,31 +33,15 @@ public interface TablesService {
List<TableDto> searchTables(String databaseId);

/**
* Given a databaseId, prepare list of {@link TableDto}s. The returned dtos are populated with
* identifier fields plus any HouseTable-resident fields listed in {@code fields}. Passing null or
* empty returns identifier-only dtos.
*
* <p>When {@code fields} is non-empty, {@code actingPrincipal} must hold the {@code
* GET_TABLE_METADATA} privilege on the database — per-table sharing ACLs are not consulted by
* this call, so this guard prevents callers from bulk-reading field data on tables they would not
* be authorized to read individually.
* Given a databaseId, prepare list of {@link TableDto}s.
*
* @param databaseId
* @param page
* @param size
* @param sortBy
* @param fields optional list of {@link TableDto}/{@code GetTableResponseBody} field names to
* populate in addition to identifiers
* @param actingPrincipal authenticated user; required when {@code fields} is non-empty
* @return list of {@link TableDto}
*/
Page<TableDto> searchTables(
String databaseId,
int page,
int size,
String sortBy,
List<String> fields,
String actingPrincipal);
Page<TableDto> searchTables(String databaseId, int page, int size, String sortBy);

/**
* Given a {@link CreateUpdateTableRequestBody}, create or update a Openhouse table for it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,9 @@ public List<TableDto> searchTables(String databaseId) {
}

@Override
public Page<TableDto> searchTables(
String databaseId,
int page,
int size,
String sortBy,
List<String> fields,
String actingPrincipal) {
if (fields != null && !fields.isEmpty()) {
authorizationUtils.checkDatabasePrivilege(
databaseId, actingPrincipal, Privileges.GET_TABLE_METADATA);
}
public Page<TableDto> searchTables(String databaseId, int page, int size, String sortBy) {
Pageable pageable = createPageable(page, size, sortBy, null);
return openHouseInternalRepository.searchTables(databaseId, pageable, fields);
return openHouseInternalRepository.searchTables(databaseId, pageable);
}

@WithSpan("TablesService.putTable")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1026,48 +1026,6 @@ public void testSearchTablesWithDatabaseIdPaginated() throws Exception {
}
}

@Test
public void testSearchTablesPaginatedWithTableLocationField() throws Exception {
List<GetTableResponseBody> tables = new ArrayList<>();
for (int i = 0; i < 3; i++) {
GetTableResponseBody table = buildGetTableResponseBodyWithDbTbl("d1", "tloc" + i);
tables.add(table);
RequestAndValidateHelper.createTableAndValidateResponse(table, mvc, storageManager);
}

mvc.perform(
MockMvcRequestBuilders.post("/v2/databases/d1/tables/search")
.param("page", "0")
.param("size", "10")
.param("fields", "tableLocation")
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.pageResults.content", hasSize(3)))
.andExpect(jsonPath("$.pageResults.content[0].tableId", notNullValue()))
.andExpect(jsonPath("$.pageResults.content[0].databaseId", is("d1")))
.andExpect(jsonPath("$.pageResults.content[0].tableLocation", notNullValue()))
.andExpect(
jsonPath(
"$.pageResults.content[0].tableLocation",
org.hamcrest.Matchers.endsWith(".metadata.json")));

for (GetTableResponseBody t : tables) {
RequestAndValidateHelper.deleteTableAndValidateResponse(mvc, t);
}
}

@Test
public void testSearchTablesPaginatedRejectsUnknownField() throws Exception {
mvc.perform(
MockMvcRequestBuilders.post("/v2/databases/d1/tables/search")
.param("fields", "schema")
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.message", containsString("schema")))
.andExpect(jsonPath("$.message", containsString("tableLocation")));
}

@Test
public void testUpdateSucceedsForColumnTags() throws Exception {
MvcResult mvcResult =
Expand Down
Loading