Re: Overwrite old properties on table replace with REST catalog

2024-10-20 Thread Jean-Baptiste Onofré
Hi Vladimir,

As Ryan said, it's not a bug: CREATE OR REPLACE can be seen as "CREATE
AND UPDATE" from table format perspective. Specifically for the
properties, it makes sense to not delete the current properties as it
can be used in several use cases (security, tables grouping, ...).
I'm not sure a REST Spec update is required, probably more on the
engine side. In the REST Spec, you can create a table
(https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L553)
and update a table
(https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L975),
and it's up to the query engine to implement the "CREATE OR REPLACE"
with the correct semantic.

Regards
JB

On Sun, Oct 20, 2024 at 9:26 PM Vladimir Ozerov  wrote:
>
> Hi Ryan,
>
> Thanks for the clarification. Yes, I think my confusion was caused by the 
> fact that many engines treat CREATE OR REPLACE as a semantic equivalent of 
> DROP + CREATE, which is performed atomically (e.g., Flink [1]). Table formats 
> add history on top of that, which is expected to be retained, no questions 
> here. Permission propagation also make sense. For properties things become a 
> bit blurry, because on the one hand there are Iceberg specific properties, 
> which may affect table maintenance, and on the other hand there are 
> user-defined properties in the same bag. The question appeared in the first 
> place because I observed a discrepancy in Trino: all catalogs except for REST 
> completely overrides table properties on REPLACE, and REST catalog merges 
> them, which might be confusing to end users. Perhaps some clarification at 
> the spec level might be useful, because without agreement between engines the 
> could be some subtle bugs in multi-engine environments, such as sudden data 
> format changes between replaces, etc.
>
> [1] 
> https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/create/#create-or-replace-table
>
> Regards,
> Vladimir.
>
> On Sun, Oct 20, 2024 at 9:20 PM rdb...@gmail.com  wrote:
>>
>> Hi Vladimir,
>>
>> This isn't a bug. The behavior of CREATE OR REPLACE is to replace the data 
>> of a table, but to maintain things like other refs, snapshot history, 
>> permissions (if supported by the catalog), and table properties. Table 
>> properties are replaced if they are set in the operation like `b` in your 
>> example. This is not the same as a drop and create, which may be what you 
>> want instead.
>>
>> The reason for this behavior is that the CREATE OR REPLACE operation is used 
>> to replace a table's data without needing to handle schema changes between 
>> versions. For example, producing a daily report table that replaces the 
>> previous day. However, the table still exists and it is valuable to be able 
>> to time travel to older versions or to be able to use branches and tags. 
>> Clearly, that means that table history and refs stick around so the table is 
>> not completely new every time it is replaced.
>>
>> Adding on to that, properties control things like ref and snapshot 
>> retention, file format, compression, and other settings. These aren't 
>> settings that need to be carried through in every replace operation. And it 
>> would make no sense if you set the snapshot retention because older 
>> snapshots are retained, only to have it discarded the next time you replace 
>> the table data. A good way to think about this is that table properties are 
>> set infrequently, while table data changes regularly. And the person 
>> changing the data may not be the person tuning the table settings.
>>
>> Hopefully that helps,
>>
>> Ryan
>>
>> On Sun, Oct 20, 2024 at 9:45 AM Vladimir Ozerov  
>> wrote:
>>>
>>> Hi,
>>>
>>> Consider a REST catalog and a user calls "CREATE OR REPLACE " 
>>> command. When processing the command, engines will usually initiate a 
>>> "createOrReplace" transaction and add metadata, such as the properties of a 
>>> new table.
>>>
>>> Users expect a table to be replaced with a new one if it exists, including 
>>> properties. However, I observe the following:
>>>
>>> RESTSessionCatalog loads previous table metadata, adds new properties 
>>> (MetadataUpdate.SetProperties), and invokes the backend
>>> The backend (e.g., Polaris) will typically invoke 
>>> "CatalogHandler.updateTable." There, the previous table state, including 
>>> its properties, is loaded
>>> Finally, metadata updates are applied, and old table properties are merged 
>>> with new ones. That is, if the old table has properties [a=1, b=2], and the 
>>> new table has properties [b=3, c=4], then the final properties would be 
>>> [a=1, b=3, c=4], while the user expects [b=3, c=4].
>>>
>>> It looks like a bug because the user expects complete property replacement 
>>> instead of a merge. Shall we explicitly clear all previous properties in 
>>> RESTSessionCatalog.Builder.replaceTransaction?
>>>
>>> Regards,
>>> Vladimir.
>>>
>>>
>>>
>>> --
>>> Vladimir Ozerov
>>> Founder
>>> querifylabs.com
>
>

