zeroshade commented on code in PR #2094:
URL: https://github.com/apache/iceberg-go/pull/2094#discussion_r4198575777


##########
table/scan_planning.go:
##########
@@ -53,6 +55,70 @@ const (
        ScanPlanningAuto ScanPlanningMode = "auto"
 )
 
+// ScanPlanningModeKey is the REST table-config key a catalog uses to tell
+// clients which planning mode a table supports. Valid values are `client` and
+// `server`.
+const ScanPlanningModeKey = "scan-planning-mode"
+
+// ScanPlanningDirective is the catalog's `scan-planning-mode` table-config
+// value. It is distinct from ScanPlanningMode, the user-facing scan option.
+type ScanPlanningDirective string
+
+const (
+       // ScanPlanningDirectiveUnknown is the zero-value string denoting an 
unknown,
+       // unrestricted or missing scan planning directive.
+       ScanPlanningDirectiveUnknown ScanPlanningDirective = ""
+       // ScanPlanningDirectiveClient indicates the catalog requires the 
client to
+       // plan scans locally.
+       ScanPlanningDirectiveClient ScanPlanningDirective = "client"
+       // ScanPlanningDirectiveServer indicates the catalog requires scans to 
be
+       // planned remotely by the catalog server.
+       ScanPlanningDirectiveServer ScanPlanningDirective = "server"
+)
+
+// ScanPlanningDirective returns the catalog's `scan-planning-mode` directive
+// taken from the `config` block of the table-load response. Table metadata
+// properties and catalog properties are not consulted. Values are matched
+// case-insensitively.
+//
+// It returns an empty directive and a nil error when the catalog supplied no
+// directive, and an error wrapping iceberg.ErrInvalidArgument when the
+// directive holds an unrecognized value.
+//
+// The directive is only known for tables built from a load response
+// (LoadTable, CreateTable, RegisterTable, and their refreshes). Tables 
returned
+// by a catalog's UpdateTable report no directive, because commit responses do
+// not carry table config; reload the table to obtain it.

Review Comment:
   Nit: tables returned by `Transaction.Commit` and `StagedTable` keep the 
source table's directive. This paragraph lists only the load paths and then 
says `UpdateTable` drops it, which reads as if every commit result loses it. 
Worth saying explicitly:
   
   ```suggestion
   // The directive is only known for tables built from a load response
   // (LoadTable, CreateTable, RegisterTable, and their refreshes); tables
   // returned by Transaction.Commit or StagedTable keep the directive of the
   // table they were derived from. Tables returned by a catalog's UpdateTable
   // report no directive, because commit responses do not carry table config;
   // reload the table to obtain it.
   ```



-- 
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]

Reply via email to