Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 merged PR #331: URL: https://github.com/apache/iceberg-rust/pull/331 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@ic

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2254530124 I think this pr is good enough, so I will merge it since it has been long. Thanks @c-thiel 's effort on this, and thanks @Xuanwo @nastra @ZENOTME 's review! -- This is an auto

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2254135228 Rebased on current master -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific c

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1693949639 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,313 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1693949266 ## crates/iceberg/src/spec/mod.rs: ## @@ -38,3 +40,6 @@ pub use sort::*; pub use table_metadata::*; pub use transform::*; pub use values::*; +pub use view_metadat

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1693949164 ## crates/iceberg/src/error.rs: ## @@ -333,6 +335,28 @@ define_from_err!( define_from_err!(std::io::Error, ErrorKind::Unexpected, "IO Operation failed"); +/// C

Re: [PR] View Spec implementation [iceberg-rust]

Xuanwo commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1693949016 ## crates/iceberg/src/error.rs: ## @@ -333,6 +335,28 @@ define_from_err!( define_from_err!(std::io::Error, ErrorKind::Unexpected, "IO Operation failed"); +/// Co

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1693948492 ## crates/iceberg/src/error.rs: ## @@ -333,6 +335,28 @@ define_from_err!( define_from_err!(std::io::Error, ErrorKind::Unexpected, "IO Operation failed"); +/// C

Re: [PR] View Spec implementation [iceberg-rust]

Xuanwo commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1692844248 ## crates/iceberg/src/spec/mod.rs: ## @@ -38,3 +40,6 @@ pub use sort::*; pub use table_metadata::*; pub use transform::*; pub use values::*; +pub use view_metadata

Re: [PR] View Spec implementation [iceberg-rust]

Xuanwo commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1692842973 ## crates/iceberg/src/error.rs: ## @@ -333,6 +335,28 @@ define_from_err!( define_from_err!(std::io::Error, ErrorKind::Unexpected, "IO Operation failed"); +/// Co

Re: [PR] View Spec implementation [iceberg-rust]

Xuanwo commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2252430149 > cc @Xuanwo @nastra @Fokko to take another look Hi, thanks for the invitation, but I'm not familiar with this topic. Also, apologies for not being involved from the start. I will

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2252171188 cc @Xuanwo @nastra @Fokko to take another look -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL a

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1692652244 ## crates/iceberg/src/spec/view_version.rs: ## @@ -71,8 +74,18 @@ impl ViewVersion { /// Get the timestamp of when the view version was created #[i

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1690895876 ## crates/iceberg/src/spec/view_version.rs: ## @@ -71,8 +74,18 @@ impl ViewVersion { /// Get the timestamp of when the view version was created #[inline]

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1690895876 ## crates/iceberg/src/spec/view_version.rs: ## @@ -71,8 +74,18 @@ impl ViewVersion { /// Get the timestamp of when the view version was created #[inline]

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1690900328 ## crates/iceberg/src/spec/table_metadata.rs: ## @@ -144,8 +144,24 @@ impl TableMetadata { /// Returns last updated time. #[inline] -pub fn last_upda

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1690898177 ## crates/iceberg/src/spec/table_metadata.rs: ## @@ -144,8 +144,24 @@ impl TableMetadata { /// Returns last updated time. #[inline] -pub fn last_upda

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1690897640 ## crates/iceberg/src/spec/table_metadata.rs: ## @@ -917,8 +933,24 @@ pub struct SnapshotLog { impl SnapshotLog { /// Returns the last updated timestamp as a

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1690895876 ## crates/iceberg/src/spec/view_version.rs: ## @@ -71,8 +74,18 @@ impl ViewVersion { /// Get the timestamp of when the view version was created #[inline]

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1690858992 ## crates/iceberg/src/spec/view_version.rs: ## @@ -155,6 +155,11 @@ impl ViewRepresentations { pub fn is_empty(&self) -> bool { self.0.is_empty()

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1689945716 ## crates/iceberg/src/spec/table_metadata.rs: ## @@ -917,8 +933,24 @@ pub struct SnapshotLog { impl SnapshotLog { /// Returns the last updated timestam

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1689941239 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1689756208 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1689756208 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