Re: Iceberg View Spec Improvements

2024-10-20 Thread Walaa Eldin Moustafa
Hi Everyone,

Thanks for all the discussion so far. I have created a PR to document the
requirements https://github.com/apache/iceberg/pull/11365. Please feel free
to review it or discuss further in the thread.

Thanks,
Walaa.


On Fri, Oct 11, 2024 at 5:19 PM Walaa Eldin Moustafa 
wrote:

> Hi Benny,
>
> > we don't need to list out such restrictions because they really depend
> on the setup
>
> I do not think this is correct. The restrictions do not depend on the
> setup. They rather dictate it. All restrictions discussed in this thread do
> that one way or the other.
>
> The single engine (Dremio) example does not apply to this discussion. The
> spec is clear if a single engine is in use, but the spec is not limited for
> single engine use cases.
>
> > If Dremio intended for that view to be readable by Spark, it would have
> to adhere to all those restrictions I listed before.
>
> Sure, but those restrictions are only stated in the mailing list (in many
> forms). We are discussing if we should add them to the spec (in one form).
>
> Thanks
> Walaa.
>
>
> On Fri, Oct 11, 2024 at 4:56 PM Benny Chow  wrote:
>
>> Hi Russell
>>
>> Yes, you listed out the requirements to make the two Spark engines case
>> work.  Basically, it allows each engine to dynamically resolve the table
>> identifiers under the correct catalog name.
>>
>> Hello Walla
>>
>> IMO, we don't need to list out such restrictions because they really
>> depend on the setup.   Multiple Iceberg catalogs?  Multiple engines?
>> Consistent catalog names?  Are views created with USE in context?  Today,
>> in Dremio, we save tons of views to Nessie with fully qualified SQL
>> identifiers to other sources such as mysql or snowflake.  Those views may
>> or may not have default-catalog and default-namespaces set depending on the
>> USE context.  If Dremio intended for that view to be readable by Spark, it
>> would have to adhere to all those restrictions I listed before.
>>
>> Thanks
>> Benny
>>
>> On Fri, Oct 11, 2024 at 10:00 AM Walaa Eldin Moustafa <
>> wa.moust...@gmail.com> wrote:
>>
>>> Benny, "Iceberg View Spec Improvements" includes documenting what is
>>> supported and what is not. You listed a few restrictions. Many of them are
>>> not documented on the current spec. Documenting them is what this thread is
>>> about. We are trying to reach a consensus on the necessary constraints (so
>>> we are not over- or under-restricting).
>>>
>>> Russell, I think what you stated is a version of the restrictions. From
>>> my point of view, the list of the necessary restrictions are:
>>>
>>> * Engines must share the same default catalog names, ensuring that
>>> partially specified SQL identifiers with catalog omitted are resolved to
>>> the same fully specified SQL identifier across all engines.
>>> * Engines must share the same default namespaces, ensuring that SQL
>>> identifiers without catalog and namespace are resolved to the same fully
>>> specified SQL identifier across all engines.
>>> * All engines must resolve a fully specified SQL identifier to the same
>>> storage table in the same storage catalog.
>>>
>>> Please let me know if this aligns with what you stated.
>>>
>>> Thanks,
>>> Walaa.
>>>
>>>


