Skip to content

Conversation

@Xuanwo
Copy link
Member

@Xuanwo Xuanwo commented Mar 16, 2023

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

refactor: Merge FileWithMetadata into StageFileInfo

Signed-off-by: Xuanwo <github@xuanwo.io>
@vercel
Copy link

vercel bot commented Mar 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
databend ⬜️ Ignored (Inspect) Mar 16, 2023 at 2:01PM (UTC)

@Xuanwo Xuanwo requested review from BohuTANG and youngsofun March 16, 2023 13:55
@mergify mergify bot added the pr-refactor this PR changes the code base without new features or bugfix label Mar 16, 2023
Signed-off-by: Xuanwo <github@xuanwo.io>
@youngsofun
Copy link
Member

youngsofun commented Mar 16, 2023

I prefer turn Metadata into StageFileInfo only when necessary.
for example, "select from stage" not need metadata other than size and is_dir/is_file.
we can list with minimal metadata fetched, but still carry a MetaData object.
(size is for read meta of parquet, not actually used for now, due to limitation of arrow2 interface)

StageFileInfo with all metas is only used when 'color' a file, and list stage

MetaKey should be decided by diff apps.


Rest LGTM.

@Xuanwo
Copy link
Member Author

Xuanwo commented Mar 16, 2023

I prefer turn Metadata into StageFileInfo only when necessary.

Great catch! I understand and agree with your suggestion to turn Metadata into StageFileInfo when necessary. However, I will need some time to review the logic and come up with a better solution.

As this pull request does not have any negative impact (since we used to receive full metadata before), how about merging it first and then refining it later?

@BohuTANG
Copy link
Member

Will merge, then we can refine it in another PR.

@BohuTANG BohuTANG merged commit 49da701 into databendlabs:main Mar 16, 2023
@Xuanwo Xuanwo deleted the refactor-not-store-opendal-metadata branch March 16, 2023 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-refactor this PR changes the code base without new features or bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants