Core: Add support for `view-default` property in catalog by ebyhr · Pull Request #11064 · apache/iceberg

ebyhr · 2024-09-02T02:39:40Z

Fixes #10822

ebyhr · 2024-09-02T11:52:02Z

CI hit #10172

ebyhr · 2024-09-02T20:38:43Z

@RussellSpitzer Can you review this PR when you have time?

docs/docs/spark-configuration.md

open-api/src/testFixtures/java/org/apache/iceberg/rest/RCKUtils.java

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java

nk1506 · 2024-09-12T14:26:37Z

core/src/main/java/org/apache/iceberg/CatalogProperties.java

  public static final String WAREHOUSE_LOCATION = "warehouse";
  public static final String TABLE_DEFAULT_PREFIX = "table-default.";
  public static final String TABLE_OVERRIDE_PREFIX = "table-override.";
+  public static final String VIEW_DEFAULT_PREFIX = "view-default.";


Thanks @ebyhr for working on this. I also raised a similar PR which I think is not required anymore. Do you think we should also add "view-override." ?

That probably makes sense since we have the table-default as well, but i have no problem this being in a followup

github-actions · 2024-11-13T00:15:44Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

nastra · 2024-11-13T06:43:19Z

@ebyhr are you still working on this?

ebyhr · 2024-11-13T06:49:17Z

@nastra Yes, I was actually waiting for next review round.

RussellSpitzer · 2024-11-13T18:07:40Z

core/src/main/java/org/apache/iceberg/inmemory/InMemoryCatalog.java

  @Override
  public void initialize(String name, Map<String, String> properties) {
    this.catalogName = name != null ? name : InMemoryCatalog.class.getSimpleName();
+    this.catalogProperties = ImmutableMap.copyOf(properties);


Careful here, ImmutableMap.copyOf isn't null safe

RussellSpitzer · 2024-11-13T18:08:24Z

core/src/main/java/org/apache/iceberg/inmemory/InMemoryCatalog.java


+  @Override
+  protected Map<String, String> properties() {
+    return catalogProperties == null ? ImmutableMap.of() : catalogProperties;


Depending on what you do above this may or may not be necessary

I think this is still needed in case initialize() hasn't been called

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java

core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java

core/src/main/java/org/apache/iceberg/view/BaseMetastoreViewCatalog.java

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java

core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java

github-actions · 2024-12-26T00:15:12Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

ebyhr · 2024-12-26T00:19:04Z

Rebased on main branch to resolve conflicts.

github-actions bot added core docs NESSIE labels Sep 2, 2024

ebyhr force-pushed the ebi/view-default-properties branch from a0ed40f to a130aaa Compare September 2, 2024 03:05

ebyhr marked this pull request as draft September 2, 2024 03:17

ebyhr force-pushed the ebi/view-default-properties branch from a130aaa to 2d41458 Compare September 2, 2024 04:03

github-actions bot added the OPENAPI label Sep 2, 2024

ebyhr force-pushed the ebi/view-default-properties branch 5 times, most recently from c4aa6ae to 1b935cb Compare September 2, 2024 05:41

singhpk234 reviewed Sep 3, 2024

View reviewed changes

docs/docs/spark-configuration.md Outdated Show resolved Hide resolved

ebyhr force-pushed the ebi/view-default-properties branch from 1b935cb to 8206c5f Compare September 3, 2024 23:51

ebyhr marked this pull request as ready for review September 4, 2024 00:00

nastra reviewed Sep 9, 2024

View reviewed changes

open-api/src/testFixtures/java/org/apache/iceberg/rest/RCKUtils.java Show resolved Hide resolved

nastra reviewed Sep 9, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Outdated Show resolved Hide resolved

nastra mentioned this pull request Sep 12, 2024

Core: Propagate catalog-level properties to iceberg-views. #11025

Closed

nk1506 reviewed Sep 12, 2024

View reviewed changes

github-actions bot added the stale label Nov 13, 2024

RussellSpitzer reviewed Nov 13, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Outdated Show resolved Hide resolved

github-actions bot removed the stale label Nov 14, 2024

nastra reviewed Nov 14, 2024

View reviewed changes

core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java Outdated Show resolved Hide resolved

nastra reviewed Nov 14, 2024

View reviewed changes

core/src/main/java/org/apache/iceberg/view/BaseMetastoreViewCatalog.java Outdated Show resolved Hide resolved

ebyhr force-pushed the ebi/view-default-properties branch from 2f6f9fc to 5a19171 Compare November 14, 2024 12:52

nastra reviewed Nov 15, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Outdated Show resolved Hide resolved

nastra reviewed Nov 15, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Outdated Show resolved Hide resolved

nastra reviewed Nov 15, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Outdated Show resolved Hide resolved

ebyhr force-pushed the ebi/view-default-properties branch 2 times, most recently from 4afe084 to 7911403 Compare November 16, 2024 07:14

github-actions bot added the hive label Nov 16, 2024

nastra approved these changes Nov 18, 2024

View reviewed changes

ebyhr force-pushed the ebi/view-default-properties branch 2 times, most recently from 178a06f to eb8b60b Compare November 24, 2024 23:43

nastra reviewed Nov 25, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java Outdated Show resolved Hide resolved

ebyhr mentioned this pull request Nov 25, 2024

Core: Set missing table-default property in RESTSessionCatalog #11646

Merged

ebyhr force-pushed the ebi/view-default-properties branch from eb8b60b to 08cb705 Compare November 25, 2024 09:53

nastra approved these changes Nov 25, 2024

View reviewed changes

github-actions bot added the stale label Dec 26, 2024

ebyhr force-pushed the ebi/view-default-properties branch from 08cb705 to 17c3043 Compare December 26, 2024 00:18

Core: Add support for view-default property in catalog

a9d239a

ebyhr force-pushed the ebi/view-default-properties branch from 17c3043 to a9d239a Compare December 26, 2024 00:41

github-actions bot removed the stale label Dec 27, 2024

nastra merged commit f129588 into apache:main Jan 7, 2025
50 checks passed

ebyhr deleted the ebi/view-default-properties branch January 17, 2025 05:18

Conversation

ebyhr commented Sep 2, 2024

Uh oh!

ebyhr commented Sep 2, 2024

Uh oh!

ebyhr commented Sep 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nk1506 Sep 12, 2024

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 13, 2024

Uh oh!

nastra commented Nov 13, 2024

Uh oh!

ebyhr commented Nov 13, 2024

Uh oh!

RussellSpitzer Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

nastra Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 26, 2024

Uh oh!

ebyhr commented Dec 26, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

RussellSpitzer Nov 13, 2024 •

edited

Loading