Re: Overwrite old properties on table replace with REST catalog

2024-10-20 Thread Vladimir Ozerov
Hi Jean-Baptiste,

Agreed. REST spec looks good. I am talking about the general spec, where it
might be useful to add a hint to engine developers, that CREATE OR REPLACE
semantics in Iceberg is expected to follow slightly different semantics.
This is already broken in Trino: depending on catalog type users may get
either classical "DROP + CREATE" (for non-REST catalogs), or "CREATE AND
UPDATE" for REST catalog. For Flink, their official docs say that CREATE OR
REPLACE == DROP + CREATE, while for Iceberg tables this should not be the
case. These are definitively things that should be fixed at engine levels.
But at the same time it highlights that engine developers are having hard
time defining proper semantics for CREATE OR REPLACE in the Iceberg
integrations, so a paragraph or so in the main Iceberg spec may help us
align expectations.

Regards,
Vladimir.

On Mon, Oct 21, 2024 at 8:28 AM Jean-Baptiste Onofré 
wrote:

> Hi Vladimir,
>
> As Ryan said, it's not a bug: CREATE OR REPLACE can be seen as "CREATE
> AND UPDATE" from table format perspective. Specifically for the
> properties, it makes sense to not delete the current properties as it
> can be used in several use cases (security, tables grouping, ...).
> I'm not sure a REST Spec update is required, probably more on the
> engine side. In the REST Spec, you can create a table
> (
> https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L553
> )
> and update a table
> (
> https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L975
> ),
> and it's up to the query engine to implement the "CREATE OR REPLACE"
> with the correct semantic.
>
> Regards
> JB
>
> On Sun, Oct 20, 2024 at 9:26 PM Vladimir Ozerov 
> wrote:
> >
> > Hi Ryan,
> >
> > Thanks for the clarification. Yes, I think my confusion was caused by
> the fact that many engines treat CREATE OR REPLACE as a semantic equivalent
> of DROP + CREATE, which is performed atomically (e.g., Flink [1]). Table
> formats add history on top of that, which is expected to be retained, no
> questions here. Permission propagation also make sense. For properties
> things become a bit blurry, because on the one hand there are Iceberg
> specific properties, which may affect table maintenance, and on the other
> hand there are user-defined properties in the same bag. The question
> appeared in the first place because I observed a discrepancy in Trino: all
> catalogs except for REST completely overrides table properties on REPLACE,
> and REST catalog merges them, which might be confusing to end users.
> Perhaps some clarification at the spec level might be useful, because
> without agreement between engines the could be some subtle bugs in
> multi-engine environments, such as sudden data format changes between
> replaces, etc.
> >
> > [1]
> https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/create/#create-or-replace-table
> >
> > Regards,
> > Vladimir.
> >
> > On Sun, Oct 20, 2024 at 9:20 PM rdb...@gmail.com 
> wrote:
> >>
> >> Hi Vladimir,
> >>
> >> This isn't a bug. The behavior of CREATE OR REPLACE is to replace the
> data of a table, but to maintain things like other refs, snapshot history,
> permissions (if supported by the catalog), and table properties. Table
> properties are replaced if they are set in the operation like `b` in your
> example. This is not the same as a drop and create, which may be what you
> want instead.
> >>
> >> The reason for this behavior is that the CREATE OR REPLACE operation is
> used to replace a table's data without needing to handle schema changes
> between versions. For example, producing a daily report table that replaces
> the previous day. However, the table still exists and it is valuable to be
> able to time travel to older versions or to be able to use branches and
> tags. Clearly, that means that table history and refs stick around so the
> table is not completely new every time it is replaced.
> >>
> >> Adding on to that, properties control things like ref and snapshot
> retention, file format, compression, and other settings. These aren't
> settings that need to be carried through in every replace operation. And it
> would make no sense if you set the snapshot retention because older
> snapshots are retained, only to have it discarded the next time you replace
> the table data. A good way to think about this is that table properties are
> set infrequently, while table data changes regularly. And the person
> changing the data may not be the person tuning the table settings.
> >>
> >> Hopefully that helps,
> >>
> >> Ryan
> >>
> >> On Sun, Oct 20, 2024 at 9:45 AM Vladimir Ozerov <
> voze...@querifylabs.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>> Consider a REST catalog and a user calls "CREATE OR REPLACE "
> command. When processing the command, engines will usually initiate a
> "createOrReplace" transaction and add metadata, such as the properties of a
> new table.
> >>>
> >>> Users expect a table to be repla

Re: Overwrite old properties on table replace with REST catalog

2024-10-20 Thread rdb...@gmail.com
Hi Vladimir,

This isn't a bug. The behavior of CREATE OR REPLACE is to replace the data
of a table, but to maintain things like other refs, snapshot history,
permissions (if supported by the catalog), and table properties. Table
properties are replaced if they are set in the operation like `b` in your
example. This is not the same as a drop and create, which may be what you
want instead.

The reason for this behavior is that the CREATE OR REPLACE operation is
used to replace a table's data without needing to handle schema changes
between versions. For example, producing a daily report table that replaces
the previous day. However, the table still exists and it is valuable to be
able to time travel to older versions or to be able to use branches and
tags. Clearly, that means that table history and refs stick around so the
table is not completely new every time it is replaced.

Adding on to that, properties control things like ref and snapshot
retention, file format, compression, and other settings. These aren't
settings that need to be carried through in every replace operation. And it
would make no sense if you set the snapshot retention because older
snapshots are retained, only to have it discarded the next time you replace
the table data. A good way to think about this is that table properties are
set infrequently, while table data changes regularly. And the person
changing the data may not be the person tuning the table settings.

Hopefully that helps,

Ryan

On Sun, Oct 20, 2024 at 9:45 AM Vladimir Ozerov 
wrote:

> Hi,
>
> Consider a REST catalog and a user calls "CREATE OR REPLACE "
> command. When processing the command, engines will usually initiate a
> "createOrReplace" transaction and add metadata, such as the properties of a
> new table.
>
> Users expect a table to be replaced with a new one if it exists,
> including properties. However, I observe the following:
>
>1. RESTSessionCatalog loads previous table metadata, adds new
>properties (MetadataUpdate.SetProperties), and invokes the backend
>2. The backend (e.g., Polaris) will typically invoke
>"CatalogHandler.updateTable." There, the previous table state, including
>its properties, is loaded
>3. Finally, metadata updates are applied, and old table properties are
>merged with new ones. That is, if the old table has properties [a=1, b=2],
>and the new table has properties [b=3, c=4], then the final properties
>would be [a=1, b=3, c=4], while the user expects [b=3, c=4].
>
> It looks like a bug because the user expects complete property replacement
> instead of a merge. Shall we explicitly clear all previous properties
> in RESTSessionCatalog.Builder.replaceTransaction?
>
> Regards,
> Vladimir.
>
>
>
> --
> *Vladimir Ozerov*
> Founder
> querifylabs.com
>


Re: [DISCUSS] Discrepancy Between Iceberg Spec and Java Implementation for Snapshot summary's 'operation' key

2024-10-20 Thread Kevin Liu
Hey folks,

Thanks, everyone for the discussion, and thanks Ryan for providing the
historical context.
Enforce the `operation` key in Snapshot’s `summary` field

When serializing the `Snapshot` object from JSON, the Java implementation
does not enforce that the `summary` field must contain an `operation` key.
In the V1 spec, the `summary` field is optional, while in the V2 spec, it
is required. However, in both versions, if a `summary` field is present, it
must include an `operation` key. Any `summary` field lacking an `operation`
key should be considered invalid.

