kevinjqliu commented on code in PR #339:
URL: https://github.com/apache/iceberg-go/pull/339#discussion_r2001731351
##########
table/metadata.go:
##########
@@ -567,14 +568,23 @@ func (b *MetadataBuilder) SetSnapshotRef(
return nil, fmt.Errorf("can't set snapshot ref %s to unknown
snapshot %d: %w", name, snapshotID, err)
}
+ isAddedSnapshot := slices.ContainsFunc(b.updates, func(u Update) bool {
+ return u.Action() == addSnapshotAction &&
u.(*addSnapshotUpdate).Snapshot.SnapshotID == snapshotID
+ })
+ if isAddedSnapshot {
+ b.lastUpdatedMS = snapshot.TimestampMs
+ }
+
if name == MainBranch {
b.updates = append(b.updates, NewSetSnapshotRefUpdate(name,
snapshotID, refType, maxRefAgeMs, maxSnapshotAgeMs, minSnapshotsToKeep))
b.currentSnapshotID = &snapshotID
+ if !isAddedSnapshot {
+ b.lastUpdatedMS = time.Now().Local().UnixMilli()
Review Comment:
nit: other places use `time.Now().UnixMilli()`
##########
table/transaction.go:
##########
@@ -178,14 +179,39 @@ func (t *Transaction) AddFiles(files []string,
snapshotProps iceberg.Properties,
return t.apply(updates, reqs)
}
-func (t *Transaction) StagedTable() (*Table, error) {
+func (t *Transaction) Scan(opts ...ScanOption) (*Scan, error) {
updatedMeta, err := t.meta.Build()
if err != nil {
return nil, err
}
- return New(t.tbl.identifier, updatedMeta,
- updatedMeta.Location(), t.tbl.fs, t.tbl.cat), nil
+ s := &Scan{
+ metadata: updatedMeta,
+ io: t.tbl.fs,
+ rowFilter: iceberg.AlwaysTrue{},
+ selectedFields: []string{"*"},
+ caseSensitive: true,
+ limit: ScanNoLimit,
Review Comment:
assuming these will be part of ScanOption in the future
##########
catalog/rest/rest.go:
##########
@@ -773,6 +773,9 @@ func (r *Catalog) CreateTable(ctx context.Context,
identifier table.Identifier,
func (r *Catalog) CommitTable(ctx context.Context, tbl *table.Table,
requirements []table.Requirement, updates []table.Update) (table.Metadata,
string, error) {
ident := tbl.Identifier()
+ if ident[0] == r.name {
+ ident = ident[1:]
+ }
Review Comment:
Catalog name can be arbitrary, its not a good idea to include it as part of
the table identifier. In pyiceberg, we had to deprecate this.
##########
catalog/internal/utils.go:
##########
@@ -61,3 +64,57 @@ func WriteMetadata(ctx context.Context, metadata
table.Metadata, loc string, pro
return json.NewEncoder(out).Encode(metadata)
}
+
+func UpdateTableMetadata(base table.Metadata, updates []table.Update,
metadataLoc string) (table.Metadata, error) {
+ bldr, err := table.MetadataBuilderFromBase(base)
+ if err != nil {
+ return nil, err
+ }
+
+ for _, update := range updates {
+ if err := update.Apply(bldr); err != nil {
+ return nil, err
+ }
+ }
+
+ if bldr.HasChanges() {
+ if metadataLoc != "" {
+ maxMetadataLogEntries := max(1,
+ base.Properties().GetInt(
+ table.MetadataPreviousVersionsMaxKey,
table.MetadataPreviousVersionsMaxDefault))
+
+ bldr.TrimMetadataLogs(maxMetadataLogEntries + 1).
+ AppendMetadataLog(table.MetadataLogEntry{
+ MetadataFile: metadataLoc,
+ TimestampMs: base.LastUpdatedMillis(),
+ })
+ }
+ if base.LastUpdatedMillis() == bldr.LastUpdatedMS() {
+ bldr.SetLastUpdatedMS()
+ }
+ }
+
+ return bldr.Build()
+}
+
+var tableMetadataFileNameRegex =
regexp.MustCompile(`^(\d+)-([\w-]{36})(?:\.\w+)?\.metadata\.json`)
Review Comment:
nit: i like the explanation of the regex here
https://github.com/apache/iceberg-python/blob/a403c65f9e958ee17b95cd08baf70e15002c1225/pyiceberg/catalog/__init__.py#L101-L110
--
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]
