huaxingao commented on code in PR #16961:
URL: https://github.com/apache/iceberg/pull/16961#discussion_r3994871090


##########
format/index-spec.md:
##########
@@ -0,0 +1,734 @@
+---
+title: "Index Spec"
+---
+<!--
+ - 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.
+ -->
+# Iceberg Index Specification
+
+## Background and Motivation
+
+An index is most valuable when it is a property of the table rather than of 
the engine that built it. This
+specification defines a common format for index metadata and a common storage 
architecture for index data, so that any
+engine can build an index, maintain it, and use it to plan queries against the 
table.
+
+## Goals
+
+* **Portability** -- An index written by one engine will be readable by any 
other engine.
+* **Separation** -- Index metadata will be committed separately from table 
metadata. Building and maintaining an index
+  will not rewrite the table.
+* **Optionality** -- Indexes will be optional. Engines may ignore an index 
they do not support.
+* **Consistency** -- Each index snapshot will index exactly the live rows of 
one source table snapshot.
+
+## Overview
+
+An index is recorded in an index metadata file that contains the index 
definition and a set of index snapshots. Each
+index snapshot corresponds to a snapshot of the source table and references 
the index data for that state.
+
+Index metadata files and index data files are immutable. Every update writes a 
new metadata file and a new tracking

Review Comment:
   Only the add-a-snapshot case writes a tracking file — line 254 also counts 
dropping a snapshot and changing properties as updates, and neither does.
   
   ```suggestion
   Index metadata files and index data files are immutable. Every update writes 
a new metadata file. An update that adds an
   index snapshot also writes a new tracking file and may reuse existing range 
files. Every update is committed by an
   atomic swap of the index metadata file, as defined in [Commits and 
Concurrency](#commits-and-concurrency).
   ```



##########
format/index-spec.md:
##########
@@ -0,0 +1,734 @@
+---
+title: "Index Spec"
+---
+<!--
+ - 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.
+ -->
+# Iceberg Index Specification
+
+## Background and Motivation
+
+An index is most valuable when it is a property of the table rather than of 
the engine that built it. This
+specification defines a common format for index metadata and a common storage 
architecture for index data, so that any
+engine can build an index, maintain it, and use it to plan queries against the 
table.
+
+## Goals
+
+* **Portability** -- An index written by one engine will be readable by any 
other engine.
+* **Separation** -- Index metadata will be committed separately from table 
metadata. Building and maintaining an index
+  will not rewrite the table.
+* **Optionality** -- Indexes will be optional. Engines may ignore an index 
they do not support.
+* **Consistency** -- Each index snapshot will index exactly the live rows of 
one source table snapshot.
+
+## Overview
+
+An index is recorded in an index metadata file that contains the index 
definition and a set of index snapshots. Each
+index snapshot corresponds to a snapshot of the source table and references 
the index data for that state.
+
+Index metadata files and index data files are immutable. Every update writes a 
new metadata file and a new tracking
+file, may reuse existing range files, and is committed by an atomic swap of 
the index metadata file, as defined in
+[Commits and Concurrency](#commits-and-concurrency).
+
+The index data of a snapshot is organized as a [tracking file](#tracking-file) 
that lists a set of
+[range files](#range-files):
+
+```text
+Index Metadata
+    |
+    +-- Index Snapshot(s)
+            |
+            +-- Tracking File
+                    |
+                    +-- Range Files
+```
+
+## Specification
+
+### Terms
+
+* **Index** -- A structure that accelerates retrieval of rows from a source 
table.
+* **Index snapshot** -- The state of an index for a single snapshot of the 
source table.
+* **Index entry** -- The values produced by the index fields for one indexed 
row of the source table.
+* **Clustering key** -- The tuple of values that determines the position of an 
index entry within an index snapshot.
+* **Tracking file** -- A file that lists the range files of an index snapshot; 
one per index snapshot.
+* **Range file** -- A file that stores the index entries for a range of 
clustering keys; a subset of an index snapshot.
+
+### Paths in Metadata
+
+Path strings stored in index metadata are classified and resolved as defined by
+[paths in metadata](spec.md#paths-in-metadata) in the table specification. 
Relative paths are resolved against the
+index `location`, which must be an absolute path.
+
+### Index Definition
+
+An index is defined by a source table, an index type, identity fields, 
materialized fields, non-materialized fields, a
+cluster spec, and optional index properties. The definition is fixed when the 
index is created and must not change for
+the lifetime of the index, so range files remain readable through every index 
snapshot that references them. A different
+definition requires a new index.
+
+A table may have multiple indexes of the same index type.
+
+#### Index Type
+
+The index type defines the logical category of an index and the class of 
queries it accelerates.
+
+| Type     | Description                                                       
                                                    |
+|----------|-----------------------------------------------------------------------------------------------------------------------|
+| `SCALAR` | Accelerates point lookups on clustered fields, and range filters 
when the clustering expressions are order preserving |
+
+This specification defines a single index type, `SCALAR`. Future 
specifications may define additional types, see
+[Future Extensions](#future-extensions).
+
+Writers must write `type` in upper case. Readers must match it 
case-insensitively. A reader that does not implement an
+index type must ignore the index and read the source table directly; it must 
not fail.
+
+#### Index Fields
+
+An index field defines one value of an index entry, produced for an indexed 
row of the source table. An index declares
+three lists of index fields: [identity fields](#identity-fields) and 
[materialized fields](#materialized-fields), whose
+values are stored in [range files](#range-files), and [non-materialized 
fields](#non-materialized-fields), which are
+represented only by statistics in [tracking file 
entries](#tracking-file-entry). Every index field has a field ID that
+must be unique across the three lists.
+
+#### Identity Fields
+
+`identity-fields` is a non-empty list of unique source table field IDs. Each 
entry must reference a data field.
+[Metadata columns](spec.md#reserved-field-ids) are not allowed. Each listed 
field is stored in the
+[range files](#range-files) under its own field ID and takes its type from the 
schema of the source table snapshot that
+an index snapshot references.
+
+Every source table field referenced by an expression field in the [cluster 
spec](#cluster-spec) must be an identity
+field.
+
+#### Expression Fields
+
+Both [materialized fields](#materialized-fields) and [non-materialized 
fields](#non-materialized-fields) are expression
+fields; they differ only in where their values are kept.
+
+The value of an expression field is produced by evaluating an
+[Iceberg value expression](expressions-spec.md#value-expressions) for an 
indexed row of the source table.
+An expression field has the following fields:
+
+| Requirement | Field name    | Type              | Description                
                                  |
+|-------------|---------------|-------------------|--------------------------------------------------------------|
+| _required_  | `field-id`    | `int`             | ID that uniquely 
identifies the index field                  |
+| _required_  | `type`        | `expr-value`      | Expression field 
representation                              |
+| _required_  | `data-type`   | Iceberg type      | Type produced by the 
expression                              |
+| _required_  | `expr`        | JSON expression   | Value expression that 
produces the field, serialized as JSON |
+
+Each expression field must satisfy the following requirements:
+
+- `expr` must contain only ID references to source table fields or
+  [metadata columns](spec.md#reserved-field-ids). Named references must not be 
used. The `_deleted`, `_change_type`,
+  `_change_ordinal`, and `_commit_snapshot_id` metadata columns must not be 
referenced, and neither must the
+  `file_path`, `pos`, and `row` columns of delete files.
+- `expr` must be deterministic and must produce the declared `data-type`.
+- `field-id` must not be a [reserved field ID](spec.md#reserved-field-ids).
+
+Expressions are serialized using the [JSON 
serialization](expressions-spec.md#appendix-b-json-serialization) defined by
+the expressions specification. Types are serialized using the [type 
serialization](spec.md#schemas) defined by the table
+specification.
+
+##### Materialized Fields
+
+`materialized-fields` is a list of expression fields whose values are stored 
in the [range files](#range-files).
+Evaluating the identity fields and the materialized fields for one indexed row 
produces one range file row.
+
+##### Non-Materialized Fields
+
+`non-materialized-fields` is a list of expression fields whose row values are 
not stored in range files. Only their
+field statistics are stored, in [tracking file entries](#tracking-file-entry).
+
+#### Cluster Spec
+
+`cluster-spec` is a list of field IDs from `identity-fields`, 
`materialized-fields`, and `non-materialized-fields`. The
+values of the referenced fields, in list order, form the clustering key of an 
indexed row and determine the row's
+position in the index, as defined in [Clustering and 
Ordering](#clustering-and-ordering). The list must not be empty.
+Every referenced field must have a primitive type.
+
+### Index Metadata
+
+The index metadata file stores the index definition and snapshot history. It 
is encoded as JSON.
+
+#### Index Metadata File
+
+The index metadata file has the following fields:
+
+| Requirement | Field name                | Type                       | 
Description                                                                     
                   |
+|-------------|---------------------------|----------------------------|----------------------------------------------------------------------------------------------------|
+| _required_  | `format-version`          | `int`                      | Index 
format version; must be `1`                                                     
             |
+| _required_  | `index-uuid`              | `string`                   | 
Stable UUID assigned at creation                                                
                   |
+| _required_  | `table-uuid`              | `string`                   | UUID 
of the indexed table                                                            
              |
+| _required_  | `location`                | `string`                   | Index 
root location                                                                   
             |
+| _required_  | `last-updated-ms`         | `long`                     | 
Timestamp when the index was last updated (ms from epoch) [1]                   
                   |
+| _required_  | `type`                    | `string`                   | 
Logical index type                                                              
                   |
+| _required_  | `identity-fields`         | `list<int>`                | 
Source table fields stored in range files, see [Identity 
Fields](#identity-fields)                 |
+| _optional_  | `materialized-fields`     | `list<expression-field>`   | 
Expression fields stored in range files, see [Materialized 
Fields](#materialized-fields)           |
+| _optional_  | `non-materialized-fields` | `list<expression-field>`   | 
Fields stored only in tracking statistics, see [Non-Materialized 
Fields](#non-materialized-fields) |
+| _required_  | `cluster-spec`            | `list<int>`                | Field 
IDs that define clustering, see [Cluster Spec](#cluster-spec)                   
             |
+| _optional_  | `properties`              | `map<string, string>`      | Index 
properties applicable for every snapshot                                        
             |
+| _optional_  | `snapshots`               | `list<index-snapshot>`     | Index 
snapshots [2]                                                                   
             |
+| _optional_  | `metadata-log`            | `list<metadata-log-entry>` | 
Previous index metadata files, see [Metadata Log](#metadata-log)                
                   |
+| _optional_  | `encryption-keys`         | `list<encryption-key>`     | 
Encryption keys used by the index, see [Encryption Keys](#encryption-keys)      
                   |
+
+A missing optional list must be read as an empty list.
+
+Notes:
+
+1. Each index metadata file should update `last-updated-ms` just before 
writing.
+2. An index that has not been built yet has no snapshots.
+3. Index names are not stored in index metadata. It is the catalog's 
responsibility to map index names to metadata file
+   locations.
+4. How the indexes of a table are discovered is out of scope for this 
specification and is defined by the catalog
+   specification.
+
+#### Index Snapshot
+
+An index snapshot is an immutable version of the index data generated from a 
specific source table snapshot. It
+references a complete set of index files through the location of a single 
[tracking file](#tracking-file).
+
+An index snapshot must index exactly the live rows of the referenced table 
snapshot.
+
+| Requirement | Field name                 | Type                  | 
Description                                                                  |
+|-------------|----------------------------|-----------------------|------------------------------------------------------------------------------|
+| _required_  | `snapshot-id`              | `long`                | Index 
snapshot identifier                                                    |
+| _required_  | `source-table-snapshot-id` | `long`                | Source 
table snapshot                                                        |
+| _required_  | `timestamp-ms`             | `long`                | Timestamp 
when the index snapshot was created (ms from epoch)                |
+| _required_  | `tracking-file`            | `string`              | Location 
of the tracking file                                                |
+| _optional_  | `properties`               | `map<string, string>` | Snapshot 
properties specific to this snapshot                                |
+| _optional_  | `key-id`                   | `string`              | ID of the 
encryption key that holds the tracking file key metadata           |
+
+Each `snapshot-id` must be unique within the `snapshots` list. Engines locate 
index data by matching
+`source-table-snapshot-id`. More than one index snapshot may reference the 
same source table snapshot, and an engine may
+use any of the matching index snapshots.
+
+#### Metadata Log
+
+`metadata-log` records the index metadata files that preceded the current one. 
A commit should append an entry for the
+metadata file it replaces. The number of entries to retain is controlled by 
the index property
+`write.metadata.previous-versions-max`, and a commit drops the oldest entries 
beyond that limit. When
+`write.metadata.delete-after-commit.enabled` is true, a commit also deletes 
the dropped metadata files.
+
+| Requirement | Field name      | Type     | Description                       
                              |
+|-------------|-----------------|----------|-----------------------------------------------------------------|
+| _required_  | `metadata-file` | `string` | Location of the index metadata 
file                             |
+| _required_  | `timestamp-ms`  | `long`   | `last-updated-ms` of the index 
metadata file at `metadata-file` |
+
+#### Encryption Keys
+
+Index metadata is not encrypted, so keys are never stored in plain form. Keys 
used for index encryption are tracked in
+index metadata as a list named `encryption-keys`, using the same structure as 
the table specification (see
+[Encryption Keys](spec.md#encryption-keys)). The schema of each key is a 
struct with the following fields:
+

Review Comment:
   The field table below mirrors the one in the linked `spec.md` section.  
could the table be dropped and use cross-reference?
   
   Separately, nothing relates index encryption to table encryption. As 
written, an engine may build an unencrypted index over an encrypted table, 
putting indexed values in plaintext range files outside the table's protection 
— a direct leak for a covering index. Maybe add:
   ```
   An index must not store indexed values with weaker protection than its 
source table. If the source table uses table encryption as defined by the table 
specification, the tracking files and range files of every index snapshot must 
be encrypted.
   ```



##########
format/index-spec.md:
##########
@@ -0,0 +1,734 @@
+---
+title: "Index Spec"
+---
+<!--
+ - 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.
+ -->
+# Iceberg Index Specification
+
+## Background and Motivation
+
+An index is most valuable when it is a property of the table rather than of 
the engine that built it. This
+specification defines a common format for index metadata and a common storage 
architecture for index data, so that any
+engine can build an index, maintain it, and use it to plan queries against the 
table.
+
+## Goals
+
+* **Portability** -- An index written by one engine will be readable by any 
other engine.
+* **Separation** -- Index metadata will be committed separately from table 
metadata. Building and maintaining an index
+  will not rewrite the table.
+* **Optionality** -- Indexes will be optional. Engines may ignore an index 
they do not support.
+* **Consistency** -- Each index snapshot will index exactly the live rows of 
one source table snapshot.
+
+## Overview
+
+An index is recorded in an index metadata file that contains the index 
definition and a set of index snapshots. Each
+index snapshot corresponds to a snapshot of the source table and references 
the index data for that state.
+
+Index metadata files and index data files are immutable. Every update writes a 
new metadata file and a new tracking
+file, may reuse existing range files, and is committed by an atomic swap of 
the index metadata file, as defined in
+[Commits and Concurrency](#commits-and-concurrency).
+
+The index data of a snapshot is organized as a [tracking file](#tracking-file) 
that lists a set of
+[range files](#range-files):
+
+```text
+Index Metadata
+    |
+    +-- Index Snapshot(s)
+            |
+            +-- Tracking File
+                    |
+                    +-- Range Files
+```
+
+## Specification
+
+### Terms
+
+* **Index** -- A structure that accelerates retrieval of rows from a source 
table.
+* **Index snapshot** -- The state of an index for a single snapshot of the 
source table.
+* **Index entry** -- The values produced by the index fields for one indexed 
row of the source table.
+* **Clustering key** -- The tuple of values that determines the position of an 
index entry within an index snapshot.
+* **Tracking file** -- A file that lists the range files of an index snapshot; 
one per index snapshot.
+* **Range file** -- A file that stores the index entries for a range of 
clustering keys; a subset of an index snapshot.

Review Comment:
   I thought we decided `region file`?



##########
format/index-spec.md:
##########
@@ -0,0 +1,734 @@
+---
+title: "Index Spec"
+---
+<!--
+ - 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.
+ -->
+# Iceberg Index Specification
+
+## Background and Motivation
+
+An index is most valuable when it is a property of the table rather than of 
the engine that built it. This
+specification defines a common format for index metadata and a common storage 
architecture for index data, so that any
+engine can build an index, maintain it, and use it to plan queries against the 
table.
+
+## Goals
+
+* **Portability** -- An index written by one engine will be readable by any 
other engine.
+* **Separation** -- Index metadata will be committed separately from table 
metadata. Building and maintaining an index
+  will not rewrite the table.
+* **Optionality** -- Indexes will be optional. Engines may ignore an index 
they do not support.
+* **Consistency** -- Each index snapshot will index exactly the live rows of 
one source table snapshot.
+
+## Overview
+
+An index is recorded in an index metadata file that contains the index 
definition and a set of index snapshots. Each
+index snapshot corresponds to a snapshot of the source table and references 
the index data for that state.
+
+Index metadata files and index data files are immutable. Every update writes a 
new metadata file and a new tracking
+file, may reuse existing range files, and is committed by an atomic swap of 
the index metadata file, as defined in
+[Commits and Concurrency](#commits-and-concurrency).
+
+The index data of a snapshot is organized as a [tracking file](#tracking-file) 
that lists a set of
+[range files](#range-files):
+
+```text
+Index Metadata
+    |
+    +-- Index Snapshot(s)
+            |
+            +-- Tracking File
+                    |
+                    +-- Range Files
+```
+
+## Specification
+
+### Terms
+
+* **Index** -- A structure that accelerates retrieval of rows from a source 
table.
+* **Index snapshot** -- The state of an index for a single snapshot of the 
source table.
+* **Index entry** -- The values produced by the index fields for one indexed 
row of the source table.
+* **Clustering key** -- The tuple of values that determines the position of an 
index entry within an index snapshot.
+* **Tracking file** -- A file that lists the range files of an index snapshot; 
one per index snapshot.
+* **Range file** -- A file that stores the index entries for a range of 
clustering keys; a subset of an index snapshot.
+
+### Paths in Metadata
+
+Path strings stored in index metadata are classified and resolved as defined by
+[paths in metadata](spec.md#paths-in-metadata) in the table specification. 
Relative paths are resolved against the
+index `location`, which must be an absolute path.
+
+### Index Definition
+
+An index is defined by a source table, an index type, identity fields, 
materialized fields, non-materialized fields, a
+cluster spec, and optional index properties. The definition is fixed when the 
index is created and must not change for

Review Comment:
   `optional index properties` is part of the definition here, and the 
definition "must not change for the lifetime of the index" — but line 254 lists 
"changing index properties" as a normal update. 
   
   Should we drop properties from the definition and noting that they may be 
changed?



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