I’ve addressed this issue in PR 11354 [1] by adding this constraint when
parsing JSON.

> We initially did not have the snapshot summary or operation. When I added
the summary, the operation was intended to be required in cases where the
summary is present. It should always be there if the summary is and the
summary should always be there unless you wrote the metadata.json file way
back in 2017 or 2018.

@Ryan, does this constraint also apply to `metadata.json` files from
2017/2018? Was it ever valid to have a `summary` field without the
`operation` key?

> Well, the spec says nothing about a top-level `operation` field in JSON
[1]. Yet the Java implementation produces it [2] and removes the operation
from the summary map. This seems inconsistent?

@Anton, the Java `Snapshot` object includes both the `summary` and
`operation` fields. When serializing to JSON, the `operation` field is
included in the `summary` map [2], rather than as a top-level field. During
deserialization from JSON, the `operation` field is extracted from the
`summary` map [3].

I believe this is consistent with the table spec, which defines the JSON
output, not how the `Snapshot` object is implemented in Java.
On REST spec and Table spec

Thanks, Yufei, for highlighting the difference between the REST spec and
the table spec. I mistakenly used the REST spec
(`rest-catalog-open-api.yaml` [4]) as the source of truth for V2 tables.

Looking at the REST spec file, it can be challenging to determine how a
REST server should handle V1 versus V2 tables. Even for V2 tables, the
current version of the file combines features from V2, along with
additional changes made in preparation for the upcoming V3 spec.

Would it be helpful to create alternative versions of the REST spec
specifically for referencing V1 and V2 tables? The goal would be to have a
"frozen" version of the REST spec dedicated to V1 tables and another for V2
tables while allowing the current REST spec file to evolve as needed.

Taking a step back, I think we need more documentation on the REST spec,
including support for different table versions and guidance on upgrading
from one version to another. I’d love to hear everyone’s thoughts on this.


Best,

Kevin Liu


[1] https://github.com/apache/iceberg/pull/11354

[2]
https://github.com/apache/iceberg/blob/17f1c4d2205b59c2bd877d4d31bbbef9e90979c5/core/src/main/java/org/apache/iceberg/SnapshotParser.java#L63-L66

[3]
https://github.com/apache/iceberg/blob/17f1c4d2205b59c2bd877d4d31bbbef9e90979c5/core/src/main/java/org/apache/iceberg/SnapshotParser.java#L124-L137

[4]
https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml


On Sat, Oct 19, 2024 at 7:48 PM Sung Yun  wrote:

> Hi Ryan, thank you for your response!
>
> That detailed context is very helpful in allowing me to understanding why
> the REST catalog spec has evolved the way it has, and how the Table Spec
> and the REST Catalog Spec should each be referenced in the sub-communities
> (like in PyIceberg). I'll keep those motivations in mind as we discuss
> those Specs in the future.
>
> Also, here's a small PR to specify more explicitly that the operation
> field should be a required field in the summary field:
> https://github.com/apache/iceberg/pull/11355
>
> Sung
>
> On 2024/10/19 22:14:59 "rdb...@gmail.com" wrote:
> > I can provide some historical context here about how the table spec
> evolved
> > and how the REST spec works with respect to table versions.
> >
> > We initially did not have the snapshot summary or operation. When I added
> > the summary, the operation was intended to be required in cases where the
> > summary is present. It should always be there if the summary is and the
> > summary should always be there unless you wrote the metadata.json file
> way
> > back in 2017 or 2018. It looks like the spec could be more clear that the
> > operation is required when summary is present. Anyone want to open a PR?
> >
> > Anton, I don't think there is a top-level operation field. The Java
> > Snapshot class tracks the operation as top-level, but it is always stored
> > in the summary. I think this is consistent with the spec.
> >
> > For the REST spec, I think that it should be strictly optional to support
> > v1 tables written with no summary, but it should always be present. I'd
> > probably leave it required since it already is and is catching a valuable
> > error case here.
> >

