gravitino icon indicating copy to clipboard operation
gravitino copied to clipboard

Published 20 hours ago • verified publisher icon apache

[#2738] feat(catalog-lakehouse-paimon): support PaimonCatalog implementation to manage Paimon table operations

Open SteNicholas opened this issue 10 months ago • 13 comments

What changes were proposed in this pull request?

Introduce baseline implementation of PaimonCatalog to manage Paimon table operations.

Why are the changes needed?

Support PaimonCatalog implementation to provide Gravitino catalog for Paimon table operation management.

Fix: #2738

Does this PR introduce any user-facing change?

Introduce PaimonCatalog, PaimonCatalogOperations and PaimonTableOps as implementation of a Paimon catalog in Gravitino.

How was this patch tested?

  • TestPaimonTableOps
  • TestCatalogUtils
  • TestTableOpsUtils
  • TestTypeUtils
  • TestPaimonConfig

SteNicholas avatar Apr 01 '24 03:04 SteNicholas

Overal LGTM, except minor comments

FANNG1 avatar Apr 01 '24 13:04 FANNG1

@SteNicholas , as Gravitino 0.5 is planned to be released at 4.19, only contains Paimon skeleton code seems not too much benefit to end users, I prefer to delay the merge after 0.5 is released, WDYT?

FANNG1 avatar Apr 02 '24 03:04 FANNG1

@FANNG1, make sense to me. I will push other pull requests of Paimon catalog based on this pull request and compared with this pull request. WDYT?

SteNicholas avatar Apr 02 '24 03:04 SteNicholas

@FANNG1, make sense to me. I will push other pull requests of Paimon catalog based on this pull request and compared with this pull request. WDYT?

ok

FANNG1 avatar Apr 02 '24 06:04 FANNG1

@FANNG1, @qqqttt123, I have addressed above comment. PTAL.

SteNicholas avatar Apr 02 '24 06:04 SteNicholas

LGTM. Prepared for 0.6. After 0.5 branch is created, we will merge this pull request.

qqqttt123 avatar Apr 02 '24 07:04 qqqttt123

@SteNicholas thanks for your work, could you split into smaller PR? like:

  1. code skeleton
  2. integrate test skeleton
  3. schema operations
  4. basic table operations
  5. more advanced features like partition, distribution,alterTable etc.

FANNG1 avatar Apr 13 '24 03:04 FANNG1

@FANNG1, IMO, splitting this pull request have certain works. Could this pull request take a review firstly?

SteNicholas avatar Apr 16 '24 13:04 SteNicholas

@FANNG1, IMO, splitting this pull request have certain works. Could this pull request take a review firstly?

sure, but we're struggling for 0.5 release recent days, plan to review it after 0.5 is released. And totally I prefer to merge and review smaller PRs.

FANNG1 avatar Apr 16 '24 14:04 FANNG1

@SteNicholas @FANNG1 can you please move forward this PR?

jerryshao avatar May 13 '24 06:05 jerryshao

@SteNicholas @FANNG1 can you please move forward this PR?

plan to review this PR today or tomorrow.

FANNG1 avatar May 13 '24 07:05 FANNG1

@FANNG1, IMO, splitting this pull request have certain works. Could this pull request take a review firstly?

@SteNicholas, the overall architecture LGTM, do you like to split to smaller PRs and add Integrate tests?

FANNG1 avatar May 13 '24 09:05 FANNG1

@FANNG1, I will split the implementation to small pull requests. Thanks for your review.

SteNicholas avatar May 13 '24 11:05 SteNicholas

@FANNG1, I will split the implementation to small pull requests. Thanks for your review.

Hi, gently ping @SteNicholas May I ask if we have any plans to move this pr forward? Thank you!

caican00 avatar Jun 03 '24 08:06 caican00

@caican00, I will push the small implementation this week.

SteNicholas avatar Jun 04 '24 03:06 SteNicholas

@caican00, I will push the small implementation this week.

Gently ping @SteNicholas hi, do you mind I help to split this PR into smaller ones? we really hope to use this feature, thank you very much and looking forward to your reply.

caican00 avatar Jun 12 '24 02:06 caican00

@caican00, I will push the small implementation this week.

Gently ping @SteNicholas hi, do you mind I help to split this PR into smaller ones? we really hope to use this feature, thank you very much and looking forward to your reply.

After communication ，i will take over from @SteNicholas to split this pr into smaller ones and add integration tests. cc @SteNicholas @FANNG1

caican00 avatar Jun 18 '24 07:06 caican00