zeroshade commented on code in PR #1229:
URL: https://github.com/apache/iceberg-go/pull/1229#discussion_r3444278034
##########
table/arrow_scanner.go:
##########
@@ -251,6 +252,158 @@ func sameDVBlob(a, b iceberg.DataFile) bool {
return *a.ContentOffset() == *b.ContentOffset()
}
+type releasableScalar interface {
+ scalar.Scalar
+ scalar.Releasable
+}
+
+func filePathScalar(v string, dt arrow.DataType) releasableScalar {
+ if dictType, ok := dt.(*arrow.DictionaryType); ok {
+ dt = dictType.ValueType
+ }
+
+ if dt.ID() == arrow.LARGE_STRING {
+ return scalar.NewLargeStringScalar(v)
+ }
+
+ return scalar.NewStringScalar(v)
+}
+
+type stringArrayValues interface {
+ Len() int
+ IsNull(int) bool
+ Value(int) string
+}
Review Comment:
Just use `array.StringLike`
(https://github.com/apache/arrow-go/blob/main/arrow/array/string.go#L32)
##########
table/arrow_scanner.go:
##########
@@ -251,6 +252,158 @@ func sameDVBlob(a, b iceberg.DataFile) bool {
return *a.ContentOffset() == *b.ContentOffset()
}
+type releasableScalar interface {
+ scalar.Scalar
+ scalar.Releasable
+}
+
+func filePathScalar(v string, dt arrow.DataType) releasableScalar {
+ if dictType, ok := dt.(*arrow.DictionaryType); ok {
+ dt = dictType.ValueType
+ }
+
+ if dt.ID() == arrow.LARGE_STRING {
+ return scalar.NewLargeStringScalar(v)
+ }
+
+ return scalar.NewStringScalar(v)
+}
+
+type stringArrayValues interface {
+ Len() int
+ IsNull(int) bool
+ Value(int) string
+}
+
+func addDistinctFilePath(seen map[string]struct{}, paths *[]string, v string) {
+ if _, ok := seen[v]; ok {
+ return
+ }
+
+ seen[v] = struct{}{}
+ *paths = append(*paths, v)
+}
+
+func collectFilePathsFromStringArray(
+ seen map[string]struct{},
+ paths *[]string,
+ values stringArrayValues,
+ chunkIdx int,
+) error {
+ for i := 0; i < values.Len(); i++ {
+ if values.IsNull(i) {
+ return fmt.Errorf("%w: null file_path in position
delete chunk %d row %d",
+ iceberg.ErrInvalidSchema, chunkIdx, i)
+ }
+
+ addDistinctFilePath(seen, paths, values.Value(i))
+ }
+
+ return nil
+}
+
+func collectFilePathsFromDictionary(
+ seen map[string]struct{},
+ paths *[]string,
+ arr *array.Dictionary,
+ values stringArrayValues,
+ chunkIdx int,
+) error {
+ for i := 0; i < arr.Len(); i++ {
+ if arr.IsNull(i) {
+ return fmt.Errorf("%w: null file_path in position
delete chunk %d row %d",
+ iceberg.ErrInvalidSchema, chunkIdx, i)
+ }
+
+ dictIdx := arr.GetValueIndex(i)
+ if values.IsNull(dictIdx) {
+ return fmt.Errorf("%w: null file_path dictionary value
in position delete chunk %d row %d",
+ iceberg.ErrInvalidSchema, chunkIdx, i)
+ }
+
+ addDistinctFilePath(seen, paths, values.Value(dictIdx))
+ }
Review Comment:
the same UniqueArray function works on dictionaries too! 😄
##########
table/arrow_scanner.go:
##########
@@ -251,6 +252,158 @@ func sameDVBlob(a, b iceberg.DataFile) bool {
return *a.ContentOffset() == *b.ContentOffset()
}
+type releasableScalar interface {
+ scalar.Scalar
+ scalar.Releasable
+}
+
+func filePathScalar(v string, dt arrow.DataType) releasableScalar {
+ if dictType, ok := dt.(*arrow.DictionaryType); ok {
+ dt = dictType.ValueType
+ }
+
+ if dt.ID() == arrow.LARGE_STRING {
+ return scalar.NewLargeStringScalar(v)
+ }
+
+ return scalar.NewStringScalar(v)
+}
+
+type stringArrayValues interface {
+ Len() int
+ IsNull(int) bool
+ Value(int) string
+}
+
+func addDistinctFilePath(seen map[string]struct{}, paths *[]string, v string) {
+ if _, ok := seen[v]; ok {
+ return
+ }
+
+ seen[v] = struct{}{}
+ *paths = append(*paths, v)
+}
+
+func collectFilePathsFromStringArray(
+ seen map[string]struct{},
+ paths *[]string,
+ values stringArrayValues,
+ chunkIdx int,
+) error {
+ for i := 0; i < values.Len(); i++ {
+ if values.IsNull(i) {
+ return fmt.Errorf("%w: null file_path in position
delete chunk %d row %d",
+ iceberg.ErrInvalidSchema, chunkIdx, i)
+ }
+
+ addDistinctFilePath(seen, paths, values.Value(i))
+ }
+
+ return nil
+}
Review Comment:
Could you just use the
[`compute.UniqueArray`](https://pkg.go.dev/github.com/apache/arrow-go/v18/arrow/compute#UniqueArray)
function instead?
##########
table/arrow_scanner.go:
##########
@@ -277,29 +430,8 @@ func readDeletes(ctx context.Context, fs iceio.IO,
dataFile iceberg.DataFile) (_
filePathCol :=
tbl.Column(tbl.Schema().FieldIndices("file_path")[0]).Data()
posCol := tbl.Column(tbl.Schema().FieldIndices("pos")[0]).Data()
- dict :=
filePathCol.Chunk(0).(*array.Dictionary).Dictionary().(*array.String)
Review Comment:
I think that `compute.Unique` will always output a dictionary, which means
that you could simply just do something like this:
```go
result, err := compute.Unique(compute.NewDatumWithoutOwning(filePathCol))
if err != nil {
return nil, err
}
defer result.Release()
dictChunks := result.(compute.ArrayLikeDatum).Chunks()
for _, c := range dictChunks {
arr, err := array.NewDictWrapper[string](c.(*array.Dictionary))
if err != nil {
return nil, fmt.Errorf("file path col not string: %w", err)
}
for i := range arr.Len() {
v := arr.Value(i)
....
}
}
```
that would probably simplify this a lot while still achieving your goal.
##########
table/arrow_scanner.go:
##########
@@ -251,6 +252,158 @@ func sameDVBlob(a, b iceberg.DataFile) bool {
return *a.ContentOffset() == *b.ContentOffset()
}
+type releasableScalar interface {
+ scalar.Scalar
+ scalar.Releasable
+}
+
+func filePathScalar(v string, dt arrow.DataType) releasableScalar {
+ if dictType, ok := dt.(*arrow.DictionaryType); ok {
+ dt = dictType.ValueType
+ }
+
+ if dt.ID() == arrow.LARGE_STRING {
+ return scalar.NewLargeStringScalar(v)
+ }
+
+ return scalar.NewStringScalar(v)
+}
+
+type stringArrayValues interface {
+ Len() int
+ IsNull(int) bool
+ Value(int) string
+}
+
+func addDistinctFilePath(seen map[string]struct{}, paths *[]string, v string) {
+ if _, ok := seen[v]; ok {
+ return
+ }
+
+ seen[v] = struct{}{}
+ *paths = append(*paths, v)
+}
+
+func collectFilePathsFromStringArray(
+ seen map[string]struct{},
+ paths *[]string,
+ values stringArrayValues,
+ chunkIdx int,
+) error {
+ for i := 0; i < values.Len(); i++ {
+ if values.IsNull(i) {
+ return fmt.Errorf("%w: null file_path in position
delete chunk %d row %d",
+ iceberg.ErrInvalidSchema, chunkIdx, i)
+ }
+
+ addDistinctFilePath(seen, paths, values.Value(i))
+ }
+
+ return nil
+}
+
+func collectFilePathsFromDictionary(
+ seen map[string]struct{},
+ paths *[]string,
+ arr *array.Dictionary,
+ values stringArrayValues,
+ chunkIdx int,
+) error {
+ for i := 0; i < arr.Len(); i++ {
+ if arr.IsNull(i) {
+ return fmt.Errorf("%w: null file_path in position
delete chunk %d row %d",
+ iceberg.ErrInvalidSchema, chunkIdx, i)
+ }
+
+ dictIdx := arr.GetValueIndex(i)
+ if values.IsNull(dictIdx) {
+ return fmt.Errorf("%w: null file_path dictionary value
in position delete chunk %d row %d",
+ iceberg.ErrInvalidSchema, chunkIdx, i)
+ }
+
+ addDistinctFilePath(seen, paths, values.Value(dictIdx))
+ }
+
+ return nil
+}
+
+func distinctPosDeleteFilePaths(filePathCol *arrow.Chunked) ([]string, error) {
Review Comment:
yea, this entire function can be replaced with just calling
[`compute.Unique(compute.NewDatumWithoutOwning(filePathCol))`](https://pkg.go.dev/github.com/apache/arrow-go/v18/arrow/compute#Unique)
and using the resulting array which would contain the unique paths
--
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]