Re: [DISCUSS] Discrepancy Between Iceberg Spec and Java Implementation for Snapshot summary's 'operation' key

2024-10-20 Thread rdb...@gmail.com
Was it ever valid to have a summary field without the operation key?

No. They were introduced at the same time.

Would it be helpful to create alternative versions of the REST spec
specifically for referencing V1 and V2 tables?

I am strongly against this. The REST spec should be independent of the
table versions. Any table format version can be passed and the table format
should be the canonical reference for what is allowed. We want to avoid
cases where there are discrepancies. The table spec is canonical for table
metadata, and the REST spec allows passing it.

On Sun, Oct 20, 2024 at 11:18 AM Kevin Liu  wrote:

> Hey folks,
>
> Thanks, everyone for the discussion, and thanks Ryan for providing the
> historical context.
> Enforce the `operation` key in Snapshot’s `summary` field
>
> When serializing the `Snapshot` object from JSON, the Java implementation
> does not enforce that the `summary` field must contain an `operation` key.
> In the V1 spec, the `summary` field is optional, while in the V2 spec, it
> is required. However, in both versions, if a `summary` field is present, it
> must include an `operation` key. Any `summary` field lacking an `operation`
> key should be considered invalid.
>
> I’ve addressed this issue in PR 11354 [1] by adding this constraint when
> parsing JSON.
>
> > We initially did not have the snapshot summary or operation. When I
> added the summary, the operation was intended to be required in cases where
> the summary is present. It should always be there if the summary is and the
> summary should always be there unless you wrote the metadata.json file
> way back in 2017 or 2018.
>
> @Ryan, does this constraint also apply to `metadata.json` files from
> 2017/2018? Was it ever valid to have a `summary` field without the
> `operation` key?
>
> > Well, the spec says nothing about a top-level `operation` field in JSON
> [1]. Yet the Java implementation produces it [2] and removes the operation
> from the summary map. This seems inconsistent?
>
> @Anton, the Java `Snapshot` object includes both the `summary` and
> `operation` fields. When serializing to JSON, the `operation` field is
> included in the `summary` map [2], rather than as a top-level field. During
> deserialization from JSON, the `operation` field is extracted from the
> `summary` map [3].
>
> I believe this is consistent with the table spec, which defines the JSON
> output, not how the `Snapshot` object is implemented in Java.
> On REST spec and Table spec
>
> Thanks, Yufei, for highlighting the difference between the REST spec and
> the table spec. I mistakenly used the REST spec
> (`rest-catalog-open-api.yaml` [4]) as the source of truth for V2 tables.
>
> Looking at the REST spec file, it can be challenging to determine how a
> REST server should handle V1 versus V2 tables. Even for V2 tables, the
> current version of the file combines features from V2, along with
> additional changes made in preparation for the upcoming V3 spec.
>
> Would it be helpful to create alternative versions of the REST spec
> specifically for referencing V1 and V2 tables? The goal would be to have a
> "frozen" version of the REST spec dedicated to V1 tables and another for V2
> tables while allowing the current REST spec file to evolve as needed.
>
> Taking a step back, I think we need more documentation on the REST spec,
> including support for different table versions and guidance on upgrading
> from one version to another. I’d love to hear everyone’s thoughts on this.
>
>
> Best,
>
> Kevin Liu
>
>
> [1] https://github.com/apache/iceberg/pull/11354
>
> [2]
> https://github.com/apache/iceberg/blob/17f1c4d2205b59c2bd877d4d31bbbef9e90979c5/core/src/main/java/org/apache/iceberg/SnapshotParser.java#L63-L66
>
> [3]
> https://github.com/apache/iceberg/blob/17f1c4d2205b59c2bd877d4d31bbbef9e90979c5/core/src/main/java/org/apache/iceberg/SnapshotParser.java#L124-L137
>
> [4]
> https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml
>
>
> On Sat, Oct 19, 2024 at 7:48 PM Sung Yun  wrote:
>
>> Hi Ryan, thank you for your response!
>>
>> That detailed context is very helpful in allowing me to understanding why
>> the REST catalog spec has evolved the way it has, and how the Table Spec
>> and the REST Catalog Spec should each be referenced in the sub-communities
>> (like in PyIceberg). I'll keep those motivations in mind as we discuss
>> those Specs in the future.
>>
>> Also, here's a small PR to specify more explicitly that the operation
>> field should be a required field in the summary field:
>> https://github.com/apache/iceberg/pull/11355
>>
>> Sung
>>
>> On 2024/10/19 22:14:59 "rdb...@gmail.com" wrote:
>> > I can provide some historical context here about how the table spec
>> evolved
>> > and how the REST spec works with respect to table versions.
>> >
>> > We initially did not have the snapshot summary or operation. When I
>> added
>> > the summary, the operation was intended to be required in cases where
>> the
>> > 

