Core: Remove deprecated APIs for 1.12.0#16452
Conversation
raushanprabhakar1
changed the title
|
Hi @manuzhang , requesting you to review. Thanks |
| /** | ||
| * @deprecated since 1.11.0, visibility will be reduced in 1.12.0. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
we should also address this deprecation and reduce the visibility
| /** | ||
| * @deprecated since 1.11.0, visibility will be reduced in 1.12.0. | ||
| */ | ||
| @Deprecated |
| @@ -777,7 +777,7 @@ protected <T> PositionDelete<T> positionDelete(CharSequence path, long pos) { | |||
|
|
|||
| protected <T> PositionDelete<T> positionDelete(CharSequence path, long pos, T row) { | |||
There was a problem hiding this comment.
we should probably remove this method entirely instead of silently ignoring row
| import org.apache.iceberg.types.Types; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class TestPartitionStats { |
| .spec(spec) | ||
| .build(); | ||
| } else { | ||
| return switch (format) { |
There was a problem hiding this comment.
can you please elaborate why these changes are being done here? This doesn't seem to be related to removing deprecated APIs
| table == null | ||
| ? MetricsConfig.forPositionDelete() | ||
| : MetricsConfig.forPositionDelete(table); | ||
| table == null ? MetricsConfig.forPositionDelete() : MetricsConfig.forPositionDelete(); |
There was a problem hiding this comment.
this now does the same thing in both branches
| GenericRecord record = GenericRecord.create(finalSchema); | ||
| record.setField("file_path", p.path()); | ||
| record.setField("pos", p.pos()); | ||
| record.setField("row", formatVersion >= 3 ? null : p.row()); |
| table == null | ||
| ? MetricsConfig.forPositionDelete() | ||
| : MetricsConfig.forPositionDelete(table); | ||
| : MetricsConfig.forPositionDelete(); |
There was a problem hiding this comment.
this does the same thing in both branches
| table == null | ||
| ? MetricsConfig.forPositionDelete() | ||
| : MetricsConfig.forPositionDelete(table); | ||
| : MetricsConfig.forPositionDelete(); |
There was a problem hiding this comment.
this does the same thing in both if branches now
| old: "method org.apache.iceberg.orc.ORC.WriteBuilder org.apache.iceberg.orc.ORC.WriteBuilder::config(java.lang.String,\ | ||
| \ java.lang.String)" | ||
| justification: "Removing deprecations for 1.2.0" | ||
| "1.10.0": |
There was a problem hiding this comment.
can you please open a separate PR against main where you run the gradle command and where the 1.1.0.0 section is being moved. This is so that we don't introduce this change here, because it's difficult to review
| /** | ||
| * @deprecated since 1.11.0, will be removed in 1.12.0. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
this only removes the javadoc but not the actual method
| @@ -67,7 +67,7 @@ public void update(T row, PartitionSpec spec, StructLike partition) { | |||
|
|
|||
| @Override | |||
| public void delete(CharSequence path, long pos, T row, PartitionSpec spec, StructLike partition) { | |||
There was a problem hiding this comment.
we should probably deprecate this method and have a delete method without the row parameter
2 participants
Summary
Removes all Core APIs marked
@deprecated ... will be removed in 1.12.0as part of #16445. This is the first module-wise PR for the 1.12.0 deprecation cleanup.Deleted classes
SystemProperties— useSystemConfigsPartitionStats— useBasePartitionStatistics/PartitionStatisticsdata.avro.DataReader— usePlannedDataReaderRemoved APIs
BaseScan.io()— usefileIO().get()TableProperties.MANIFEST_LISTS_ENABLED/_DEFAULT(manifest lists are always enabled)ThreadPools.WORKER_THREAD_POOL_SIZE_PROP,newWorkerPool(...)— useSystemConfigs.WORKER_THREAD_POOL_SIZE/newExitingWorkerPool(...)TableScanUtil.hasDeletes(CombinedScanTask),hasEqDeletes(CombinedScanTask)SystemConfigs.NETFLIX_UNSAFE_PARQUET_ID_FALLBACK_ENABLED(Parquet unsafe ID fallback removed)MetricsConfig.forPositionDelete(Table)— useforPositionDelete()StandardEncryptionManager2-arg constructor andwrapKey/unwrapKeySnapshotUtil.newFiles(...),newFilesBetween(...)— useSnapshotChangesRESTSessionCatalog.REST_PAGE_SIZE— useRESTCatalogProperties.PAGE_SIZECatalogHandlers.loadTable(Catalog, TableIdentifier)— use overload withSnapshotModeAvroSchemaUtil.pruneColumns(Schema, Set, NameMapping)— useapplyNameMapping+pruneColumns(Schema, Set)ContentCache.invalidateAll()BaseScanTaskResponse.Builder.withDeleteFiles(...)— usewithFileScanTasks(...)RawDecoderdeprecated constructor — useRawDecoder.create(...)RESTUtil.NAMESPACE_JOINER,NAMESPACE_SPLITTER,encodeNamespace(Namespace),decodeNamespace(String)— use separator-aware overloadsManifestFiles.readPaths/read/open(ManifestFile, FileIO)— use overloads withspecsByIdPartitionStatsHandlerdeprecated field constants,schema(...),readPartitionStatsFile(...)— usePartitionStatisticsequivalents /PartitionStatisticsScanPositionDelete.set(path, pos, row),row()— useset(path, pos)onlyRewriteTablePathUtil.PositionDeleteReaderWriter.writer(..., Schema rowSchema)— use 4-argwriter(...)HadoopFileIO(SerializableSupplier<Configuration>)— useHadoopFileIO(Configuration)Note:
HadoopFileIO.serializeConfWith(...)is retained without@Deprecatedbecause it is still required byHadoopConfigurable(same asResolvingFileIO).Call-site updates outside Core
Spark, Flink, Data, Parquet, and ORC modules were updated where they depended on removed Core APIs (e.g. position-delete
set(path, pos)migration,MetricsConfig.forPositionDelete()).API compatibility
Adds a
1.11.0section to.palantir/revapi.ymldocumenting intentional breaks (justification: "Removing deprecated code for 1.12.0").Test plan
./gradlew :iceberg-core:compileJava :iceberg-core:compileTestJava./gradlew revapi spotlessCheck./gradlew :iceberg-core:test --tests "org.apache.iceberg.rest.TestRESTUtil" --tests "org.apache.iceberg.TestManifestReader" --tests "org.apache.iceberg.PartitionStatisticsScanTestBase"./gradlew :iceberg-core:test./gradlew build -x integrationTest