Re: [DISCUSS] Apache Iceberg 1.7.0 Release Cutoff

2024-10-22 Thread Prashant Singh
Hi Russell,

I recently stumbled across an issue in Iceberg Kafka-Connect which shows up
when we are using UUID / FixedType, which leads to failure when we are
actually writing the files, so makes it kinda unusable when we are using
these data-types.
I have a fix out for it already :
https://github.com/apache/iceberg/pull/11346/files . Would request you
include this as well into 1.7.0.

Regards,
Prashant Singh

On Mon, Oct 21, 2024 at 9:13 AM Wing Yew Poon 
wrote:

> Hi Russell,
> There is a data correctness issue (
> https://github.com/apache/iceberg/issues/11221) that I have a fix for (
> https://github.com/apache/iceberg/pull/11247). This is a serious issue,
> and I'd like to see the fix go into 1.7.0.
> Eduard has already approved the PR, but he asked if you or Amogh would
> take a look as well.
> Thanks,
> Wing Yew
>
>
> On Mon, Oct 21, 2024 at 8:56 AM Russell Spitzer 
> wrote:
>
>> That's still my current plan
>>
>> On Mon, Oct 21, 2024 at 10:52 AM Rodrigo Meneses 
>> wrote:
>>
>>> Hi, team. Are we still targeting to cut off on October 25th and release
>>> by Oct 31the, for the 1.7.0 release?
>>> Thanks
>>> -Rodrigo
>>>
>>>
>>> On Thu, Oct 3, 2024 at 9:03 PM Jean-Baptiste Onofré 
>>> wrote:
>>>
 Hi Russ

 As discussed during the community sync, I agree with this plan.

 In the meantime (as you saw on row lineage doc), I'm working on V3
 spec proposals (reviews, PRs, ...).

 If needed, I can be volunteer as release manager for the 1.7.0 release.

 Regards
 JB

 On Fri, Oct 4, 2024 at 5:55 AM Russell Spitzer
  wrote:
 >
 > Hi y'all!
 >
 > As discussed at the community sync on Wednesday, October has begun
 and we are beginning to flesh out the 1.7.0 release as well as the V3 Table
 Spec. Since we are a little worried that we won't have all of the Spec
 items we want by the end of October,  we discussed that we may want to just
 do a release with what we have at the end of the month.
 >
 > It was noted that we have a lot of exciting things already in and we
 may be able to get just the beginning of V3 support as well.
 >
 > To that end it was proposed that we do the next Iceberg release at
 the end of the month (Oct 31st) , and have the cutoff a week before (Oct
 25th). Does anyone have objections or statements of support for this plan?
 >
 > With this in mind please also start marking any remaining PR's or
 Issues that you have with 1.7.0 so we can prioritize them for the cutoff
 date.
 >
 >
 > Thanks everyone for your time,
 > Russ

>>>


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

2024-10-22 Thread rdb...@gmail.com
> For example, the `Snapshot` `summary` field is optional in V1 but
required in V2. Therefore, the REST spec definition should mark the
`summary` field as optional to support both versions.

Yeah, this is technically true. But as I said in my first email, unless you
have tables that are 5 years old, it's unlikely that this is going to be a
problem. A failure here is more likely with newer implementations that have
a bug. So I'd argue there's value in leaving it as required.

On Mon, Oct 21, 2024 at 9:41 AM Kevin Liu  wrote:

> > No. They were introduced at the same time.
> Great! Since the `summary` field and the `operation` key were introduced
> together, we should enforce the rule that the `summary` field must always
> have an accompanying `operation` key. This has been addressed in PR 11354
> [1].
>
> > I am strongly against this. The REST spec should be independent of the
> table versions.
> That makes sense. For the REST spec to support both V1 and V2 tables, it
> should "accept" the least common denominator between the two versions. For
> example, the `Snapshot` `summary` field is optional in V1 but required in
> V2. Therefore, the REST spec definition should mark the `summary` field as
> optional to support both versions. However, the current REST spec leans
> towards the V2 table spec; fields that are optional in V1 and required in
> V2 are marked as required in the spec, such as `TableMetadata.table-uuid`
> [2][3] and `Snapshot.summary` [4][5].
>
> Would love to get other people's thoughts on this.
>
> Best,
> Kevin Liu
>
> [1] https://github.com/apache/iceberg/pull/11354
> [2]
> https://github.com/apache/iceberg/blob/8e743a5b5209569f84b6bace36e1106c67e1eab3/open-api/rest-catalog-open-api.yaml#L2414
> [3] https://iceberg.apache.org/spec/#table-metadata-fields
> [4]
> https://github.com/apache/iceberg/blob/8e743a5b5209569f84b6bace36e1106c67e1eab3/open-api/rest-catalog-open-api.yaml#L2325
> [5] https://iceberg.apache.org/spec/#snapshots
>
> On Sun, Oct 20, 2024 at 11:24 AM rdb...@gmail.com 
> wrote:
>
>> 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. 

Re: Overwrite old properties on table replace with REST catalog

2024-10-22 Thread rdb...@gmail.com
Thanks Vladimir! Would you like to open a PR to make that change? It sounds
like another good item to put into the "Implementation notes" section.

On Sun, Oct 20, 2024 at 11:41 PM Vladimir Ozerov 
wrote:

> 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 <
>> vo

Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Prashant Singh
+1 (non-binding)

Regards,
Prashant

On Tue, Oct 22, 2024 at 10:50 AM John Zhuge  wrote:

> +1 (non-binding)
>
> John Zhuge
>
>
> On Tue, Oct 22, 2024 at 9:45 AM Jack Ye  wrote:
>
>> +1 (binding)
>>
>> Best,
>> Jack Ye
>>
>> On Tue, Oct 22, 2024 at 9:32 AM Dmitri Bourlatchkov
>>  wrote:
>>
>>> Thanks for the reply Eduard!
>>>
>>> I think it is fine to defer fine-tuning credential refreshes to a later
>>> PR.
>>>
>>> I'm upgrading my vote to +1 (non-binding).
>>>
>>> Cheers,
>>> Dmitri.
>>>
>>> On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner <
>>> etudenhoef...@apache.org> wrote:
>>>
 Hey Dmitri,

 the idea behind the endpoint itself is really just to provide *valid*
 credentials for a given table when a client asks for them.
 If the server returned you two S3 credentials, the client will use the
 one with the longest prefix and if that credential expires, it will ask the
 server again for *valid* credentials.
 That means the server can again return you two S3 credentials, even if
 that second unused credential from the previous endpoint call didn't expire
 yet.
 I don't think we'd want to complicate the endpoint *at this point* to
 have a differentiation between what specific credentials a client wants to
 receive from the server.

 Thanks,
 Eduard

 On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
  wrote:

> -0 (non-binding)
>
> If multiple credentials are vended for a table (which is allowed) the
> current API requires all credentials to be refreshed, when any of the
> previous credentials expires. I think this is suboptimal (but can probably
> be made to work in most practical cases).
>
> Cheers,
> Dmitri.
>
> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
> etudenhoef...@apache.org> wrote:
>
>> Hey everyone,
>>
>> I'd like to vote on #11281
>> , which introduces a
>> new endpoint and allows retrieving/refreshing vended credentials for a
>> given table.
>>
>> Please vote +1 if you generally agree with the path forward.
>>
>> Please vote in the next 72 hours
>>
>> [ ] +1, commit the proposed spec changes
>> [ ] -0
>> [ ] -1, do not make these changes because . . .
>>
>>
>> Thanks everyone,
>>
>> Eduard
>>
>


Re: Spec changes for deletion vectors

2024-10-22 Thread rdb...@gmail.com
Thanks for the careful consideration here, everyone.

I completely agree that we do not want this to be confused as setting a
precedent about delegating design decisions. I don't think the Delta
community would do that either! We have to make decisions that are right
for Iceberg.

It sounds like we're mostly in agreement on both of Russell's concerns. The
value of compatibility in this case likely overrides the pain and annoyance.

I'll update the spec PRs so that we can move forward.

Ryan

On Mon, Oct 21, 2024 at 3:05 PM Szehon Ho 
wrote:

> Im +1 for adding DV (Goal 1) and also +1 for the ability for Iceberg
> readers to read Delta Lake DV’s, as the magic byte, CRC make sense
> design-wise (Goal 2).
>
> Its nice that there's cross-community collaboration probably in other
> areas Im not looking at, but I'm -0.5 on writing an otherwise unjustifiable
> (albeit small) format change to be compatible by Delta Lake readers (Goal
> 3), to register that this is irregular direction for the project, for
> Iceberg to choose a spec just so it can be read by another project.  For
> example, it can be debated, but discussions in the past about adding export
> functionality to Hive, were not supported to be added to Iceberg project.
> So it won’t block if community supports it, but want to raise awareness.
>
> Thanks,
> Szehon
>
>
>
> On Oct 21, 2024, at 2:36 PM, Micah Kornfield 
> wrote:
>
> I agree with everything Russell said.  I think we should move forward with
> the current format of DVs to favor compatibility.  I'll add that I think
> the collaboration aspect likely applies to other aspects as well outside of
> Deletion Vectors (e.g. the work that is happening on Variant type).
>
> Thanks,
> Micah
>
> On Mon, Oct 21, 2024 at 1:45 PM Russell Spitzer 
> wrote:
>
>> I've thought about this a lot and talked it over with a lot of folks. As
>> I've noted before my main concerns are
>>
>> A. Setting a precedent that we are delegating design decisions to another
>> project
>> B. Setting unnecessary requirements that can only really be checked by
>> integration tests with another system
>>
>> I think the value of compatibility can override the pain/annoyance of B.
>>
>> For A, I just want to make clear I will not go along with any sort of
>> concession in design in the future. I think it's ok that we do it this
>> time, but in the future if the Delta Delete Vector format changes I would
>> really hope that the community around Delta would make those decisions in
>> collaboration with the Apache Iceberg community. This is probably to be a
>> red line for me in the future and I won't be able to go ahead in the future
>> with any changes that aren't necessary for the Iceberg project regardless
>> of their adoption in other formats.
>>
>> So with that said, I'm in support of any of the above solutions but I
>> think just going with full compatibility with Delta (down to storage format
>> details) is the right choice to try to get the two communities working
>> together in the future.
>>
>> On Sat, Oct 19, 2024 at 4:38 PM rdb...@gmail.com 
>> wrote:
>>
>>> Thanks for the summary, Szehon!
>>>
>>> I would add one thing to the "minimum" for each option. Because we want
>>> to be able to seek directly to the DV for a particular data file, I think
>>> it's important to start the blob with magic bytes. That way the reader can
>>> validate that the offset was correct and that the contents of the blob are
>>> in the expected format. So I'd add magic bytes to options (1) and (2). In
>>> (2) we would want the magic bytes to match the ones from Delta to be able
>>> to read DV files written by Delta.
>>>
>>> Ryan
>>>
>>> On Thu, Oct 17, 2024 at 8:55 PM Szehon Ho 
>>> wrote:
>>>
 So based on Micah's original goals, switch 2 and 3:

 1. The best possible implementation of DVs (limited redundancy, no
 extraneous fields, CPU efficiency, minimal space, etc).
 2.  The ability for Iceberg readers to read Delta Lake DVs
 3.  The ability for Delta Lake readers to read Iceberg DVs

 The minimum for each option are:
 (1) = DV
 (2) = DV (little-endian) + CRC  (big-endian)
 (3) = Len (big-endian) + Magic + DV (litte-endian) + CRC (big-endian)

 Design wise,  CRC can be useful, Magic may be useful/OK, but Len is
 controversial as its a different length than the bounding Puffin length
 (uncompressed and also a partial length without CRC).  Big-endian Len/CRC
 is not ideal.

 I hope thats right?  As it took me somet ime to go through the thread,
 hope it saves others some time too.
 Thanks
 Szehon


 On Thu, Oct 17, 2024 at 4:16 PM Anton Okolnychyi 
 wrote:

> For the conversion from Delta to Iceberg, wouldn't we need to scan all
>> of the Delta Vectors if we choose a different CRC or other endian-ness?
>
>
> Exactly, we would not be able to expose Delta as Iceberg if we choose
> a different checksum type or byte order.
>

Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread rdb...@gmail.com
+1 (binding)

Thanks for your work on this!

On Tue, Oct 22, 2024 at 2:47 PM Prashant Singh 
wrote:

> +1 (non-binding)
>
> Regards,
> Prashant
>
> On Tue, Oct 22, 2024 at 10:50 AM John Zhuge  wrote:
>
>> +1 (non-binding)
>>
>> John Zhuge
>>
>>
>> On Tue, Oct 22, 2024 at 9:45 AM Jack Ye  wrote:
>>
>>> +1 (binding)
>>>
>>> Best,
>>> Jack Ye
>>>
>>> On Tue, Oct 22, 2024 at 9:32 AM Dmitri Bourlatchkov
>>>  wrote:
>>>
 Thanks for the reply Eduard!

 I think it is fine to defer fine-tuning credential refreshes to a later
 PR.

 I'm upgrading my vote to +1 (non-binding).

 Cheers,
 Dmitri.

 On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner <
 etudenhoef...@apache.org> wrote:

> Hey Dmitri,
>
> the idea behind the endpoint itself is really just to provide *valid*
> credentials for a given table when a client asks for them.
> If the server returned you two S3 credentials, the client will use the
> one with the longest prefix and if that credential expires, it will ask 
> the
> server again for *valid* credentials.
> That means the server can again return you two S3 credentials, even if
> that second unused credential from the previous endpoint call didn't 
> expire
> yet.
> I don't think we'd want to complicate the endpoint *at this point* to
> have a differentiation between what specific credentials a client wants to
> receive from the server.
>
> Thanks,
> Eduard
>
> On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
>  wrote:
>
>> -0 (non-binding)
>>
>> If multiple credentials are vended for a table (which is allowed) the
>> current API requires all credentials to be refreshed, when any of the
>> previous credentials expires. I think this is suboptimal (but can 
>> probably
>> be made to work in most practical cases).
>>
>> Cheers,
>> Dmitri.
>>
>> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
>> etudenhoef...@apache.org> wrote:
>>
>>> Hey everyone,
>>>
>>> I'd like to vote on #11281
>>> , which introduces a
>>> new endpoint and allows retrieving/refreshing vended credentials for a
>>> given table.
>>>
>>> Please vote +1 if you generally agree with the path forward.
>>>
>>> Please vote in the next 72 hours
>>>
>>> [ ] +1, commit the proposed spec changes
>>> [ ] -0
>>> [ ] -1, do not make these changes because . . .
>>>
>>>
>>> Thanks everyone,
>>>
>>> Eduard
>>>
>>


Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Amogh Jahagirdar
+1 (binding)

On Tue, Oct 22, 2024 at 5:20 PM rdb...@gmail.com  wrote:

> +1 (binding)
>
> Thanks for your work on this!
>
> On Tue, Oct 22, 2024 at 2:47 PM Prashant Singh 
> wrote:
>
>> +1 (non-binding)
>>
>> Regards,
>> Prashant
>>
>> On Tue, Oct 22, 2024 at 10:50 AM John Zhuge  wrote:
>>
>>> +1 (non-binding)
>>>
>>> John Zhuge
>>>
>>>
>>> On Tue, Oct 22, 2024 at 9:45 AM Jack Ye  wrote:
>>>
 +1 (binding)

 Best,
 Jack Ye

 On Tue, Oct 22, 2024 at 9:32 AM Dmitri Bourlatchkov
  wrote:

> Thanks for the reply Eduard!
>
> I think it is fine to defer fine-tuning credential refreshes to a
> later PR.
>
> I'm upgrading my vote to +1 (non-binding).
>
> Cheers,
> Dmitri.
>
> On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner <
> etudenhoef...@apache.org> wrote:
>
>> Hey Dmitri,
>>
>> the idea behind the endpoint itself is really just to provide *valid*
>> credentials for a given table when a client asks for them.
>> If the server returned you two S3 credentials, the client will use
>> the one with the longest prefix and if that credential expires, it will 
>> ask
>> the server again for *valid* credentials.
>> That means the server can again return you two S3 credentials, even
>> if that second unused credential from the previous endpoint call didn't
>> expire yet.
>> I don't think we'd want to complicate the endpoint *at this point*
>> to have a differentiation between what specific credentials a client 
>> wants
>> to receive from the server.
>>
>> Thanks,
>> Eduard
>>
>> On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
>>  wrote:
>>
>>> -0 (non-binding)
>>>
>>> If multiple credentials are vended for a table (which is allowed)
>>> the current API requires all credentials to be refreshed, when any of 
>>> the
>>> previous credentials expires. I think this is suboptimal (but can 
>>> probably
>>> be made to work in most practical cases).
>>>
>>> Cheers,
>>> Dmitri.
>>>
>>> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
>>> etudenhoef...@apache.org> wrote:
>>>
 Hey everyone,

 I'd like to vote on #11281
 , which introduces a
 new endpoint and allows retrieving/refreshing vended credentials for a
 given table.

 Please vote +1 if you generally agree with the path forward.

 Please vote in the next 72 hours

 [ ] +1, commit the proposed spec changes
 [ ] -0
 [ ] -1, do not make these changes because . . .


 Thanks everyone,

 Eduard

>>>


Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Christian Thiel
+1 (non-binding). Great feature, thanks!

Von: Amogh Jahagirdar <2am...@gmail.com>
Gesendet: Tuesday, October 22, 2024 8:00:00 PM
An: dev@iceberg.apache.org 
Betreff: Re: [VOTE] Endpoint for refreshing vended credentials

+1 (binding)

On Tue, Oct 22, 2024 at 5:20 PM rdb...@gmail.com 
mailto:rdb...@gmail.com>> wrote:
+1 (binding)

Thanks for your work on this!

On Tue, Oct 22, 2024 at 2:47 PM Prashant Singh 
mailto:prashant010...@gmail.com>> wrote:
+1 (non-binding)

Regards,
Prashant

On Tue, Oct 22, 2024 at 10:50 AM John Zhuge 
mailto:jzh...@apache.org>> wrote:
+1 (non-binding)

John Zhuge


On Tue, Oct 22, 2024 at 9:45 AM Jack Ye 
mailto:yezhao...@gmail.com>> wrote:
+1 (binding)

Best,
Jack Ye

On Tue, Oct 22, 2024 at 9:32 AM Dmitri Bourlatchkov 
 wrote:
Thanks for the reply Eduard!

I think it is fine to defer fine-tuning credential refreshes to a later PR.

I'm upgrading my vote to +1 (non-binding).

Cheers,
Dmitri.

On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner 
mailto:etudenhoef...@apache.org>> wrote:
Hey Dmitri,

the idea behind the endpoint itself is really just to provide valid credentials 
for a given table when a client asks for them.
If the server returned you two S3 credentials, the client will use the one with 
the longest prefix and if that credential expires, it will ask the server again 
for valid credentials.
That means the server can again return you two S3 credentials, even if that 
second unused credential from the previous endpoint call didn't expire yet.
I don't think we'd want to complicate the endpoint at this point to have a 
differentiation between what specific credentials a client wants to receive 
from the server.

Thanks,
Eduard

On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov 
 wrote:
-0 (non-binding)

If multiple credentials are vended for a table (which is allowed) the current 
API requires all credentials to be refreshed, when any of the previous 
credentials expires. I think this is suboptimal (but can probably be made to 
work in most practical cases).

Cheers,
Dmitri.

On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner 
mailto:etudenhoef...@apache.org>> wrote:
Hey everyone,

I'd like to vote on #11281, which 
introduces a new endpoint and allows retrieving/refreshing vended credentials 
for a given table.

Please vote +1 if you generally agree with the path forward.

Please vote in the next 72 hours

[ ] +1, commit the proposed spec changes
[ ] -0
[ ] -1, do not make these changes because . . .


Thanks everyone,

Eduard


Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Jean-Baptiste Onofré
Hi

+1 (non binding)

@Dmitri I don't think it's an issue: the endpoint can return multiple
credentials, the client will take one. The credential refresh is not
so costly though (and required to verify valid credential at retrieval
time).

Regards
JB

On Tue, Oct 22, 2024 at 5:09 PM Eduard Tudenhöfner
 wrote:
>
> Hey Dmitri,
>
> the idea behind the endpoint itself is really just to provide valid 
> credentials for a given table when a client asks for them.
> If the server returned you two S3 credentials, the client will use the one 
> with the longest prefix and if that credential expires, it will ask the 
> server again for valid credentials.
> That means the server can again return you two S3 credentials, even if that 
> second unused credential from the previous endpoint call didn't expire yet.
> I don't think we'd want to complicate the endpoint at this point to have a 
> differentiation between what specific credentials a client wants to receive 
> from the server.
>
> Thanks,
> Eduard
>
> On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov 
>  wrote:
>>
>> -0 (non-binding)
>>
>> If multiple credentials are vended for a table (which is allowed) the 
>> current API requires all credentials to be refreshed, when any of the 
>> previous credentials expires. I think this is suboptimal (but can probably 
>> be made to work in most practical cases).
>>
>> Cheers,
>> Dmitri.
>>
>> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner 
>>  wrote:
>>>
>>> Hey everyone,
>>>
>>> I'd like to vote on #11281, which introduces a new endpoint and allows 
>>> retrieving/refreshing vended credentials for a given table.
>>>
>>> Please vote +1 if you generally agree with the path forward.
>>>
>>> Please vote in the next 72 hours
>>>
>>> [ ] +1, commit the proposed spec changes
>>> [ ] -0
>>> [ ] -1, do not make these changes because . . .
>>>
>>>
>>> Thanks everyone,
>>>
>>> Eduard


Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Jack Ye
+1 (binding)

Best,
Jack Ye

On Tue, Oct 22, 2024 at 9:32 AM Dmitri Bourlatchkov
 wrote:

> Thanks for the reply Eduard!
>
> I think it is fine to defer fine-tuning credential refreshes to a later PR.
>
> I'm upgrading my vote to +1 (non-binding).
>
> Cheers,
> Dmitri.
>
> On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner <
> etudenhoef...@apache.org> wrote:
>
>> Hey Dmitri,
>>
>> the idea behind the endpoint itself is really just to provide *valid*
>> credentials for a given table when a client asks for them.
>> If the server returned you two S3 credentials, the client will use the
>> one with the longest prefix and if that credential expires, it will ask the
>> server again for *valid* credentials.
>> That means the server can again return you two S3 credentials, even if
>> that second unused credential from the previous endpoint call didn't expire
>> yet.
>> I don't think we'd want to complicate the endpoint *at this point* to
>> have a differentiation between what specific credentials a client wants to
>> receive from the server.
>>
>> Thanks,
>> Eduard
>>
>> On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
>>  wrote:
>>
>>> -0 (non-binding)
>>>
>>> If multiple credentials are vended for a table (which is allowed) the
>>> current API requires all credentials to be refreshed, when any of the
>>> previous credentials expires. I think this is suboptimal (but can probably
>>> be made to work in most practical cases).
>>>
>>> Cheers,
>>> Dmitri.
>>>
>>> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
>>> etudenhoef...@apache.org> wrote:
>>>
 Hey everyone,

 I'd like to vote on #11281
 , which introduces a new
 endpoint and allows retrieving/refreshing vended credentials for a given
 table.

 Please vote +1 if you generally agree with the path forward.

 Please vote in the next 72 hours

 [ ] +1, commit the proposed spec changes
 [ ] -0
 [ ] -1, do not make these changes because . . .


 Thanks everyone,

 Eduard

>>>


Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Dmitri Bourlatchkov
Thanks for the reply Eduard!

I think it is fine to defer fine-tuning credential refreshes to a later PR.

I'm upgrading my vote to +1 (non-binding).

Cheers,
Dmitri.

On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner <
etudenhoef...@apache.org> wrote:

> Hey Dmitri,
>
> the idea behind the endpoint itself is really just to provide *valid*
> credentials for a given table when a client asks for them.
> If the server returned you two S3 credentials, the client will use the one
> with the longest prefix and if that credential expires, it will ask the
> server again for *valid* credentials.
> That means the server can again return you two S3 credentials, even if
> that second unused credential from the previous endpoint call didn't expire
> yet.
> I don't think we'd want to complicate the endpoint *at this point* to
> have a differentiation between what specific credentials a client wants to
> receive from the server.
>
> Thanks,
> Eduard
>
> On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
>  wrote:
>
>> -0 (non-binding)
>>
>> If multiple credentials are vended for a table (which is allowed) the
>> current API requires all credentials to be refreshed, when any of the
>> previous credentials expires. I think this is suboptimal (but can probably
>> be made to work in most practical cases).
>>
>> Cheers,
>> Dmitri.
>>
>> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
>> etudenhoef...@apache.org> wrote:
>>
>>> Hey everyone,
>>>
>>> I'd like to vote on #11281
>>> , which introduces a new
>>> endpoint and allows retrieving/refreshing vended credentials for a given
>>> table.
>>>
>>> Please vote +1 if you generally agree with the path forward.
>>>
>>> Please vote in the next 72 hours
>>>
>>> [ ] +1, commit the proposed spec changes
>>> [ ] -0
>>> [ ] -1, do not make these changes because . . .
>>>
>>>
>>> Thanks everyone,
>>>
>>> Eduard
>>>
>>


Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread John Zhuge
+1 (non-binding)

John Zhuge


On Tue, Oct 22, 2024 at 9:45 AM Jack Ye  wrote:

> +1 (binding)
>
> Best,
> Jack Ye
>
> On Tue, Oct 22, 2024 at 9:32 AM Dmitri Bourlatchkov
>  wrote:
>
>> Thanks for the reply Eduard!
>>
>> I think it is fine to defer fine-tuning credential refreshes to a later
>> PR.
>>
>> I'm upgrading my vote to +1 (non-binding).
>>
>> Cheers,
>> Dmitri.
>>
>> On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner <
>> etudenhoef...@apache.org> wrote:
>>
>>> Hey Dmitri,
>>>
>>> the idea behind the endpoint itself is really just to provide *valid*
>>> credentials for a given table when a client asks for them.
>>> If the server returned you two S3 credentials, the client will use the
>>> one with the longest prefix and if that credential expires, it will ask the
>>> server again for *valid* credentials.
>>> That means the server can again return you two S3 credentials, even if
>>> that second unused credential from the previous endpoint call didn't expire
>>> yet.
>>> I don't think we'd want to complicate the endpoint *at this point* to
>>> have a differentiation between what specific credentials a client wants to
>>> receive from the server.
>>>
>>> Thanks,
>>> Eduard
>>>
>>> On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
>>>  wrote:
>>>
 -0 (non-binding)

 If multiple credentials are vended for a table (which is allowed) the
 current API requires all credentials to be refreshed, when any of the
 previous credentials expires. I think this is suboptimal (but can probably
 be made to work in most practical cases).

 Cheers,
 Dmitri.

 On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
 etudenhoef...@apache.org> wrote:

> Hey everyone,
>
> I'd like to vote on #11281
> , which introduces a
> new endpoint and allows retrieving/refreshing vended credentials for a
> given table.
>
> Please vote +1 if you generally agree with the path forward.
>
> Please vote in the next 72 hours
>
> [ ] +1, commit the proposed spec changes
> [ ] -0
> [ ] -1, do not make these changes because . . .
>
>
> Thanks everyone,
>
> Eduard
>



Re: [Discuss] Iceberg View Interoperability

2024-10-22 Thread Will Raschkowski
It’s all nascent, but we have a relatively solid experience going from Spark to 
Substrait [1], Substrait to Calcite [2], and Calcite to one of its supported 
SQL dialects [3]. All these translations are Java-based.

[1]: https://github.com/substrait-io/substrait-java/pull/271
[2]: https://github.com/substrait-io/substrait-java/blob/main/isthmus/README.md
[3]: 
https://svn.apache.org/repos/asf/calcite/site/apidocs/org/apache/calcite/sql/dialect/package-summary.html

From: Ajantha Bhat 
Date: Tuesday, 22 October 2024 at 08:22
To: dev@iceberg.apache.org 
Subject: Re: [Discuss] Iceberg View Interoperability
CAUTION: This email originates from an external party (outside of Palantir). If 
you believe this message is suspicious in nature, please use the "Report 
Message" button built into Outlook.

Thanks Dan for the reply.
but I'm not convinced using a procedure is a good idea or really moves things 
forward in that direction.
I just feel like the procedure approach has a number of drawbacks including, 
relying on the user to do the translation, being dependent on Spark for 
defining the other representations (the API would typically be available to you 
if you're using spark), there are a number of quoting/escaping issues that make 
it awkward to define, and it introduces a second way to define a view.

I didn't consider this as a problem as the procedure is not exposing any new 
functionalities, users can still achieve this using java API and end up with 
the same situations mentioned above. Also, not just Spark, each engine can have 
this procedure.
But I see what you mean, having a centralized solution independent of the 
engine can be better here.
I feel like PyIceberg is the right path forward here.

If we are using the python based solutions like SQLGlot, it makes sense to use 
the PyIceberg to expose a view API to translate and add additional dialects 
from it. But SQLGlot also doesn't support every engine out there (Like Flink, 
Dremio, Impala etc). So, we may still need an API to manually add the dialect 
for unsupported engines from python.

I am sure we have people here who work on Coral, SQLGlot, Substrait and similar 
technologies in this mailing list. I will wait for a week to see if there are 
any opinions on the same. If not, we can start the development with SQLGlot.

- Ajantha

On Thu, Oct 17, 2024 at 9:32 PM Daniel Weeks 
mailto:dwe...@apache.org>> wrote:
Hey Ajantha,

I think it's good to figure out a path forward for extending view support, but 
I'm not convinced using a procedure is a good idea or really moves things 
forward in that direction.

As you already indicated, there are a number of different libraries to 
translate views, but of the various options, It feels like there are two 
approaches that resonate with me are either letting the engine perform the 
translation (like Trino has done with Coral) or supporting views in python 
[github.com]
 so that you can use a combination of libraries like SQLGlot.  Python is the 
most universally available and is already used in a lot of tooling for 
running/managing DDL statements.

I just feel like the procedure approach has a number of drawbacks including, 
relying on the user to do the translation, being dependent on Spark for 
defining the other representations (the API would typically be available to you 
if you're using spark), there are a number of quoting/escaping issues that make 
it awkward to define, and it introduces a second way to define a view.

Long-term IR (e.g. substrate) is the ideal, but I have questions about whether 
we'll be able to achieve enough consensus to gain wide adoption.  I feel like 
PyIceberg is the right path forward here.

-Dan





On Thu, Oct 17, 2024 at 5:19 AM Ajantha Bhat 
mailto:ajanthab...@gmail.com>> wrote:
Hi everyone,

It’s been over six months since Iceberg views were introduced (in version 
1.5.0), and while we’ve seen some benefits—such as versioning and cross-engine 
recognition—there’s still a critical gap in terms of true interoperability. 
Although views created by one engine are recognized by another, they currently 
cannot be queried across engines as one engine cannot understand the SQL 
dialect of another engine.

To address this, I propose a two-step approach:

Step 1: A temporary solution to manually add SQL dialects.
It is already possible to do this using some java code but most of the view 
users interact via SQL, there’s currently no SQL-native method to manually add 
dialects from other engines into Iceberg views. I suggest adding a call 
procedure to enable this. I've recently worked on this and have submitted the 
following PRs for review:

  *   Spark-3.5: Refactor BaseProcedure to support views [#11326 
[github.com]

Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Fanng
+1 (non-binding)

Christian Thiel  于2024年10月23日周三 08:22写道:

> +1 (non-binding). Great feature, thanks!
> --
> *Von:* Amogh Jahagirdar <2am...@gmail.com>
> *Gesendet:* Tuesday, October 22, 2024 8:00:00 PM
> *An:* dev@iceberg.apache.org 
> *Betreff:* Re: [VOTE] Endpoint for refreshing vended credentials
>
> +1 (binding)
>
> On Tue, Oct 22, 2024 at 5:20 PM rdb...@gmail.com  wrote:
>
> +1 (binding)
>
> Thanks for your work on this!
>
> On Tue, Oct 22, 2024 at 2:47 PM Prashant Singh 
> wrote:
>
> +1 (non-binding)
>
> Regards,
> Prashant
>
> On Tue, Oct 22, 2024 at 10:50 AM John Zhuge  wrote:
>
> +1 (non-binding)
>
> John Zhuge
>
>
> On Tue, Oct 22, 2024 at 9:45 AM Jack Ye  wrote:
>
> +1 (binding)
>
> Best,
> Jack Ye
>
> On Tue, Oct 22, 2024 at 9:32 AM Dmitri Bourlatchkov
>  wrote:
>
> Thanks for the reply Eduard!
>
> I think it is fine to defer fine-tuning credential refreshes to a later PR.
>
> I'm upgrading my vote to +1 (non-binding).
>
> Cheers,
> Dmitri.
>
> On Tue, Oct 22, 2024 at 11:11 AM Eduard Tudenhöfner <
> etudenhoef...@apache.org> wrote:
>
> Hey Dmitri,
>
> the idea behind the endpoint itself is really just to provide *valid*
> credentials for a given table when a client asks for them.
> If the server returned you two S3 credentials, the client will use the one
> with the longest prefix and if that credential expires, it will ask the
> server again for *valid* credentials.
> That means the server can again return you two S3 credentials, even if
> that second unused credential from the previous endpoint call didn't expire
> yet.
> I don't think we'd want to complicate the endpoint *at this point* to
> have a differentiation between what specific credentials a client wants to
> receive from the server.
>
> Thanks,
> Eduard
>
> On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
>  wrote:
>
> -0 (non-binding)
>
> If multiple credentials are vended for a table (which is allowed) the
> current API requires all credentials to be refreshed, when any of the
> previous credentials expires. I think this is suboptimal (but can probably
> be made to work in most practical cases).
>
> Cheers,
> Dmitri.
>
> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
> etudenhoef...@apache.org> wrote:
>
> Hey everyone,
>
> I'd like to vote on #11281 ,
> which introduces a new endpoint and allows
> retrieving/refreshing vended credentials for a given table.
>
> Please vote +1 if you generally agree with the path forward.
>
> Please vote in the next 72 hours
>
> [ ] +1, commit the proposed spec changes
> [ ] -0
> [ ] -1, do not make these changes because . . .
>
>
> Thanks everyone,
>
> Eduard
>
>


Re: [DISCUSS] Apache Iceberg 1.7.0 Release Cutoff

2024-10-22 Thread Jean-Baptiste Onofré
Hi Prashant

Thanks for the heads up. We still have time to review and include in
the release.

I will do a pass on your PR.

Thanks !
Regards
JB

On Tue, Oct 22, 2024 at 9:29 PM Prashant Singh  wrote:
>
> Hi Russell,
>
> I recently stumbled across an issue in Iceberg Kafka-Connect which shows up 
> when we are using UUID / FixedType, which leads to failure when we are 
> actually writing the files, so makes it kinda unusable when we are using 
> these data-types.
> I have a fix out for it already : 
> https://github.com/apache/iceberg/pull/11346/files . Would request you 
> include this as well into 1.7.0.
>
> Regards,
> Prashant Singh
>
> On Mon, Oct 21, 2024 at 9:13 AM Wing Yew Poon  
> wrote:
>>
>> Hi Russell,
>> There is a data correctness issue 
>> (https://github.com/apache/iceberg/issues/11221) that I have a fix for 
>> (https://github.com/apache/iceberg/pull/11247). This is a serious issue, and 
>> I'd like to see the fix go into 1.7.0.
>> Eduard has already approved the PR, but he asked if you or Amogh would take 
>> a look as well.
>> Thanks,
>> Wing Yew
>>
>>
>> On Mon, Oct 21, 2024 at 8:56 AM Russell Spitzer  
>> wrote:
>>>
>>> That's still my current plan
>>>
>>> On Mon, Oct 21, 2024 at 10:52 AM Rodrigo Meneses  wrote:

 Hi, team. Are we still targeting to cut off on October 25th and release by 
 Oct 31the, for the 1.7.0 release?
 Thanks
 -Rodrigo


 On Thu, Oct 3, 2024 at 9:03 PM Jean-Baptiste Onofré  
 wrote:
>
> Hi Russ
>
> As discussed during the community sync, I agree with this plan.
>
> In the meantime (as you saw on row lineage doc), I'm working on V3
> spec proposals (reviews, PRs, ...).
>
> If needed, I can be volunteer as release manager for the 1.7.0 release.
>
> Regards
> JB
>
> On Fri, Oct 4, 2024 at 5:55 AM Russell Spitzer
>  wrote:
> >
> > Hi y'all!
> >
> > As discussed at the community sync on Wednesday, October has begun and 
> > we are beginning to flesh out the 1.7.0 release as well as the V3 Table 
> > Spec. Since we are a little worried that we won't have all of the Spec 
> > items we want by the end of October,  we discussed that we may want to 
> > just do a release with what we have at the end of the month.
> >
> > It was noted that we have a lot of exciting things already in and we 
> > may be able to get just the beginning of V3 support as well.
> >
> > To that end it was proposed that we do the next Iceberg release at the 
> > end of the month (Oct 31st) , and have the cutoff a week before (Oct 
> > 25th). Does anyone have objections or statements of support for this 
> > plan?
> >
> > With this in mind please also start marking any remaining PR's or 
> > Issues that you have with 1.7.0 so we can prioritize them for the 
> > cutoff date.
> >
> >
> > Thanks everyone for your time,
> > Russ


Re: Overwrite old properties on table replace with REST catalog

2024-10-22 Thread Jean-Baptiste Onofré
I second Ryan here, it would be great to clarify in the
"implementation notes" section.

Thanks !
Regards
JB

On Wed, Oct 23, 2024 at 1:10 AM rdb...@gmail.com  wrote:
>
> Thanks Vladimir! Would you like to open a PR to make that change? It sounds 
> like another good item to put into the "Implementation notes" section.
>
> On Sun, Oct 20, 2024 at 11:41 PM Vladimir Ozerov  
> wrote:
>>
>> 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

Re: [Discuss] Iceberg View Interoperability

2024-10-22 Thread Ajantha Bhat
Thanks Dan for the reply.

but I'm not convinced using a procedure is a good idea or really moves
> things forward in that direction.
> I just feel like the procedure approach has a number of drawbacks
> including, relying on the user to do the translation, being dependent on
> Spark for defining the other representations (the API would typically be
> available to you if you're using spark), there are a number of
> quoting/escaping issues that make it awkward to define, and it introduces a
> second way to define a view.


I didn't consider this as a problem as the procedure is not exposing any
new functionalities, users can still achieve this using java API and end up
with the same situations mentioned above. Also, not just Spark, each engine
can have this procedure.
But I see what you mean, having a centralized solution independent of the
engine can be better here.

I feel like PyIceberg is the right path forward here.


If we are using the python based solutions like SQLGlot, it makes sense to
use the PyIceberg to expose a view API to translate and add additional
dialects from it. But SQLGlot also doesn't support every engine out there
(Like Flink, Dremio, Impala etc). So, we may still need an API to manually
add the dialect for unsupported engines from python.

I am sure we have people here who work on Coral, SQLGlot, Substrait and
similar technologies in this mailing list. I will wait for a week to see if
there are any opinions on the same. If not, we can start the development
with SQLGlot.

- Ajantha

On Thu, Oct 17, 2024 at 9:32 PM Daniel Weeks  wrote:

> Hey Ajantha,
>
> I think it's good to figure out a path forward for extending view support,
> but I'm not convinced using a procedure is a good idea or really moves
> things forward in that direction.
>
> As you already indicated, there are a number of different libraries to
> translate views, but of the various options, It feels like there are two
> approaches that resonate with me are either letting the engine perform the
> translation (like Trino has done with Coral) or supporting views in python
>  so that you can use
> a combination of libraries like SQLGlot.  Python is the most universally
> available and is already used in a lot of tooling for running/managing DDL
> statements.
>
> I just feel like the procedure approach has a number of drawbacks
> including, relying on the user to do the translation, being dependent on
> Spark for defining the other representations (the API would typically be
> available to you if you're using spark), there are a number of
> quoting/escaping issues that make it awkward to define, and it introduces a
> second way to define a view.
>
> Long-term IR (e.g. substrate) is the ideal, but I have questions about
> whether we'll be able to achieve enough consensus to gain wide adoption.  I
> feel like PyIceberg is the right path forward here.
>
> -Dan
>
>
>
>
>
> On Thu, Oct 17, 2024 at 5:19 AM Ajantha Bhat 
> wrote:
>
>> Hi everyone,
>>
>> It’s been over six months since Iceberg views were introduced (in version
>> 1.5.0), and while we’ve seen some benefits—such as versioning and
>> cross-engine recognition—there’s still a critical gap in terms of true
>> interoperability. Although views created by one engine are recognized by
>> another, they currently cannot be queried across engines as one engine
>> cannot understand the SQL dialect of another engine.
>>
>> To address this, I propose a two-step approach:
>>
>>
>> *Step 1: A temporary solution to manually add SQL dialects.*It is
>> already possible to do this using some java code but most of the view users
>> interact via SQL, there’s currently no SQL-native method to manually add
>> dialects from other engines into Iceberg views. I suggest adding a call
>> procedure to enable this. I've recently worked on this and have submitted
>> the following PRs for review:
>>
>>- Spark-3.5: Refactor BaseProcedure to support views [#11326
>>]
>>- Spark-3.5: Procedure to add view dialect [#11327
>>]
>>
>> With these changes, Iceberg views could be queried across different
>> engines using their respective dialects.
>> Please review and provide feedback.
>>
>>
>> *Step 2: A long-term solution for full view interoperability.*For a
>> sustainable solution, we need to consider both open-source and commercial
>> engines (e.g., Snowflake, Dremio, BigQuery). A framework that can
>> automatically translate SQL dialects from one engine to another seems ideal.
>>
>> I’ve explored Coral*[1]*, which works well with open-source engines like
>> Hive, Trino, and Spark, but doesn’t fully cover commercial engines. In
>> contrast, SQLGlot*[2]* seems to be a better fit, as it supports 23 SQL
>> dialects and would provide a more robust solution for interoperability. But
>> SQLGlot is a python project. So, we need REST or Py4J to work with 

Re: [VOTE] Endpoint for refreshing vended credentials

2024-10-22 Thread Eduard Tudenhöfner
Hey Dmitri,

the idea behind the endpoint itself is really just to provide *valid*
credentials for a given table when a client asks for them.
If the server returned you two S3 credentials, the client will use the one
with the longest prefix and if that credential expires, it will ask the
server again for *valid* credentials.
That means the server can again return you two S3 credentials, even if that
second unused credential from the previous endpoint call didn't expire yet.
I don't think we'd want to complicate the endpoint *at this point* to have
a differentiation between what specific credentials a client wants to
receive from the server.

Thanks,
Eduard

On Mon, Oct 21, 2024 at 6:36 PM Dmitri Bourlatchkov
 wrote:

> -0 (non-binding)
>
> If multiple credentials are vended for a table (which is allowed) the
> current API requires all credentials to be refreshed, when any of the
> previous credentials expires. I think this is suboptimal (but can probably
> be made to work in most practical cases).
>
> Cheers,
> Dmitri.
>
> On Mon, Oct 21, 2024 at 6:07 AM Eduard Tudenhöfner <
> etudenhoef...@apache.org> wrote:
>
>> Hey everyone,
>>
>> I'd like to vote on #11281 ,
>> which introduces a new endpoint and allows
>> retrieving/refreshing vended credentials for a given table.
>>
>> Please vote +1 if you generally agree with the path forward.
>>
>> Please vote in the next 72 hours
>>
>> [ ] +1, commit the proposed spec changes
>> [ ] -0
>> [ ] -1, do not make these changes because . . .
>>
>>
>> Thanks everyone,
>>
>> Eduard
>>
>