Overwrite old properties on table replace with REST catalog

2024-10-20 Thread Vladimir Ozerov
Hi,

Consider a REST catalog and a user calls "CREATE OR REPLACE "
command. When processing the command, engines will usually initiate a
"createOrReplace" transaction and add metadata, such as the properties of a
new table.

Users expect a table to be replaced with a new one if it exists,
including properties. However, I observe the following:

   1. RESTSessionCatalog loads previous table metadata, adds new properties
   (MetadataUpdate.SetProperties), and invokes the backend
   2. The backend (e.g., Polaris) will typically invoke
   "CatalogHandler.updateTable." There, the previous table state, including
   its properties, is loaded
   3. Finally, metadata updates are applied, and old table properties are
   merged with new ones. That is, if the old table has properties [a=1, b=2],
   and the new table has properties [b=3, c=4], then the final properties
   would be [a=1, b=3, c=4], while the user expects [b=3, c=4].

It looks like a bug because the user expects complete property replacement
instead of a merge. Shall we explicitly clear all previous properties
in RESTSessionCatalog.Builder.replaceTransaction?

Regards,
Vladimir.



-- 
*Vladimir Ozerov*
Founder
querifylabs.com


Re: [DISCUSS] Iceberg Summit 2025 ?

2024-10-20 Thread Jean-Baptiste Onofré
Thanks everyone for your feedback.

The preferred format is hybrid and we have new talks tracks possible.

I will prepare the formal proposal to submit to PMC for approval and
organization.

Thanks again,
Regards
JB

Le ven. 27 sept. 2024 à 19:50, Jean-Baptiste Onofré  a
écrit :