nastra commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1689745756 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688304774 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688330949 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688304774 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688295210 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688276072 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688273791 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688272043 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688253972 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688182253 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor licens

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1688104381 ## crates/iceberg/src/spec/table_metadata.rs: ## @@ -332,7 +332,7 @@ impl TableMetadataBuilder { let table_metadata = TableMetadata { f

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687855767 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687841673 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687837575 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687825014 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687814896 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687796916 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687793896 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,675 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1687784774 ## crates/iceberg/src/spec/table_metadata.rs: ## @@ -332,7 +332,7 @@ impl TableMetadataBuilder { let table_metadata = TableMetadata { format_

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1684477421 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,477 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license

Re: [PR] View Spec implementation [iceberg-rust]

liurenjie1024 commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2233432563 > @nastra just following up on my last comment - could you close our discussion where you agree with my fix? @liurenjie1024 could you maybe have a look at the rust side of things

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2232971946 @nastra just following up on my last comment - could you close our discussion where you agree with my fix? @liurenjie1024 could you maybe have a look at the rust side of things? Ple

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2226849477 @nastra I hope I addressed all your issues regarding functionality. If you are happy with the implementations, feel free to close our discussions :). @ZENOTME it would now be great t

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676802744 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,347 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676801198 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,347 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676800028 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,347 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676796496 ## crates/iceberg/src/spec/view_version.rs: ## @@ -0,0 +1,347 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676794013 ## crates/iceberg/testdata/view_metadata/ViewMetadataV2Valid.json: ## @@ -0,0 +1,58 @@ +{ Review Comment: Ok, thanks Eduard for the Feedback. My preferred

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676795561 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,682 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676795328 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,682 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676794680 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,682 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agre

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1676794013 ## crates/iceberg/testdata/view_metadata/ViewMetadataV2Valid.json: ## @@ -0,0 +1,58 @@ +{ Review Comment: Ok, thanks Eduard for the Feedback. My preferred

Re: [PR] View Spec implementation [iceberg-rust]

nastra commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1675578086 ## crates/iceberg/src/spec/view_metadata.rs: ## @@ -0,0 +1,682 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agree

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1670513358 ## crates/iceberg/testdata/view_metadata/ViewMetadataV2Valid.json: ## @@ -0,0 +1,58 @@ +{ Review Comment: @nastra I added more tests that cover non-happy cases.

Re: [PR] View Spec implementation [iceberg-rust]

twuebi commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2208353517 Thanks for checking this @nastra, I understand that we should be relaxing the constraints of NamespaceIdent then if an empty default-namespace is expected. -- This is an automated me

Re: [PR] View Spec implementation [iceberg-rust]

nastra commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2205682556 Ok I checked the surrounding code and the handling is correct. In https://github.com/apache/iceberg/blob/2a39af894f4f00aa37922ef765cc2583517fa1d1/spark/v3.5/spark-extensions/src/main/sca

Re: [PR] View Spec implementation [iceberg-rust]

nastra commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2205517652 `"default-namespace": []` indicates an empty namespace, not a null one, which is a valid case and there are tests for that here: https://github.com/apache/iceberg/blob/42a2c19cec31c626c

Re: [PR] View Spec implementation [iceberg-rust]

twuebi commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2205402299 Setting the namespace via: ```python3 spark.sql("USE spark_demo") spark.sql("CREATE view vv3 as select * from spark_demo.mytable") ``` ends up setting `default-name

Re: [PR] View Spec implementation [iceberg-rust]

twuebi commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2205383749 Hi @nastra, this script: ```python3 import pyspark from pyspark.conf import SparkConf from pyspark.sql import SparkSession import pandas as pd CATALOG_URL

Re: [PR] View Spec implementation [iceberg-rust]

nastra commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2205201047 > When creating views, spark creates Metadata objects that specify the "default-namespace" field but as an empty vec Is this with plain OSS Spark or are you using the iceberg-spar

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2204539127 @nastra, @Fokko during testing we found a Problem with the "default-namespace". I am hoping for some insights from your side: According to the iceberg view spec, "default-namespace"

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on code in PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1651128747 ## crates/iceberg/testdata/view_metadata/ViewMetadataV2Valid.json: ## @@ -0,0 +1,58 @@ +{ Review Comment: Ok, thanks for the feedback @nastra. I'll add some more

Re: [PR] View Spec implementation [iceberg-rust]

c-thiel commented on PR #331: URL: https://github.com/apache/iceberg-rust/pull/331#issuecomment-2094154795 @Fokko from my side this is good to merge - types are complete and tests are passing. -- This is an automated message from the Apache Git Service. To respond to the message, please l