Fokko commented on code in PR #8673:
URL: https://github.com/apache/iceberg/pull/8673#discussion_r1341606912
##########
api/src/main/java/org/apache/iceberg/ManifestFile.java:
##########
@@ -49,7 +49,7 @@ public interface ManifestFile {
Types.LongType.get(),
"Lowest sequence number in the manifest");
Types.NestedField SNAPSHOT_ID =
- optional(
+ required(
Review Comment:
My point is that the code should follow the spec, and not the other way
around. However, if we've been writing nullable, then we can't make it required
anymore, but my experiment below shows otherwise. We just updated it a week
ago, but I think that's incorrect: https://github.com/apache/iceberg/pull/8600
On the current docs it is still both required:
```
bin/spark-sql \
--packages
org.apache.iceberg:iceberg-spark-runtime-3.4_2.12:1.3.1,software.amazon.awssdk:bundle:2.20.18
\
--conf
spark.sql.extensions=org.apache.iceberg.spark.extensions.IcebergSparkSessionExtensions
...
spark-sql (spielerij)> create table null_or_not_null AS SELECT 1 as id UNION
ALL select 2 UNION ALL select 3;
spark-sql (spielerij)> describe table extended null_or_not_null;
id int
# Metadata Columns
_spec_id int
_partition struct<>
_file string
_pos bigint
_deleted boolean
# Detailed Table Information
Name sandbox.spielerij.null_or_not_null
Type MANAGED
Location
s3://tabular-wh-us-east-1/6efbcaf4-21ae-499d-b340-3bc1a7003f52/45bc0c6c-b104-4b82-8b18-12d0f7acc287
Provider iceberg
Table Properties
[current-snapshot-id=1823638994492416744,format=iceberg/parquet,format-version=1,manifest-rewrite.submitted=2023-09-29T16:53:31.667746002Z,optimizer.enabled=true,write.delete.parquet.compression-codec=zstd,write.metadata.compression-codec=gzip,write.object-storage.enabled=true,write.parquet.compression-codec=zstd,write.summary.partition-limit=100]
```
Cleared the caches:
```
rm -rf ~/.m2/repository/org/apache/iceberg
rm -rf /Users/fokkodriesprong/.ivy2/cache
```
And confirmed:
```
::::::::::::::::::::::::::::::::::::::::::::::
:: UNRESOLVED DEPENDENCIES ::
::::::::::::::::::::::::::::::::::::::::::::::
::
org.apache.iceberg#iceberg-spark-runtime-3.5_2.12;1.4.0-SNAPSHOT: not found
::::::::::::::::::::::::::::::::::::::::::::::
```
Ran locally `./gradlew publishToMavenLocal`:
```
bin/spark-sql \
--packages
org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.4.0-SNAPSHOT,software.amazon.awssdk:bundle:2.20.18
\
--conf
spark.sql.extensions=org.apache.iceberg.spark.extensions.IcebergSparkSessionExtensions
\
spark-sql ()> SELECT * FROM spielerij.null_or_not_null;
23/09/29 18:59:01 WARN ObjectStore: Failed to get database global_temp,
returning NoSuchObjectException
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further
details.
1
2
3
Time taken: 8.449 seconds, Fetched 3 row(s)
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