> Hi folks,
>
> Last year in June we started to discuss the first edition of the Iceberg
> Summit (https://lists.apache.org/thread/cbgx1jlc9ywn618yod2487g498lgrkt3).
>
>
> The Iceberg Summit was in May 2024, and it was clearly a great community
> event, with a lot of nice talks.
> This first edition was fully virtual.
>
> I think it would be great to have Iceberg Summit 2025, community event,
> but maybe this time a hybrid event.
> Also, regarding the number of talks received by the selection committee
> for Iceberg Summit 2024, I would suggest (for the future Selection
> Committee) to have new talk tracks (like user stories, practitioners, ...).
>
> The process would be similar of Iceberg Summit 2024:
> - first the community discuss here about the idea, kind of event (virtual,
> in person, hybrid), ...
>* should we have another event ?
>* would you like there to be an in-person event ?
>* what kind of talks would you like to hear at such an event ?
>* what kind of talks would you not like to hear at such an event ?
> - if there's no objections, the Iceberg PMC should approve the use of
> Iceberg and the ASF VP M&P should be notified. I can help on the paperwork
> and process again.
> - the PMC will appoint two committees (at least selection and sponsoring
> committees)
>
> Thoughts ?
>
> Regards
> JB
>


Re: Overwrite old properties on table replace with REST catalog

2024-10-20 Thread Vladimir Ozerov
Hi Ryan,

Thanks for the clarification. Yes, I think my confusion was caused by the
fact that many engines treat CREATE OR REPLACE as a semantic equivalent of
DROP + CREATE, which is performed atomically (e.g., Flink [1]). Table
formats add history on top of that, which is expected to be retained, no
questions here. Permission propagation also make sense. For properties
things become a bit blurry, because on the one hand there are Iceberg
specific properties, which may affect table maintenance, and on the other
hand there are user-defined properties in the same bag. The question
appeared in the first place because I observed a discrepancy in Trino: all
catalogs except for REST completely overrides table properties on REPLACE,
and REST catalog merges them, which might be confusing to end users.
Perhaps some clarification at the spec level might be useful, because
without agreement between engines the could be some subtle bugs in
multi-engine environments, such as sudden data format changes between
replaces, etc.

[1]
https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/create/#create-or-replace-table

Regards,
Vladimir.

On Sun, Oct 20, 2024 at 9:20 PM rdb...@gmail.com  wrote:

> Hi Vladimir,
>
> This isn't a bug. The behavior of CREATE OR REPLACE is to replace the data
> of a table, but to maintain things like other refs, snapshot history,
> permissions (if supported by the catalog), and table properties. Table
> properties are replaced if they are set in the operation like `b` in your
> example. This is not the same as a drop and create, which may be what you
> want instead.
>
> The reason for this behavior is that the CREATE OR REPLACE operation is
> used to replace a table's data without needing to handle schema changes
> between versions. For example, producing a daily report table that replaces
> the previous day. However, the table still exists and it is valuable to be
> able to time travel to older versions or to be able to use branches and
> tags. Clearly, that means that table history and refs stick around so the
> table is not completely new every time it is replaced.
>
> Adding on to that, properties control things like ref and snapshot
> retention, file format, compression, and other settings. These aren't
> settings that need to be carried through in every replace operation. And it
> would make no sense if you set the snapshot retention because older
> snapshots are retained, only to have it discarded the next time you replace
> the table data. A good way to think about this is that table properties are
> set infrequently, while table data changes regularly. And the person
> changing the data may not be the person tuning the table settings.
>
> Hopefully that helps,
>
> Ryan
>
> On Sun, Oct 20, 2024 at 9:45 AM Vladimir Ozerov 
> wrote:
>
>> Hi,
>>
>> Consider a REST catalog and a user calls "CREATE OR REPLACE "
>> command. When processing the command, engines will usually initiate a
>> "createOrReplace" transaction and add metadata, such as the properties of a
>> new table.
>>
>> Users expect a table to be replaced with a new one if it exists,
>> including properties. However, I observe the following:
>>
>>1. RESTSessionCatalog loads previous table metadata, adds new
>>properties (MetadataUpdate.SetProperties), and invokes the backend
>>2. The backend (e.g., Polaris) will typically invoke
>>"CatalogHandler.updateTable." There, the previous table state, including
>>its properties, is loaded
>>3. Finally, metadata updates are applied, and old table properties
>>are merged with new ones. That is, if the old table has properties [a=1,
>>b=2], and the new table has properties [b=3, c=4], then the final
>>properties would be [a=1, b=3, c=4], while the user expects [b=3, c=4].
>>
>> It looks like a bug because the user expects complete property
>> replacement instead of a merge. Shall we explicitly clear all previous
>> properties in RESTSessionCatalog.Builder.replaceTransaction?
>>
>> Regards,
>> Vladimir.
>>
>>
>>
>> --
>> *Vladimir Ozerov*
>> Founder
>> querifylabs.com
>>
>

-- 
*Vladimir Ozerov*
Founder
querifylabs.com