Fokko commented on code in PR #108:
URL: https://github.com/apache/iceberg-go/pull/108#discussion_r1675971993
##########
exprs.go:
##########
@@ -538,11 +557,11 @@ func (up *unboundUnaryPredicate) Bind(schema *Schema,
caseSensitive bool) (Boole
// fast case optimizations
switch up.op {
case OpIsNull:
- if bound.Ref().Field().Required {
+ if bound.Ref().Field().Required &&
!schema.FieldHasOptionalParent(bound.Ref().Field().ID) {
Review Comment:
Hmm, is it possible to write a field, where the parent is null?
##########
schema.go:
##########
@@ -885,6 +919,66 @@ func (findLastFieldID) Map(_ MapType, keyResult,
valueResult int) int {
func (findLastFieldID) Primitive(PrimitiveType) int { return 0 }
+// IndexParents generates an index of field IDs to their parent field
+// IDs. Root fields are not indexed
+func IndexParents(schema *Schema) (map[int]int, error) {
+ indexer := &indexParents{
+ idToParent: make(map[int]int),
+ idStack: make([]int, 0),
+ }
+ return Visit(schema, indexer)
+}
+
+type indexParents struct {
+ idToParent map[int]int
+ idStack []int
+}
+
+func (i *indexParents) BeforeField(field NestedField) {
+ i.idStack = append(i.idStack, field.ID)
+}
+
+func (i *indexParents) AfterField(field NestedField) {
+ i.idStack = i.idStack[:len(i.idStack)-1]
Review Comment:
Interesting:
https://stackoverflow.com/questions/52546470/whats-the-go-idiom-for-pythons-list-pop-method
##########
table/metadata.go:
##########
@@ -156,6 +166,42 @@ type commonMetadata struct {
Refs map[string]SnapshotRef `json:"refs"`
}
+func (c *commonMetadata) Equals(other *commonMetadata) bool {
+ switch {
+ case c.LastPartitionID == nil && other.LastPartitionID != nil:
+ fallthrough
+ case c.LastPartitionID != nil && other.LastPartitionID == nil:
+ fallthrough
+ case c.CurrentSnapshotID == nil && other.CurrentSnapshotID != nil:
+ fallthrough
+ case c.CurrentSnapshotID != nil && other.CurrentSnapshotID == nil:
+ return false
+ }
+
+ switch {
+ case !sliceEqualHelper(c.SchemaList, other.SchemaList):
+ fallthrough
+ case !sliceEqualHelper(c.SnapshotList, other.SnapshotList):
+ fallthrough
+ case !sliceEqualHelper(c.Specs, other.Specs):
+ fallthrough
+ case !maps.Equal(c.Props, other.Props):
Review Comment:
Should we add the sort-order here as well?
##########
table/metadata.go:
##########
@@ -156,6 +166,42 @@ type commonMetadata struct {
Refs map[string]SnapshotRef `json:"refs"`
}
+func (c *commonMetadata) Equals(other *commonMetadata) bool {
Review Comment:
This might cut it for now, but I'm unsure if this is sufficient later on.
For example, in the in-memory representation of PyIceberg we don't use the
`spec` field, but only the V2 `specs`. If we detect a `spec` using parsing, we
move that one to `specs` and set the `default-partition-spec-id`. This makes
the metadata the same, but they are not strickly equal.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
