laskoviymishka commented on code in PR #16131:
URL: https://github.com/apache/iceberg/pull/16131#discussion_r4015232448
##########
core/src/main/java/org/apache/iceberg/rest/RESTTable.java:
##########
@@ -18,74 +18,46 @@
*/
package org.apache.iceberg.rest;
-import java.util.Map;
-import java.util.Set;
-import java.util.function.Supplier;
+import java.util.Optional;
import org.apache.iceberg.BaseTable;
-import org.apache.iceberg.BatchScan;
-import org.apache.iceberg.BatchScanAdapter;
-import org.apache.iceberg.ImmutableTableScanContext;
-import org.apache.iceberg.SupportsDistributedScanPlanning;
+import org.apache.iceberg.SupportsReadRestrictions;
import org.apache.iceberg.TableOperations;
-import org.apache.iceberg.TableScan;
-import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.metrics.MetricsReporter;
+import org.apache.iceberg.rest.restrictions.ReadRestrictions;
-class RESTTable extends BaseTable implements SupportsDistributedScanPlanning {
- private final RESTClient client;
- private final Supplier<Map<String, String>> headers;
- private final MetricsReporter reporter;
- private final ResourcePaths resourcePaths;
- private final TableIdentifier tableIdentifier;
- private final Set<Endpoint> supportedEndpoints;
- private final Map<String, String> catalogProperties;
- private final Object hadoopConf;
+/**
+ * BaseTable specialization for tables loaded via a REST catalog. Carries the
per-principal {@link
+ * ReadRestrictions} that the REST server may have attached to the load
response and advertises the
+ * capability via {@link SupportsReadRestrictions}.
+ *
+ * <p>Used by {@link RESTSessionCatalog} for every table loaded through REST;
{@link
+ * RESTScanPlanningTable} extends this class to add server-side scan planning.
Non-REST catalogs
+ * (Hadoop, Hive, Glue, JDBC, Nessie, etc.) construct {@link BaseTable}
directly and do not
+ * advertise the capability — they have no pathway to produce a {@link
ReadRestrictions}.
+ */
+class RESTTable extends BaseTable implements SupportsReadRestrictions {
+ private final Optional<ReadRestrictions> readRestrictions;
RESTTable(
TableOperations ops,
String name,
MetricsReporter reporter,
- RESTClient client,
- Supplier<Map<String, String>> headers,
- TableIdentifier tableIdentifier,
- ResourcePaths resourcePaths,
- Set<Endpoint> supportedEndpoints,
- Map<String, String> catalogProperties,
- Object hadoopConf) {
+ ReadRestrictions readRestrictions) {
super(ops, name, reporter);
- this.reporter = reporter;
- this.client = client;
- this.headers = headers;
- this.tableIdentifier = tableIdentifier;
- this.resourcePaths = resourcePaths;
- this.supportedEndpoints = supportedEndpoints;
- this.catalogProperties = catalogProperties;
- this.hadoopConf = hadoopConf;
- }
-
- @Override
- public TableScan newScan() {
- return new RESTTableScan(
- this,
- schema(),
- ImmutableTableScanContext.builder().metricsReporter(reporter).build(),
- client,
- headers.get(),
- operations(),
- tableIdentifier,
- resourcePaths,
- supportedEndpoints,
- catalogProperties,
- hadoopConf);
- }
-
- @Override
- public BatchScan newBatchScan() {
- return new BatchScanAdapter(newScan());
+ this.readRestrictions =
+ readRestrictions != null && !readRestrictions.isEmpty()
+ ? Optional.of(readRestrictions)
+ : Optional.empty();
+ // Validate here, where server-provided restrictions first meet a table,
so every reader
+ // inherits the check rather than re-deriving it per scan. Reads on the
loaded table still fail
+ // closed on anything else they cannot apply. Uses ops directly rather
than the overridable
+ // schemas() to avoid calling an overridable method from a constructor.
+ this.readRestrictions.ifPresent(
+ restrictions -> restrictions.validate(ops.current().schemasById()));
Review Comment:
`expireSnapshots` prunes schemas no live snapshot references, so validating
against `schemasById()` can reject a restriction on a since-expired field id
and make the table unloadable — I'd validate against the full schema history.
##########
core/src/main/java/org/apache/iceberg/rest/restrictions/ReadRestrictions.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.rest.restrictions;
+
+import java.io.Serializable;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.functions.IcebergFunction;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+
+/**
+ * Server-provided read restrictions for the authenticated principal.
+ *
+ * <p>Applies only to the principal identified by the request's
authentication. An empty instance
+ * (no row filter, no column projections) is equivalent to the property being
absent from the
+ * response.
+ */
+public class ReadRestrictions implements Serializable {
Review Comment:
`ReadRestrictions` is `Serializable` but pins no `serialVersionUID`, so a
class change breaks deserialize across versions — I'd add `serialVersionUID =
1L` here too.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
